kimgr added inline comments.
================
Comment at: clang/lib/AST/Expr.cpp:1949-1950
if (E->getCastKind() == CK_ConstructorConversion)
return cast<CXXConstructExpr>(SubExpr)->getConstructor();
----------------
kimgr wrote:
> davrec wrote:
> > I think the errors prove we should fall back to the most conservative
> > possible fix: remove all the other changes and just change these 2 lines to:
> > ```
> > if (E->getCastKind() == CK_ConstructorConversion) {
> > if (auto *CE = dyn_cast<ConstantExpr>(SubExpr)
> > SubExpr = CE->getSubExpr();
> > return cast<CXXConstructExpr>(SubExpr)->getConstructor();
> > }
> > ```
> > I'm can't quite remember but I believe that would solve the bug as narrowly
> > as possible. @kimgr can you give it a try if and see if it avoids the
> > errors?
> > (If it doesn't, I'm stumped...)
> Yes, it does. I needed to add the same `ConstantExpr` skipping to the
> user-defined conversion handling below, but once those two were in place the
> new unittests succeed and the existing lit tests also.
>
> It does feel a little awkward, though... I wish I had a clearer mental model
> of how the implicit-skipping is supposed to work. My intuition is telling me
> `skipImplicitTemporary` should probably disappear and we should use the
> `IgnoreExpr.h` facilities directly instead, but it's all a little fuzzy to me
> at this point.
>
> I'll run the full `check-clang` suite overnight with this alternative patch,
> I have no reason to think there will be problems, but I'll make sure in the
> morning.
>
> Thanks!
I can confirm that full `check-clang` also works with the narrower fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117391/new/
https://reviews.llvm.org/D117391
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
