t-tye added inline comments.
================ Comment at: include/clang/AST/Type.h:339-340 + auto Addr = getAddressSpace(); + if (Addr == 0) + return 0; + return Addr - LangAS::target_first; ---------------- Anastasia wrote: > yaxunl wrote: > > t-tye wrote: > > > Since you mention this is only used for > > > `__attribute__((address_space(n)))`, why is this check for 0 needed? > > > > > > If it is needed then to match other places should it simply be: > > > > > > ``` > > > if (Addr) > > > return Addr - LangAS::target_first; > > > return 0; > > > ``` > > It is for `__attribute__((address_space(n)))` and the default addr space 0. > > > > For the default addr space 0, we want to print 0 instead of > > `-LangAS::target_first`. > > > > I will make the change for matching other places. > Could we use LangAS::Count instead? I do not think the address space 0 should be returned as 0 as then it is impossible to distinguish between a type that has no address space attribute, and one that has an explicit address space attribute with the value 0. But that seems to be a bug in the original code so I would suggest leaving this for now and fixing it as a separate patch. The diagnostic message should really be checking if an address space attribute was present (by checking for 0), and changing the working of the message accordingly. Suggest add a TODO here to mention this which can be fixed in a later patch. https://reviews.llvm.org/D31404 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
