[clang] [clang-format] Fix a bug in annotating angles containing FatArrow (PR #108671)

[kadir çetinkaya via cfe-commits]

================
@@ -243,14 +244,16 @@ class AnnotatingParser {
       // operator that was misinterpreted because we are parsing template
       // parameters.
       // FIXME: This is getting out of hand, write a decent parser.
-      if (InExpr && !Line.startsWith(tok::kw_template) &&
+      if (InExpr && !SeenFatArrow && !Line.startsWith(tok::kw_template) &&
           Prev.is(TT_BinaryOperator)) {
         const auto Precedence = Prev.getPrecedence();
         if (Precedence > prec::Conditional && Precedence < prec::Relational)
           return false;
       }
       if (Prev.isOneOf(tok::question, tok::colon) && !Style.isProto())
         SeenTernaryOperator = true;
+      else if (Prev.is(TT_FatArrow))
----------------
kadircet wrote:

> The GitHub examples you listed here have zero arguments and thus are unambiguous to clang-format. (See https://github.com/llvm/llvm-project/pull/110408.) As I mentioned or implied above, a pair of empty parentheses after the > would help. We can add another patch (if needed) for multiple arguments such as return FixedInt<N | M>(foo, bar); , which is also unambiguous.

Sorry I wasn't paying attention to such details, as I was rather trying to explain the current logic and the previous one has different characteristics around `<>`. Trying to fix it for certain special cases won't change that fact and we'll let regressions around cases that we haven't discovered to slip.

Hence, I totally agree with your approach on only changing behavior in special cases, rather than changing overall behavior. The point that's puzzling me is, why don't we treat 834ac2e205dd8e492d6084a7952e68e19a1f54db and 73c961a3345c697f40e2148318f34f5f347701c1 similarly. If they were to just change handling of ternary expressions, but they changed behavior for expressions that don't have `?` in them, it sounds like an unintended change.

> However, the initial version of your C++ example return FixedInt<N | M>(foo); has exactly one argument and can be also interpreted as a non-template expression similar to Foo < A || B > (C ^ D);.

I completely agree with that, and that's the regression I am trying to express as well. Previously clang-format was erring towards recognizing these as template arguments, now it's erring towards recognizing them as comparisons. I am trying to figure out if we share the same understanding on this topic. If we both agree on this one, but you folks believe that's a regression we're willing to eat, I am totally fine with that. In that case I think https://github.com/llvm/llvm-project/pull/109916 LGTM!

https://github.com/llvm/llvm-project/pull/108671