AaronBallman wrote:
What I don't want to lose from this patch are the changes to places like
`InitializationStyle getInitializationStyle() const` and
`CXXNewExpr::CXXNewExpr` where the old code was unclear and the new code is
significantly more clear. We should not be performing math on the enumerator
values to encode in-band extra information. What I see being clarified by this
patch is:
```
struct S { int x; };
auto *s0 = new int; // None, scalar types have no notional constructor
initialization
auto *s1 = new S; // Implicit, class type has a notional constructor call
auto *s2 = new S(0); // Call (ParenList is a much better name)
auto *s3 = new S{0}; // List (BraceList is a much better name)
```
> In both cases, the news would report the initialization as Implicit, where
> actually no initialization is performed. There is no call to the constructor
> inserted.
There is an implicit constructor call but it's a noop because the type is
trivial, so I think `Implicit` is what I would expect given the comment `///
New-expression has no written initializer, but has an implicit one.`
https://godbolt.org/z/353G45vnc
That said, I can see why it may be confusing to say there's an implicit
initialization for something that is a noop which performs no initialization
and we have an enumerator for "no initialization". I think
@tomasz-kaminski-sonarsource would like for the extra enumerator to be removed,
but I don't think that's possible to do without also losing the benefits of the
changes. But perhaps we could rename `Implicit` and `None` to something more
clear?
> In short, as the downstream that uses AST for writing rules, we will need to
> update all the uses of NoInit to also check for Implicit, without getting any
> value from the distinction.
The C++ APIs have no stability guarantees and not every change will be to the
benefit of all downstreams; I see the changes in this PR as being an
improvement over the status quo because they clarify code in our code base and
I'm not seeing the same level of confusion you are in your downstream. (That
said, I'm also totally happy to rename enumeration members to pick more
descriptive names.)
https://github.com/llvm/llvm-project/pull/71417
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
