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