Skip to content

Commit 4593492

Browse files
committed
[clang-format] improve distinction of K&R function definitions vs attributes
After 9da70ab 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
1 parent 6c14688 commit 4593492

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed

clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "UnwrappedLineParser.h"
1616
#include "FormatToken.h"
17+
#include "clang/Basic/TokenKinds.h"
1718
#include "llvm/ADT/STLExtras.h"
1819
#include "llvm/Support/Debug.h"
1920
#include "llvm/Support/raw_ostream.h"
@@ -1007,7 +1008,7 @@ static bool isC78ParameterDecl(const FormatToken *Tok) {
10071008

10081009
if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
10091010
tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
1010-
tok::kw_unsigned, tok::kw_register, tok::identifier))
1011+
tok::kw_unsigned, tok::kw_register))
10111012
return false;
10121013

10131014
Tok = Tok->Previous;
@@ -1378,7 +1379,8 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
13781379
break;
13791380
if (Previous->Previous && Previous->Previous->is(tok::at))
13801381
break;
1381-
if (isC78ParameterDecl(FormatTok)) {
1382+
if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
1383+
isC78ParameterDecl(FormatTok)) {
13821384
addUnwrappedLine();
13831385
return;
13841386
}

clang/unittests/Format/FormatTest.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8247,14 +8247,25 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
82478247
" return a + b < c;\n"
82488248
"};",
82498249
Style);
8250-
// The return breaking style doesn't affect object definitions with
8251-
// attribute-like macros.
8250+
8251+
// The return breaking style doesn't affect:
8252+
// * function and object definitions with attribute-like macros
82528253
verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n"
82538254
" ABSL_GUARDED_BY(mutex) = {};",
82548255
getGoogleStyleWithColumns(40));
82558256
verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n"
82568257
" ABSL_GUARDED_BY(mutex); // comment",
82578258
getGoogleStyleWithColumns(40));
8259+
verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n"
8260+
" ABSL_GUARDED_BY(mutex1)\n"
8261+
" ABSL_GUARDED_BY(mutex2);",
8262+
getGoogleStyleWithColumns(40));
8263+
verifyFormat("Tttttt f(int a, int b)\n"
8264+
" ABSL_GUARDED_BY(mutex1)\n"
8265+
" ABSL_GUARDED_BY(mutex2);",
8266+
getGoogleStyleWithColumns(40));
8267+
// * typedefs
8268+
verifyFormat("typedef ATTR(X) char x;", getGoogleStyle());
82588269

82598270
Style = getGNUStyle();
82608271

0 commit comments

Comments
 (0)