LegalizeAdulthood added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:204
+ return true;
+}
+
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > aaron.ballman wrote:
> > > > Comma operator?
> > > Remember that the use case here is identifying expressions that are
> > > initializers for an enum. If you were doing a code review and you saw
> > > this:
> > > ```
> > > enum {
> > > FOO = (2, 3)
> > > };
> > > ```
> > > Would you be OK with that? I wouldn't. Clang even warns about it:
> > > https://godbolt.org/z/Y641cb8Y9
> > >
> > > Therefore I deliberately left comma operator out of the grammar.
> > This is another case where I think you're predicting that users won't be
> > using the full expressivity of the language and we'll get bug reports
> > later. Again, in insolation, I tend to agree that I wouldn't be happy
> > seeing that code. However, users write some very creative code and there's
> > no technical reason why we can't or shouldn't handle comma operators.
> "Don't let the perfect be the enemy of the good."
>
> My inclination is to simply explicitly state that comma operator is not
> recognized in the documentation. It's already implicit by it's absence from
> the list of recognized operators.
>
> Again, the worst that happens is that your macro isn't converted.
>
> I'm open to being convinced that it's important, but you haven't convinced me
> yet `:)`
It wasn't much extra work/code to add comma operator support so I've done that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124500/new/
https://reviews.llvm.org/D124500
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
