[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

[Owen Pan via Phabricator via cfe-commits]

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