Page MenuHomePhabricator

[clang-format] [PR45614] Incorrectly indents [[nodiscard]] attribute funtions after a macro without semicolon
ClosedPublic

Authored by MyDeveloperDay on May 15 2020, 2:49 AM.

Details

Summary

https://bugs.llvm.org/show_bug.cgi?id=45614

[[nodiscard]] after a macro doesn't behave the same as an attribute resulting in incorrect indentation

This revision corrects that behavior

MACRO

__attribute__((warn_unused_result)) int f3(); // ok

MACRO

    [[nodiscard]] int
    f4(); // bad: unexpectedly indented!

See original Mozilla bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1629756

Before:

class FooWidget : public nsBaseWidget {
public:
  FooWidget();

  NS_DECL_ISUPPORTS_INHERITED

      [[nodiscard]] nsresult
      FunctionOne();
  [[nodiscard]] nsresult FunctionTwo();
};

After:

class FooWidget : public nsBaseWidget {
public:
  FooWidget();

  NS_DECL_ISUPPORTS_INHERITED

  [[nodiscard]] nsresult FunctionOne();
  [[nodiscard]] nsresult FunctionTwo();
};

Diff Detail

Event Timeline

MyDeveloperDay created this revision.May 15 2020, 2:49 AM
MyDeveloperDay retitled this revision from [clang-format] Incorrectly indents [[nodiscard]] attribute funtions after a macro without semicolon to [clang-format] [PR45614] Incorrectly indents [[nodiscard]] attribute funtions after a macro without semicolon.May 15 2020, 2:55 AM

@MyDeveloperDay thanks for the patch, I'm gonna run it agains mozilla to see if there is any fallout.

@MyDeveloperDay thanks for the patch, I'm gonna run it agains mozilla to see if there is any fallout.

I wonder is there a documented way to run clang-format over the gekco sources? when I make a change to clang-format I like to run it over some real code not just the tests, as I know Mozilla is supposed to be completed formatted this might prove to be a better test than running it over LLVM itself (which is alas not 100% formatted)

krasimir added inline comments.May 15 2020, 6:23 AM
clang/lib/Format/UnwrappedLineParser.cpp
906

nit: pass Tok by const reference (makes sure we don't have to deal with nullptr and as bonus will make this patch diff smaller).

MyDeveloperDay marked an inline comment as done.

Change to const &

Abpostelnicu accepted this revision.May 19 2020, 8:14 AM
This revision is now accepted and ready to land.May 19 2020, 8:14 AM
This revision was automatically updated to reflect the committed changes.