yaxunl marked an inline comment as done.
yaxunl added inline comments.

================
Comment at: include/clang/AST/ASTContext.h:2319
     return AddrSpaceMapMangling || 
-           AS < LangAS::Offset || 
-           AS >= LangAS::Offset + LangAS::Count;
+           AS > LangAS::target_first;
   }
----------------
t-tye wrote:
> Should this be >= since it wants to return for all target address spaces, and 
> target_first is the first one?
Will do.


================
Comment at: include/clang/AST/Type.h:336-342
+  /// Get the address space value used in diagnostics.
+  unsigned getAddressSpacePrintValue() const {
+    unsigned AS = getAddressSpace();
+    if (AS >= LangAS::target_first)
+      return AS - LangAS::target_first;
+    return AS;
+  }
----------------
t-tye wrote:
> Is this the right thing to do? This is making the CLANG named address spaces 
> have the same numbers as the target-specific address space numbers which 
> would seem rather confusing.
> 
> What do the diagnostics want to display? For example, they could display the 
> address space as a human readable form such as "Default", "OpenCL-global", 
> CUDA-device", "target-specific(5)", etc. To do that this function could take 
> an iostream and a LangAS value and use << to write to the iostream.
This function is used by diagnostics for address spaces specified by 
`__attribute__((address_space(n)))`. There are several lit tests for that, e.g. 

https://github.com/llvm-mirror/clang/blob/master/test/SemaCXX/address-space-newdelete.cpp
https://github.com/llvm-mirror/clang/blob/master/test/SemaCXX/address-space-references.cpp

It is desirable to use the same value as specified by the attribute, otherwise 
it may confuse the user.


https://reviews.llvm.org/D31404



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to