================
Comment at: lib/Sema/Sema.cpp:339
@@ -338,3 +338,3 @@
ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
CastKind Kind, ExprValueKind VK,
const CXXCastPath *BasePath,
----------------
rsmith wrote:
> sfantao wrote:
> > rsmith wrote:
> > > Why is `Kind` == `CK_NoOp` being passed in here for an address space
> > > cast? That seems like it might be a bug in the caller. Do you know which
> > > calls to `ImpCastExprToType` result in this happening?
> > This is the stack I have when I reach the point where the address cast is
> > performed.
> >
> > ```
> > clang::Sema::ImpCastExprToType() at Sema.cpp:370 0x141629b8
> > clang::Sema::PerformImplicitConversion() at SemaExprCXX.cpp:3,447
> > 0x1456981c
> > clang::Sema::PerformImplicitConversion() at SemaExprCXX.cpp:2,964
> > 0x14567124
> > clang::InitializationSequence::Perform() at SemaInit.cpp:6,315 0x1465e444
> > TryStaticImplicitCast() at SemaCast.cpp:1,522 0x141bb004
> > TryStaticCast() at SemaCast.cpp:988 0x141b9e98
> > (anonymous namespace)::CastOperation::CheckCXXCStyleCast at
> > SemaCast.cpp:2,130 0x141b4248
> > clang::Sema::BuildCStyleCastExpr() at SemaCast.cpp:2,455 0x141b3b60
> > clang::Sema::ActOnCastExpr() at SemaExpr.cpp:5,545 0x144533a8
> > ```
> >
> > I see CK_NoOp is hardcoded in the caller:
> >
> > ```
> > case ICK_Qualification: {
> > // The qualification keeps the category of the inner expression, unless
> > the
> > // target type isn't a reference.
> > ExprValueKind VK = ToType->isReferenceType() ?
> > From->getValueKind() : VK_RValue;
> > From = ImpCastExprToType(From, ToType.getNonLValueExprType(Context),
> > CK_NoOp, VK, /*BasePath=*/nullptr, CCK).get();
> > ```
> > Should I fix something in here?
> >
> If that's the only code path through which we get this wrong, my preference
> would be to move the fix into `PerformImplicitConversion`, and add an assert
> to `ImpCastExprToType` that we're not changing the address space with a
> `CK_NoOp` cast.
I was comparing what was going on for C and C++ and I see that for C the
address cast is identified right away in CheckCStyleCast in:
```
Kind = Self.PrepareScalarCast(SrcExpr, DestType);
```
Therefore I thought it would make sense to do the same for C++ in
CheckCXXCStyleCast. I added the assertion in the place you suggested anyway.
I understand this is not exactly what you requested but it looks as a more
clean fix. If you do not agree, let me know and I can switch back to what you
were originally proposing.
================
Comment at: test/Sema/address-space-cast.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -emit-llvm -o - -x c %s | FileCheck -check-prefix=CHECK %s
+// RUN: %clang_cc1 -emit-llvm -o - -x c++ %s | FileCheck
-check-prefix=CHECK-CXX %s
----------------
rsmith wrote:
> Please move this test to test/CodeGen (I know, you're testing a Sema change,
> but the primary impact of your change is to make us emit the right IR, and
> that's what you're testing.)
Done!
http://reviews.llvm.org/D7606
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
