owenpan added inline comments.

================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3970-4009
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren)) {
     if (Right.is(TT_CastRParen) ||
         (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen))) {
       return Style.SpacesInParensOptions.InCStyleCasts;
     }
     if (Left.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||
         Right.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||
----------------
Consider wrapping this in a function or lambda. Not sure if it would simply the 
logic with the parens being handled separately:
```
if (Left.is(tok::l_paren)) {
  ...
} else if (Right.is(tok::r_paren)) {
  ...
}


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3990
+          return Style.SpacesInParensOptions.InFunctionDefinitions;
+        else
+          return Style.SpacesInParensOptions.InFunctionDeclarations;
----------------
No `else` after `return`.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);
----------------
gedare wrote:
> HazardyKnusperkeks wrote:
> > Why change this?
> The original test is incomplete/ambiguous. It's either a declaration missing 
> a semicolon, or it's the start of a definition. I made it a definition.
Then can you also add a declaration?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156360/new/

https://reviews.llvm.org/D156360

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to