cor3ntin added inline comments.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+ if (IsConsteval && NotLocation.isValid()) {
+ if (ElseStmt.isUnset())
+ ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > So this is interesting. I'm not sure how I feel about having the AST not
> > represent the textual representation like this. I'm interested what others
> > have to say.
> >
> > My understanding is that this makes:
> >
> > `if consteval { thenstmt; } else { elsestmt;`
> > be represented as:
> > `IfStmt isConsteval, with getThen()== thenstmt`
> >
> > however
> > `if not consteval { thenstmt; } else { elsestmt;}`
> > be represented as:
> > `IfStmt isConsteval, with getThen()== elsestmt`
> >
> > IMO, this feels too clever.
> >
> > I think I'd prefer that the IfStmt know whether it is a 'not consteval' and
> > select the right one that way.
> I haven't had the chance to go over this review yet, but this comment caught
> my eye in my inbox so I figured I'd respond quickly.
>
> The current approach is definitely clever, but I don't think it's the right
> way to tackle this. Generally, the AST needs to retain enough fidelity to be
> pretty printed back out to the original source, which wouldn't work here. But
> also, this makes it harder to write AST matchers over the construct because
> it's not really matching what the user wrote in source (we sometimes get
> around this by having a "semantic" and "syntactic" AST representation, but
> that seems like overkill here).
This is exactly the wording though.
> An if statement of the form if ! consteval compound-statement is not itself a
> consteval if statement, but is equivalent to the consteval if statement `if
> consteval { } else compound-statement`
An if statement of the form `if ! consteval compound-statement1 else
statement2` is not itself a consteval if statement, but is equivalent to the
consteval if statement
Doing something else would require storing the not position, and I don't think
we gain any functionality?
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
