cor3ntin added inline comments.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1520-1525
+ if (IsConsteval) {
+ if (!isa_and_nonnull<class CompoundStmt>(ThenStmt.get())) {
+ Diag(ConstevalLoc, diag::err_expected_after) << "consteval"
+ << "{";
+ return StmtError();
+ }
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > Oh. I see where the diagnostic is happening. I think it's unclean to
> > > > parse a statement and then diagnose it later as being the wrong kind of
> > > > statement -- why not parse the correct kind of statement initially?
> > > I think the code reads better that way - we need to parse any kind of
> > > statement in other cases. In any case the work is about the same
> > Hmmm, this makes me a bit uncomfortable, but I'm not convinced it's a
> > problem either. Calling `ParseCompoundStmt()` would require checking the
> > token first to avoid an assert, so I can understand better wanting to avoid
> > the messier code that comes from that. However, calling the correct parsing
> > method also means that we don't attempt to parse something we shouldn't
> > parse only to reject it much later; I'm a bit concerned that we'll get some
> > bad error recovery when parsing the more general thing. However, my
> > attempts to come up with test cases where this matters are so far falling
> > pretty flat. One test that would help (and should pass still) is:
> > ```
> > if consteval ({1;}); // Do we correctly reject the statement expression?
> > ```
> Please hold off on changes here for the moment, as I think I may have
> identified a DR. Consider code like:
> ```
> if consteval [[]] {
> }
> ```
> According to the C++ grammar, this construct should not be accepted. However,
> that's inconsistent with every other form of if statement and seems like a
> defect to me. I've asked on the Core reflectors to see.
As discussed offline I'm letting this as is so that we can parse attributes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110482/new/
https://reviews.llvm.org/D110482
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
