[clang-format] Allow ternary in all templates

[rymiel]

Currently, question mark and colon tokens are not allowed between angle brackets, as a template argument, if we are in an expression context.

However, expressions can still allowed in non-expression contexts, leading to inconsistent formatting.

Removing this check entirely fixes this issue, and, surprisingly, breaks no tests.

Fixes #81385

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Emilia Kond (rymiel)

Changes

Currently, question mark and colon tokens are not allowed between angle brackets, as a template argument, if we are in an expression context.

However, expressions can still allowed in non-expression contexts, leading to inconsistent formatting.

Removing this check entirely fixes this issue, and, surprisingly, breaks no tests.

Fixes #81385

---

Full diff: https://github.com/llvm/llvm-project/pull/96801.diff

2 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+1-8)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+18)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 89e134144d433..03082cd2742c8 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -176,10 +176,6 @@ class AnnotatingParser {
     Left->ParentBracket = Contexts.back().ContextKind;
     ScopedContextCreator ContextCreator(*this, tok::less, 12);
 
-    // If this angle is in the context of an expression, we need to be more
-    // hesitant to detect it as opening template parameters.
-    bool InExprContext = Contexts.back().IsExpression;
-
     Contexts.back().IsExpression = false;
     // If there's a template keyword before the opening angle bracket, this is a
     // template parameter, not an argument.
@@ -231,11 +227,8 @@ class AnnotatingParser {
         next();
         continue;
       }
-      if (CurrentToken->isOneOf(tok::r_paren, tok::r_square, tok::r_brace) ||
-          (CurrentToken->isOneOf(tok::colon, tok::question) && InExprContext &&
-           !Style.isCSharp() && !Style.isProto())) {
+      if (CurrentToken->isOneOf(tok::r_paren, tok::r_square, tok::r_brace))
         return false;
-      }
       // If a && or || is found and interpreted as a binary operator, this set
       // of angles is likely part of something like "a < b && c > d". If the
       // angles are inside an expression, the ||/&& might also be a binary
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index d3b310fe59527..5d83d8a0c4429 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -567,6 +567,24 @@ TEST_F(TokenAnnotatorTest, UnderstandsGreaterAfterTemplateCloser) {
   EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTernaryInTemplate) {
+  // IsExpression = false
+  auto Tokens = annotate("foo<true ? 1 : 2>();");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[3], tok::question, TT_ConditionalExpr);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_ConditionalExpr);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser);
+
+  // IsExpression = true
+  Tokens = annotate("return foo<true ? 1 : 2>();");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[4], tok::question, TT_ConditionalExpr);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ConditionalExpr);
+  EXPECT_TOKEN(Tokens[8], tok::greater, TT_TemplateCloser);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
   auto Tokens = annotate("return a < b && c > d;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;

[rymiel]

I'm not even sure if this change is a good idea! I think it's very likely it will lead to regressions, for we don't have tests for such cases yet, it seems

[HazardyKnusperkeks]

I'm not even sure if this change is a good idea! I think it's very likely it will lead to regressions, for we don't have tests for such cases yet, it seems

I'm sure you will look after them, so I'd say let them come. ;)