Fznamznon added inline comments.
================
Comment at: clang/lib/Sema/SemaType.cpp:8824
+ if (auto *Cast = dyn_cast<CXXStaticCastExpr>(E)) {
+ if (auto *SubInit = dyn_cast<CXXParenListInitExpr>(Cast->getSubExpr())) {
+ const Type *InnerType = SubInit->getType().getTypePtr();
----------------
erichkeane wrote:
> Fznamznon wrote:
> > erichkeane wrote:
> > > Fznamznon wrote:
> > > > erichkeane wrote:
> > > > > I am not really sure this is the right way about this. You're
> > > > > supposed to be testing `T`, but this looks like it is checking the
> > > > > type of `E`, isn't it? I think you just need to check
> > > > > `Cast->getType()`.
> > > > For the case I'm trying to fix, `T` is array of unknown bound, it is
> > > > already checked before calling `completeExprArrayBound`. Right now when
> > > > you write something like
> > > > ```
> > > > int (&&arr1)[1] = static_cast<int[]>(42);
> > > > ```
> > > > Clang actually is able to realize that parenthesized initialization is
> > > > made, so it actually generates `CXXParenListInitExpr` that has type
> > > > int[1]. Here I'm just paranoidally double-checking that is the case
> > > > before switching the type. Maybe I don't need this double-check at all
> > > > then?
> > > >
> > > That makes me wonder if this is the correct place for this? Should when
> > > we generate this type when we do the `CXXParenListInitExpr` fixup?
> > >
> > > Either way, I think making this depend on that behavior (which would
> > > possibly be fragile), we should just do it based on the type passed ot
> > > the `static_cast`.
> > >
> > > Another question: is this something that needs to be done for other cast
> > > types? Does similar behavior exist for the other casts? Should it also
> > > happen with 'clobber' casts (C-style) that are effectively static?
> > > That makes me wonder if this is the correct place for this? Should when
> > > we generate this type when we do the CXXParenListInitExpr fixup?
> >
> > The function called `completeExprArrayBound` seemed like an appropriate
> > place for me.
> > Could you please elaborate what you mean under `CXXParenListInitExpr
> > fixup`?
> >
> > > is this something that needs to be done for other cast types?
> >
> > I'm not sure.
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1975r0.html seems
> > to be extending only static_casts. And this was the original purpose of
> > this bugfix.
> > gcc and msvc don't agree on that matter https://godbolt.org/z/1M3ahhsYr.
> >>Could you please elaborate what you mean under CXXParenListInitExpr fixup?
>
> You mentioned that at a different place we set the type of the
> `CXXParenListInitExpr` to be the 1-arity array. I was saying that if there
> is logic THERE based on whether its an incomplete array type it would be for
> the same purpose, and perhaps be the correct place to do this.
>
> >>I'm not sure.
> >>https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1975r0.html seems
> >>to be extending only static_casts.
>
> Yep, thats what I was hoping to see, thanks.
>
> >>And this was the original purpose of this bugfix.
>
> I was concerned that we increasing the inconsistency by only fixing 1 of the
> types of casts. It might seem like 'feature creep', but it is very important
> that we make sure a patch is reasonably complete, so that it doesn't make the
> compiler less consistent.
>
> I was saying that if there is logic THERE based on whether its an incomplete
> array type it would be for the same purpose, and perhaps be the correct place
> to do this.
Well, it seems to be happening inside of a `TryStaticImplicitCast` before the
`CXXStaticCastExpr` created. There is initialization sequence performed, and
the source expression comes out of as an array of 1 element. It is not very
convenient to switch the type there since there is no cast expression itself
yet and the destination type is passed as a copy through several functions.
Will it be ok to check and switch the type of resulting static_cast expression
just before/after it is created?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152003/new/
https://reviews.llvm.org/D152003
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
