================
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,
----------------
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.

================
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
----------------
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.)

http://reviews.llvm.org/D7606

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to