arichardson 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:
> > > > 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.
> My guess is that it doesn't go through that switch statement but rather
> through `Qualifiers::print()`. I'll try adding a llvm_unreachable() to see if
> there are any tests that go down this path.
I just ran the clang tests with an llvm_unreachable() here and none of them
failed. So it seems like we don't have anything testing this code path.
https://reviews.llvm.org/D38816
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
