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&id=119090#toc
Repository:
rL LLVM
https://reviews
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
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:
> > arich
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:
> >
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:
> >
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 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:
>
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:
> >
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:
>
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:
> >
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
https:/
yaxunl added inline comments.
Comment at: tools/libclang/CXType.cpp:402
+ ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext();
+ return Ctx.getTargetAddressSpace(T);
}
arichardson wrote:
> yaxunl wrote:
> > Is this function suppose to return AST add
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 `getLangASFro
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.
===
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
http://lists.llvm.o
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
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?
https://rev
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
lib/AST/A
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 ca
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 nex
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 you
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 CodeG
22 matches
Mail list logo
