[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: clang/lib/AST/Type.cpp:382-386 +QualType QualType::IgnoreMacroDefinitions(QualType T) { + while (const auto *MDT = T->getAs()) +T = MDT->getUnderlyingType(); + return T; +} rsmith wrote: > This doesn't seem lik

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 184439. leonardchan marked 15 inline comments as done. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/ https://reviews.llvm.org/D51329 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Recurs

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

rsmith added inline comments. Comment at: clang/include/clang/AST/Type.h:4183 +/// Sugar type that represents a type that was defined in a macro. +class MacroDefinedType : public Type { + friend class ASTContext; // ASTContext creates these. This represents a ma

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan marked 2 inline comments as done. leonardchan added inline comments. Comment at: clang/lib/AST/TypePrinter.cpp:958-964 + // Remove the address_space qualifier so it does not get printed. We + // instead want to print the macro only. + SplitQualType Spli

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 183242. leonardchan added a comment. - Rebase after D55447 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/ https://reviews.llvm.org/D51329 Files: clang/include/clang/AST/ASTCo

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 177335. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/ https://reviews.llvm.org/D51329 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/AST/Type.

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: clang/lib/AST/TypePrinter.cpp:958-964 + // Remove the address_space qualifier so it does not get printed. We + // instead want to print the macro only. + SplitQualType SplitTy = AttrTy->getModifiedType().split(); +

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. 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.Address

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 173090. leonardchan marked an inline comment as done. leonardchan added a comment. Herald added a subscriber: arphaman. - Added a new type sugar node `MacroDefinedType` which wraps `AttributedType`s for the purpose of determining if an attribute was defin

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

rsmith added a comment. In https://reviews.llvm.org/D51329#1229061, @leonardchan wrote: > @rsmith So I chose to go with the type sugar route since we would prefer to > have multiple attributes in one attribute list. This type just contains the > underlying type and the macro identifier original

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan marked 7 inline comments as done. leonardchan added a comment. @rsmith So I chose to go with the type sugar route since we would prefer to have multiple attributes in one attribute list. This type just contains the underlying type and the macro identifier originally held by the `Att

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

mcgrathr added inline comments. Comment at: lib/Parse/ParseDecl.cpp:244-252 +// If this was declared in a macro, attatch the macro IdentifierInfo to the +// parsed attribute. +for (const auto &MacroPair : PP.getAttributeMacros()) { + if (SourceLocInSourceRange(At

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

rsmith added a comment. I think this patch is nearly ready to commit. However... if you want to handle macros that specify a sequence of attributes, we should switch to treating the macro name as a separate type sugar node rather than modeling it as a name for the `AttributedType` node. Up to y

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: lib/AST/TypePrinter.cpp:1370 + +// Remove the underlying address space so it won't be printed. +SplitQualType SplitTy = T->getModifiedType().split(); rsmith wrote: > leonardchan wrote: > > rsmith wrote: > > >

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 163604. leonardchan marked 8 inline comments as done. leonardchan added a comment. - Removed default value for `getAttributedType` and added any macro identifier necessary as a fourth argument to `getAttributedType` - Added tracking for `LateAttr`s to kee

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

aaron.ballman added inline comments. Comment at: include/clang/AST/Type.h:4343 QualType getEquivalentType() const { return EquivalentType; } + IdentifierInfo *getAddressSpaceMacroII() const { return AddressSpaceMacroII; } + bool hasAddressSpaceMacroII() const { return Addre

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: lib/Parse/ParseDecl.cpp:244-252 +// If this was declared in a macro, attatch the macro IdentifierInfo to the +// parsed attribute. +for (const auto &MacroPair : PP.getAttributeMacros()) { + if (SourceLocInSourceRange

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan marked 5 inline comments as done. leonardchan added a comment. @rsmith When you clarify what you mean by `outermost macro expansion`? Say I have the following: #define ATTR __attribute__ #define AS(i) address_space(i) #define AS1 ATTR((address_space(1))) #define AS2 ATTR((AS(

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

rsmith added a comment. You also need to update the various other places that create `AttributedType`s (eg, template instantiation, ASTImporter, deserialization) to pass in the macro name. You can try removing the default argument from `ASTContext::getAttributedType` to find other places that

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: lib/AST/TypePrinter.cpp:1370 + +// Remove the underlying address space so it won't be printed. +SplitQualType SplitTy = T->getModifiedType().split(); rsmith wrote: > leonardchan wrote: > > rsmith wrote: > > >

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 163155. Repository: rC Clang https://reviews.llvm.org/D51329 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Lex/Preprocessor.h include/clang/Sema/ParsedAttr.h lib/AST/ASTContext.cpp lib/AST/TypePrinter.cpp li

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

rsmith added a comment. +@aaron.ballman for attributes-related changes. It would be easier and more precise to track the macro corresponding to an attribute in the `Parser` rather than in `Sema` (and stash it on the `ParsedAttr` for consumers such as `Sema` that want it). That way, we still ha

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added inline comments. Comment at: lib/AST/TypePrinter.cpp:1370 + +// Remove the underlying address space so it won't be printed. +SplitQualType SplitTy = T->getModifiedType().split(); rsmith wrote: > This is unnecessary; just print the modifi

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan updated this revision to Diff 162966. leonardchan marked 3 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D51329 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Lex/Preprocessor.h lib/AST/ASTContext.cpp lib/AST/TypePrin

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

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 addres

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan added a comment. Actually, after posting this I realize I didn't handle the macro expansion very well. I'll fix this first. Repository: rC Clang https://reviews.llvm.org/D51329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

leonardchan created this revision. leonardchan added reviewers: rsmith, rjmccall, ebevhan, mcgrathr, phosek. leonardchan added a project: clang. If an `address_space` attribute is defined in a macro, print the macro instead when diagnosing a warning or error for incompatible pointers with differe