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<LangAS>((TargetAS) + ---------------- yaxunl wrote: > how about `getLangASFromTargetAS` ? It is preferred to start with small > letters. Sounds good, I'll do that. ================ Comment at: tools/libclang/CXType.cpp:402 + ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext(); + return Ctx.getTargetAddressSpace(T); } ---------------- yaxunl wrote: > Is this function suppose to return AST address space or target address space? > > Some targets e.g. x86 maps all AST address spaces to 0. Returning target > address space will not let the client unable to differentiate different > address spaces in the source language. I am not entirely sure what the correct return value is here because the current implementation returns either the LanguageAS or `LangAS - LangAS::FirstTargetAddressSpace` which can also overlap. So possibly it should just always returning the AST address space? I think for now I will just keep the current behaviour with a FIXME and create a followup patch. https://reviews.llvm.org/D38816 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits