================
@@ -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))
----------------
owenca wrote:
> > In your [#108671
> > (comment)](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 GitHub examples you listed here have zero arguments and thus are
unambiguous to clang-format. (See #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.
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);`.
> > 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
> #100980, didn't simply improve formatting for some mishandled pattern, it
> also regressed handling of some existing pattern.
It goes without saying that we should strive to keep clang-format regression
free. Nevertheless, not all behavior changes are regressions. Also, some past
regressions can't simply be reverted as it would cause new regressions. The
change in #100980 is an example of the latter.
> 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.
With #109916 we can cover both `return Foo < A || B > (C ^ D);` and `return
Foo<A | B>(C ^ D);`. It may require changing the existing configuration for the
latter case, but the alternative (i.e. not supporting the former case or having
a `NonTemplateNames` option instead) would be worse IMO.
This patch is to fix the TypeScript issue as initially reported in #108536. As
#109925 also fixes that issue (probably incidentally), I'll close this one if
you think special casing the fat arrow for JavaScript/TypeScript is unnecessary.
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
