krasimir created this revision.
krasimir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107950
Files:
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,14 +8247,25 @@
" 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();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ 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 @@
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 @@
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;
}
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,14 +8247,25 @@
" 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();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ 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 @@
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 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
