Hi Hal,
Thanks for reviewing the patch! The answer to your remarks is bellow.
Regards,
Samuel
================
Comment at: lib/Sema/Sema.cpp:351
@@ +350,3 @@
+ // If we are casting pointers, we need to make sure we deal with address
+ // spaces properly
+ if (Kind == CK_NoOp && ExprTy->isPointerType() && TypeTy->isPointerType() &&
----------------
hfinkel wrote:
> Add a period to the end of the comment.
Will do!
================
Comment at: lib/Sema/Sema.cpp:352
@@ +351,3 @@
+ // spaces properly
+ if (Kind == CK_NoOp && ExprTy->isPointerType() && TypeTy->isPointerType() &&
+ ExprTy->getPointeeType().getAddressSpace() !=
----------------
hfinkel wrote:
> Does this apply to CK_LValueBitCast too?
>
This seems to be specific to the AddressCast. For these kind of casts we were
getting for C++:
`-CStyleCastExpr 0x1000b254df8 <col:6, col:26>
'__attribute__((address_space(1))) char *' <NoOp>
`-ImplicitCastExpr 0x1000b254de0 <col:26>
'__attribute__((address_space(1))) char *' <NoOp>
`-ImplicitCastExpr 0x1000b254dc8 <col:26> 'const char *'
<ArrayToPointerDecay>
`-StringLiteral 0x1000b254d58 <col:26> 'const char [2]' lvalue "a"
That kind of implicit cast seems to be generated only for address space casts.
The patch just fixes the Noop, so we get:
`-CStyleCastExpr 0x10010154df8 <col:6, col:26>
'__attribute__((address_space(1))) char *' <NoOp>
`-ImplicitCastExpr 0x10010154de0 <col:26>
'__attribute__((address_space(1))) char *' <AddressSpaceConversion>
`-ImplicitCastExpr 0x10010154dc8 <col:26> 'const char *'
<ArrayToPointerDecay>
`-StringLiteral 0x10010154d58 <col:26> 'const char [2]' lvalue "a"
I don't fully understand why a pair of CStyleCastExpr+ImplicitCastExpr is being
used instead of a single CStyleCastExpr. I see however, that the pair version
seem to fit the Sema implementation better.
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
