Author: Krasimir Georgiev Date: 2021-08-12T10:29:06+02:00 New Revision: 45934922fa88b7542c8bcd86889d062fb78efdda
URL: https://github.com/llvm/llvm-project/commit/45934922fa88b7542c8bcd86889d062fb78efdda DIFF: https://github.com/llvm/llvm-project/commit/45934922fa88b7542c8bcd86889d062fb78efdda.diff LOG: [clang-format] improve distinction of K&R function definitions vs attributes After https://github.com/llvm/llvm-project/commit/9da70ab3d43c79116f80fc06aa7cf517374ce42c we saw a few regressions around trailing attribute definitions and in typedefs (examples in the added test cases). There's some tension distinguishing K&R definitions from attributes at the parser level, where we have to decide if we need to put the type of the K&R definition on a new unwrapped line before we have access to the rest of the line, so we're scanning backwards and looking for a pattern like f(a, b). But this type of pattern could also be an attribute macro, or the whole declaration could be a typedef itself. I updated the code to check for a typedef at the beginning of the line and to not consider raw identifiers as possible first K&R declaration (but treated as an attribute macro instead). This is not 100% correct heuristic, but I think it should be reasonably good in practice, where we'll: * likely be in some very C-ish code when using K&R style (e.g., stuff that uses `struct name a;` instead of `name a;` * likely be in some very C++-ish code when using attributes * unlikely mix up the two in the same declaration. Ideally, we should only decide to add the unwrapped line before the K&R declaration after we've scanned the rest of the line an noticed the variable declarations and the semicolon, but the way the parser is organized I don't see a good way to do this in the current parser, which only has good context for the previously visited tokens. I also tried not emitting an unwrapped line there and trying to resolve the situation later in the token annotator and the continuation indenter, and that approach seems promising, but I couldn't make it to work without messing up a bunch of other cases in unit tests. Reviewed By: MyDeveloperDay Differential Revision: https://reviews.llvm.org/D107950 Added: Modified: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index d6b1ebb1239d..0c4cacab5050 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -14,6 +14,7 @@ #include "UnwrappedLineParser.h" #include "FormatToken.h" +#include "clang/Basic/TokenKinds.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" @@ -1007,7 +1008,7 @@ static bool isC78ParameterDecl(const FormatToken *Tok) { if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double, tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short, - tok::kw_unsigned, tok::kw_register, tok::identifier)) + tok::kw_unsigned, tok::kw_register)) return false; Tok = Tok->Previous; @@ -1378,7 +1379,8 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) { break; if (Previous->Previous && Previous->Previous->is(tok::at)) break; - if (isC78ParameterDecl(FormatTok)) { + if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) && + isC78ParameterDecl(FormatTok)) { addUnwrappedLine(); return; } diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 670788caac00..1283aa67b337 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -8247,14 +8247,25 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " return a + b < c;\n" "};", Style); - // The return breaking style doesn't affect object definitions with - // attribute-like macros. + + // The return breaking style doesn't affect: + // * function and object definitions with attribute-like macros verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n" " ABSL_GUARDED_BY(mutex) = {};", getGoogleStyleWithColumns(40)); verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n" " ABSL_GUARDED_BY(mutex); // comment", getGoogleStyleWithColumns(40)); + verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n" + " ABSL_GUARDED_BY(mutex1)\n" + " ABSL_GUARDED_BY(mutex2);", + getGoogleStyleWithColumns(40)); + verifyFormat("Tttttt f(int a, int b)\n" + " ABSL_GUARDED_BY(mutex1)\n" + " ABSL_GUARDED_BY(mutex2);", + getGoogleStyleWithColumns(40)); + // * typedefs + verifyFormat("typedef ATTR(X) char x;", getGoogleStyle()); Style = getGNUStyle(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
