Anastasia added inline comments.

================
Comment at: lib/AST/TypePrinter.cpp:1323
     OS << "address_space(";
-    OS << T->getEquivalentType().getAddressSpace();
+    OS << T->getEquivalentType()
+              .getQualifiers()
----------------
arichardson wrote:
> Anastasia wrote:
> > arichardson wrote:
> > > Anastasia wrote:
> > > > Why do we need this change?
> > > `__attribute__((address_space(n)))` is a target address space and not a 
> > > language address space like `LangAS::opencl_generic`. Isn't 
> > > `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for this 
> > > use case?
> > Yes, I think there are some adjustment we do in this method to get the 
> > original source value to be printed corerctly. Does this mean we have no 
> > tests that caught this issue?
> Seems like it, all tests pass both with and without this patch.
Strange considering that we have this attribute printed in some error messages 
of some Sema tests. If I compile this code without your patch:
 
```
typedef int __attribute__((address_space(1))) int_1;
typedef int __attribute__((address_space(2))) int_2;

void f0(int_1 &); 
void f0(const int_1 &);

void test_f0() {
  int i;
  static int_2 i2;
  f0(i);
  f0(i2);
}
```

I get the address spaces printed correctly inside the type:
  note: candidate function not viable: 1st argument ('int_2' (aka 
'__attribute__((address_space(2))) int')) is in address space 2, but parameter 
must be in address space 1

Perhaps @yaxunl could comment further on whether this change is needed.


https://reviews.llvm.org/D38816



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

Reply via email to