[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-15 Thread Alexander Richardson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL315871: Convert clang::LangAS to a strongly typed enum (authored by arichardson). Changed prior to commit: https://reviews.llvm.org/D38816?vs=118915=119090#toc Repository: rL LLVM

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision. yaxunl added a comment. LGTM. Thanks! Great work! https://reviews.llvm.org/D38816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() arichardson wrote: > yaxunl wrote: > >

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() yaxunl wrote: > arichardson wrote: > >

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() arichardson wrote: > arichardson wrote: > >

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
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:

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() Anastasia wrote: > arichardson wrote:

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Anastasia Stulova via Phabricator via cfe-commits
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: >

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() Anastasia wrote: > arichardson wrote:

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Anastasia Stulova via Phabricator via cfe-commits
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: >

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 118915. arichardson edited the summary of this revision. arichardson added a comment. This revision is now accepted and ready to land. - Keep old behaviour for clang_getAddressSpace() - renamed to getLangASFromTargetAS() - rebased on latest trunk

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: tools/libclang/CXType.cpp:402 + ASTContext = cxtu::getASTUnit(GetTU(CT))->getASTContext(); + return Ctx.getTargetAddressSpace(T); } arichardson wrote: > yaxunl wrote: > > Is this function suppose to return AST

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson planned changes to this revision. arichardson added inline comments. Comment at: include/clang/Basic/AddressSpaces.h:66 +inline LangAS LangASFromTargetAS(unsigned TargetAS) { + return static_cast((TargetAS) + yaxunl wrote: > how about

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: include/clang/Basic/AddressSpaces.h:66 +inline LangAS LangASFromTargetAS(unsigned TargetAS) { + return static_cast((TargetAS) + how about `getLangASFromTargetAS` ? It is preferred to start with small letters.

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Alexey Bader via Phabricator via cfe-commits
bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. @arichardson, great work! Thanks a lot! https://reviews.llvm.org/D38816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() Anastasia wrote: > Why do we need this

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments. Comment at: lib/AST/TypePrinter.cpp:1323 OS << "address_space("; -OS << T->getEquivalentType().getAddressSpace(); +OS << T->getEquivalentType() + .getQualifiers() Why do we need this change?

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-12 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 118784. arichardson added a comment. Removed the additional namespace https://reviews.llvm.org/D38816 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Basic/AddressSpaces.h include/clang/Basic/TargetInfo.h

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-11 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. > The only reason I added this namespace is that I wasn't sure whether having > those functions in the clang namespace is acceptable. Maybe someone else will object, or suggest an existing namespace they should be in. FWIW I think it's fine. > Not quite sure what to

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-11 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments. Comment at: include/clang/Basic/AddressSpaces.h:51 +namespace LanguageAS { /// The type of a lookup table which maps from language-specific address spaces jlebar wrote: > I wonder if you need this namespace? LangAS right

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-11 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. My only regret is that I have but one +1 to give to this patch. Comment at: include/clang/Basic/AddressSpaces.h:51 +namespace LanguageAS { /// The type of a lookup table which maps from language-specific address spaces I wonder if

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-11 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson created this revision. Herald added subscribers: Anastasia, nhaehnle, jholewinski. Currently both clang AST address spaces and target specific address spaces are represented as unsigned which can lead to subtle errors if the wrong type is passed. It is especially confusing in the