rsmith added inline comments.
================ Comment at: include/clang/AST/ASTContext.h:1428 + QualType equivalentType, + IdentifierInfo *AddressSpaceMacroII = nullptr); ---------------- There is no reason to make this specific to address space attributes. Please keep it general. ================ Comment at: lib/AST/TypePrinter.cpp:1370 + + // Remove the underlying address space so it won't be printed. + SplitQualType SplitTy = T->getModifiedType().split(); ---------------- This is unnecessary; just print the modified type here. (The modified type by definition does not have the attribute applied to it.) ================ Comment at: lib/Sema/SemaType.cpp:5817-5822 + IdentifierInfo *AddressSpaceMacroII = nullptr; + auto FoundMacro = S.PP.AddressSpaceMacros.find( + S.SourceMgr.getSpellingLoc(Attr.getLoc())); + if (FoundMacro != S.PP.AddressSpaceMacros.end()) + AddressSpaceMacroII = FoundMacro->second; + ---------------- This is not the right way to find the macro. What you should do is this: look at the source range of the attribute, and find the object-like macro expansion(s) common to both the start and end location. Search through those for the outermost macro expansion whose range is that of the attribute plus the enclosing syntax (`__attribute__((` ... `))` or `[[` ... `]]` or similar). That macro name is the name that was used to write this type, and that's the name we should preserve. Plus, please do this all from `getAttributedType` itself so that it applies to all type attributes, not just address space attributes. ================ Comment at: test/Sema/address_space_print_macro.c:3-4 + +#define _AS1 __attribute__((address_space(1))) +#define _AS2 __attribute__((address_space(2))) + ---------------- I know this is just a test, but please avoid using reserved identifiers here. (Drop the leading `_` from these macro names.) Repository: rC Clang https://reviews.llvm.org/D51329 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
