https://github.com/Endilll created
https://github.com/llvm/llvm-project/pull/71417
This patch converts CXXNewExpr::InitializationStyle into a scoped enum at
namespace scope. It also affirms the status quo by adding a new enumerator to
represent implicit initializer.
This is a re-land of https
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-modules
Author: Vlad Serebrennikov (Endilll)
Changes
This patch converts CXXNewExpr::InitializationStyle into a scoped enum at
namespace scope. It also affirms the status quo by adding a new enumerator to
represent impl
tomasz-kaminski-sonarsource wrote:
> I had an offline discussion with @Endilll during my morning office hours
> today, and our current plan is:
>
> 1. Remove `Implicit` from the enumeration, rename `Call` and `List` to
> `ParenList` and `BraceList`, respectively
> 2. Add a new bit to the bit-f
Endilll wrote:
> I'd qualify this as a regression, by looking at that the commit was supposed
> to be an NFC.
Could you please confirm @Endilll?
I'll leave to @AaronBallman to decide whether this is a functional change, but
I can confirm that patch is working as intended, because there is an i
cor3ntin wrote:
@steakhal Thanks for raising this.
I agree this stretches the definition of NFC commit.
But it was dully reviewed and approved
https://github.com/llvm/llvm-project/pull/71322
We usually do not make any guarantees as the stability of the C++ interfaces,
so as this test did no
Endilll wrote:
> I agree this stretches the definition of NFC commit.
But it was dully reviewed and approved
https://github.com/llvm/llvm-project/pull/71322
I agree with this assessment. I think it really started as regular NFC, but
then me and Aaron realized that we can get rid of some ugly c
steakhal wrote:
> Please let us know if this change is disruptive to you though, thanks!
I'm not really well versed about c++ initialization, so I asked my collage
@tomasz-kaminski-sonarsource, to double-check how `CXXNewExpr` initialization
is done per standard.
I'll try to summarize what we
Endilll wrote:
> The enum we had in the past described the syntax of the new expression.
Even if it was the case at some point, I'm not sure it held when I created the
PR, which eliminated this kind of nasty mapping, encoding how this enum was
actually used:
```cpp
CXXNewExprBits.StoredInitia
AaronBallman wrote:
> > The enum we had in the past described the syntax of the new expression.
>
> Even if it was the case at some point, I'm not sure it held when I created
> the PR, which eliminated this kind of nasty mapping, encoding how this enum
> was actually used:
>
> ```c++
> CXXNe
tomasz-kaminski-sonarsource wrote:
> > > The enum we had in the past described the syntax of the new expression.
> >
> >
> > Even if it was the case at some point, I'm not sure it held when I created
> > the PR, which eliminated this kind of nasty mapping, encoding how this enum
> > was actua
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 en
tomasz-kaminski-sonarsource 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 pe
tomasz-kaminski-sonarsource wrote:
Reworked response to provide constructive feedback.
> 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 cod
13 matches
Mail list logo
