Anastasia marked 4 inline comments as done.
Anastasia added inline comments.
================
Comment at: lib/Sema/SemaCast.cpp:279
+ case tok::kw_addrspace_cast:
+ if (!TypeDependent) {
+ Op.CheckAddrspaceCast();
----------------
mantognini wrote:
> Still learning here, so could you/someone tell me if I understood things
> correctly? :)
>
> The type is "dependent" when templates are involved, right? And, here, when
> we don't know all the concrete types we defer the checking to a later
> compiler phase? And that later compiler phase would be implemented by
> "TreeTransform", right?
Precisely!
================
Comment at: lib/Sema/SemaCast.cpp:285
+ return Op.complete(CXXAddrspaceCastExpr::Create(Context, Op.ResultType,
+ Op.ValueKind, Op.SrcExpr.get(),
+ DestTInfo,
----------------
mantognini wrote:
> The formatting looks a bit funny here.
I agree, I just matched the style of the old code to keep it coherent. Although
perhaps I should rather adhere to the current style?
================
Comment at: lib/Sema/SemaCast.cpp:2319
unsigned &msg) {
if (!Self.getLangOpts().OpenCL)
// FIXME: As compiler doesn't have any information about overlapping addr
----------------
mantognini wrote:
> Just to make sure I understand things correctly: `OpenCL` is true when
> dealing with C or C++ mode for OpenCL, right?
Precisely!
================
Comment at: lib/Sema/SemaCast.cpp:2338
auto DestPointeeType = DestPtrType->getPointeeType();
if (SrcPointeeType.getAddressSpace() == DestPointeeType.getAddressSpace())
return TC_NotApplicable;
----------------
mantognini wrote:
> Wouldn't this limit usage of the cast unnecessarily? I'm thinking this could
> be transformed to a NOP, which could be beneficial to make (template) code
> simpler to write.
I am not sure what you mean. I have added the test for templates and it caught
a bug in lib/AST/Expr.cpp with assert condition.
However, now that I think about this more, I believe we should allow compiling
this?
```
__private int* i;
addrspace_cast<private int*>(i);
```
Currently it outputs an error.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60193/new/
https://reviews.llvm.org/D60193
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
