[PATCH] Fix adress cast for C++ in SEMA

Richard Smith richard at metafoo.co.uk
Mon Jun 8 11:17:58 PDT 2015


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

================
Comment at: lib/Sema/Sema.cpp:364-370
@@ -363,9 +363,9 @@
 
   if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
     if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) {
       ImpCast->setType(Ty);
       ImpCast->setValueKind(VK);
       return E;
     }
   }
 
----------------
If we (somehow) hit this case and have an inner cast of kind `CK_NoOp`, we'll fail to create a `CK_AddressSpaceConversion` cast. The added code should be above this block.

================
Comment at: test/Sema/address-space-cast.c:1-2
@@ +1,3 @@
+// RUN: %clang_cc1 %s -ast-dump -x c %s | FileCheck -check-prefix=CHKC %s
+// RUN: %clang_cc1 %s -ast-dump -x c++ %s | FileCheck -check-prefix=CHKCXX %s
+
----------------
Please use the conventional check prefix of `CHECK` for the first of these, and `CHECK-CXX` for the second.

Also, it would be preferable to check the generated IR here rather than an AST dump -- the AST dump format is less stable than the textual IR format, and it's much more important that we get the IR right than the dump :-)

http://reviews.llvm.org/D7606

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






More information about the cfe-commits mailing list