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