[clang-format] Fix template arguments in macros

[sstwcw]

Summary

Fixes https://github.com/llvm/llvm-project/issues/57738

old

#define FOO(typeName, realClass)                               \
  {                                                            \
#typeName, foo < FooType>(new foo <realClass>(#typeName))  \
  }

new

#define FOO(typeName, realClass)                                \
  { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }

Previously, when an UnwrappedLine began with a hash in a macro
definition, the program incorrectly assumed the line was a preprocessor
directive. It should be stringification.

The rule in spaceRequiredBefore was added in 8b5297117b. Its purpose is
to add a space in an include directive. It also added a space to a
template opener when the line began with a stringification hash. So we
changed it.

[owenpan]

clang/unittests/Format/FormatTest.cpp

9803–9805

And perhaps move it to the end of the test.

clang/lib/Format/TokenAnnotator.h

 };
 
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front().Tok), Level(Line.Level),
         MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
         MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
-        InPPDirective(Line.InPPDirective),
+        InPPDirective(Line.InPPDirective), InMacroBody(Line.InMacroBody),
         MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
         IsMultiVariableDeclStmt(false), Affected(false),
         LeadingEmptyLinesAffected(false), ChildrenAffected(false),
         IsContinuation(Line.IsContinuation),
         FirstStartColumn(Line.FirstStartColumn) {
     assert(!Line.Tokens.empty());
 
     // Calculate Next and Previous for all tokens. Note that we must overwrite
public:
 
   SmallVector<AnnotatedLine *, 0> Children;
 
   LineType Type;
   unsigned Level;
   size_t MatchingOpeningBlockLineIndex;
   size_t MatchingClosingBlockLineIndex;
   bool InPPDirective;
+  bool InMacroBody;
   bool MustBeDeclaration;
   bool MightBeFunctionDecl;
   bool IsMultiVariableDeclStmt;
 
   /// \c True if this line should be formatted, i.e. intersects directly or
   /// indirectly with one of the input ranges.
   bool Affected;
 

clang/lib/Format/TokenAnnotator.cpp

LineType parsePreprocessorDirective() {
     return Type;
   }
 
 public:
   LineType parseLine() {
     if (!CurrentToken)
       return LT_Invalid;
     NonTemplateLess.clear();
-    if (CurrentToken->is(tok::hash)) {
+    if (!Line.InMacroBody && CurrentToken->is(tok::hash)) {
       // We were not yet allowed to use C++17 optional when this was being
       // written. So we used LT_Invalid to mark that the line is not a
       // preprocessor directive.
       auto Type = parsePreprocessorDirective();
       if (Type != LT_Invalid)
         return Type;
     }
 
if ((Right.is(TT_BinaryOperator) && !Left.is(tok::l_paren)) ||
     return true;
   }
   if (Right.is(TT_TemplateOpener) && Left.is(tok::r_paren) &&
       Left.MatchingParen &&
       Left.MatchingParen->is(TT_OverloadedOperatorLParen)) {
     return false;
   }
   if (Right.is(tok::less) && Left.isNot(tok::l_paren) &&
-      Line.startsWith(tok::hash)) {
+      Line.Type == LT_ImportStatement) {
     return true;
   }
   if (Right.is(TT_TrailingUnaryOperator))
     return false;
   if (Left.is(TT_RegexLiteral))
     return false;
   return spaceRequiredBetween(Line, Left, Right);
 }

clang/lib/Format/UnwrappedLineParser.h

struct UnwrappedLine {
   /// The \c Tokens comprising this \c UnwrappedLine.
   std::list<UnwrappedLineNode> Tokens;
 
   /// The indent level of the \c UnwrappedLine.
   unsigned Level;
 
   /// Whether this \c UnwrappedLine is part of a preprocessor directive.
   bool InPPDirective;
+  /// Whether it is part of a macro body.
+  bool InMacroBody;
 
   bool MustBeDeclaration;
 
   /// \c True if this line should be indented by ContinuationIndent in
   /// addition to the normal indention level.
   bool IsContinuation = false;
 
   /// If this \c UnwrappedLine closes a block in a sequence of lines,
struct UnwrappedLineNode {
   UnwrappedLineNode() : Tok(nullptr) {}
   UnwrappedLineNode(FormatToken *Tok) : Tok(Tok) {}
 
   FormatToken *Tok;
   SmallVector<UnwrappedLine, 0> Children;
 };
 
 inline UnwrappedLine::UnwrappedLine()
-    : Level(0), InPPDirective(false), MustBeDeclaration(false),
-      MatchingOpeningBlockLineIndex(kInvalidIndex) {}
+    : Level(0), InPPDirective(false), InMacroBody(false),
+      MustBeDeclaration(false), MatchingOpeningBlockLineIndex(kInvalidIndex) {}
 
 } // end namespace format
 } // end namespace clang
 
 #endif

clang/lib/Format/UnwrappedLineParser.cpp

ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
       : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
         PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
         Token(nullptr), PreviousToken(nullptr) {
     FakeEOF.Tok.startToken();
     FakeEOF.Tok.setKind(tok::eof);
     TokenSource = this;
     Line.Level = 0;
     Line.InPPDirective = true;
+    // InMacroBody gets set after the `#define x` part.
   }
 
   ~ScopedMacroState() override {
     TokenSource = PreviousTokenSource;
     ResetToken = Token;
     Line.InPPDirective = false;
+    Line.InMacroBody = false;
     Line.Level = PreviousLineLevel;
   }
 
   FormatToken *getNextToken() override {
     // The \c UnwrappedLineParser guards against this by never calling
     // \c getNextToken() after it has encountered the first eof token.
     assert(!eof());
     PreviousToken = Token;
ScopedLineState(UnwrappedLineParser &Parser,
     if (SwitchToPreprocessorLines)
       Parser.CurrentLines = &Parser.PreprocessorDirectives;
     else if (!Parser.Line->Tokens.empty())
       Parser.CurrentLines = &Parser.Line->Tokens.back().Children;
     PreBlockLine = std::move(Parser.Line);
     Parser.Line = std::make_unique<UnwrappedLine>();
     Parser.Line->Level = PreBlockLine->Level;
     Parser.Line->InPPDirective = PreBlockLine->InPPDirective;
+    Parser.Line->InMacroBody = PreBlockLine->InMacroBody;
   }
 
   ~ScopedLineState() {
     if (!Parser.Line->Tokens.empty())
       Parser.addUnwrappedLine();
     assert(Parser.Line->Tokens.empty());
     Parser.Line = std::move(PreBlockLine);
     if (Parser.CurrentLines == &Parser.PreprocessorDirectives)
void UnwrappedLineParser::parsePPDefine() {
   if (FormatTok->Tok.getKind() == tok::l_paren &&
       !FormatTok->hasWhitespaceBefore()) {
     parseParens();
   }
   if (Style.IndentPPDirectives != FormatStyle::PPDIS_None)
     Line->Level += PPBranchLevel + 1;
   addUnwrappedLine();
   ++Line->Level;
+  Line->InMacroBody = true;
 
   // Errors during a preprocessor directive can only affect the layout of the
   // preprocessor directive, and thus we ignore them. An alternative approach
   // would be to use the same approach we use on the file level (no
   // re-indentation if there was a structural error) within the macro
   // definition.
   parseFile();
 }

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsTemplateParameters) {
   verifyFormat("A<A<A<int>>> a;");
   verifyFormat("A<A<A<int, 2>, 3>, 4> a;");
   verifyFormat("bool x = a < 1 || 2 > a;");
   verifyFormat("bool x = 5 < f<int>();");
   verifyFormat("bool x = f<int>() > 5;");
   verifyFormat("bool x = 5 < a<int>::x;");
   verifyFormat("bool x = a < 4 ? a > 2 : false;");
   verifyFormat("bool x = f() ? a < 2 : a > 2;");
 
   verifyGoogleFormat("A<A<int>> a;");
   verifyGoogleFormat("A<A<A<int>>> a;");
   verifyGoogleFormat("A<A<A<A<int>>>> a;");
   verifyGoogleFormat("A<A<int> > a;");
   verifyGoogleFormat("A<A<A<int> > > a;");
   verifyGoogleFormat("A<A<A<A<int> > > > a;");
   verifyGoogleFormat("A<::A<int>> a;");
   verifyGoogleFormat("A<::A> a;");
   verifyGoogleFormat("A< ::A> a;");
   verifyGoogleFormat("A< ::A<int> > a;");
TEST_F(FormatTest, UnderstandsTemplateParameters) {
 
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaa >> aaaaa);",
                getLLVMStyleWithColumns(60));
   verifyFormat("static_assert(is_convertible<A &&, B>::value, \"AAA\");");
   verifyFormat("Constructor(A... a) : a_(X<A>{std::forward<A>(a)}...) {}");
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
   verifyFormat("some_templated_type<decltype([](int i) { return i; })>");
+
+  verifyFormat("#define FOO(typeName, realClass)                           \\\n"
+               "  { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }",
+               getLLVMStyleWithColumns(60));
 }
 
 TEST_F(FormatTest, UnderstandsShiftOperators) {
   verifyFormat("if (i < x >> 1)");
   verifyFormat("while (i < x >> 1)");
   verifyFormat("for (unsigned i = 0; i < i; ++i, v = v >> 1)");
   verifyFormat("for (unsigned i = 0; i < x >> 1; ++i, v = v >> 1)");
   verifyFormat(

clang/unittests/Format/TokenAnnotatorTest.cpp

 TEST_F(TokenAnnotatorTest, UnderstandsVariableTemplates) {
   auto Tokens =
       annotate("template <typename T> "
                "inline bool var = is_integral_v<int> && is_signed_v<int>;");
   EXPECT_EQ(Tokens.size(), 20u) << Tokens;
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTemplatesInMacros) {
+  auto Tokens =
+      annotate("#define FOO(typeName) \\\n"
+               "  { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[17], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
 
   auto Tokens = annotate("FOO(1+2 )\n", Style);
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[0], tok::identifier, TT_UntouchableMacroFunc);