================
@@ -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:
> This patch IMO is a narrowed down version of the other patch
> https://github.com/llvm/llvm-project/pull/100980, which fixed a real
> regression https://github.com/llvm/llvm-project/issues/100300 caused by yet
> another patch https://github.com/llvm/llvm-project/pull/96801, which in turn
> was an attempt at fixing a real bug
> https://github.com/llvm/llvm-project/issues/81385.
I totally get that. The point I am trying to make is, these sequences of
patches, fixed a real regression, at the cost of introducing another one.
> In your
> https://github.com/llvm/llvm-project/pull/108671#discussion_r1762725421
> above, it doesn't seem that your C++ example return FixedInt<N | M>(foo); is
> from an existing codebase.
Sorry if I gave that impression, but I was delibaretly trying to make it more
concrete as you were asking for more specifics, here's some real world
examples:
https://github.com/search?q=lang%3Ac%2B%2B+%2FFixedInt%3C.*%5C%7C.*%3E%2F&type=code
> The fact that it happened to get formatted that way before doesn't
> necessarily mean that we should always format another similar pattern e.g.
> return Foo < A || B > (C ^ D); the same way, especially if the latter is much
> more likely.
No I totally get that. That's how clang-format evolves over time. But as I
explained above, my understanding is this is done in a way that it doesn't
regress behavior for already formatted code. As otherwise it becomes really
troublesome to adopt new versions of clang-format.
Hence the change in https://github.com/llvm/llvm-project/pull/100980, didn't
simply improve formatting for some mishandled pattern, it also regressed
handling of some existing pattern.
I am totally OK if your verdict here is this is a cost we're willing to pay, in
the end you're the maintainer. I am just trying to make sure this doesn't go
unnoticed and someone is making a call deliberately. Even if it isn't aligned
with the outcome that I might like.
> I've added an option in https://github.com/llvm/llvm-project/pull/109916 to
> help disambiguate the < in the C++ examples above.
It might be a nice addition, but it doesn't really change the fact that
clang-format-18 is WAI for such code patterns, and starting with
clang-format-19 this has regressed and users need to change their
configuration. Moreover it also means that patch now needs to be backported (or
clang-format-19 is just broken for certain codebases).
https://github.com/llvm/llvm-project/pull/108671
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
