rnk added a comment. Sorry for the delay, overall this seems like the right approach.
================ Comment at: clang/include/clang/AST/Type.h:477-479 + ((isPtrSizeAddressSpace(A) && B == LangAS::Default) || + (isPtrSizeAddressSpace(B) && A == LangAS::Default) || + (isPtrSizeAddressSpace(A) && isPtrSizeAddressSpace(B))); ---------------- Can this be simplified to: ((isPtrSizeAddressSpace(A) || A == LangAS::Default) && (isPtrSizeAddressSpace(B) || B == LangAS::Default)) Mainly I wanted to avoid recomputing isPtrSizeAddressSpace for A and B. I think it's only not equivalent when A and B are both default, but we already return true in that case. ================ Comment at: clang/lib/AST/MicrosoftMangle.cpp:1874-1881 + case LangAS::ptr32_sptr: + Extra.mangleSourceName("_ASPtr32_sptr"); + break; + case LangAS::ptr32_uptr: + Extra.mangleSourceName("_ASPtr32_uptr"); + break; + case LangAS::ptr64: ---------------- This code should be unreachable because you check for these address spaces at the call site. I think you can do something like this: case LangAS::...: case LangAS::...: case LangAS::...: llvm_unreachable("don't mangle ptr address spaces with _AS"); ================ Comment at: clang/lib/AST/TypePrinter.cpp:1824 + case LangAS::ptr32_sptr: + OS << "__ptr32_sptr"; + break; ---------------- Think we should say `"__sptr __ptr32"`? This code doesn't guarantee that it can be parsed back as valid source, but it's closer. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:3156 + +static bool HasSameFunctionTypeIgnoringPointerSizes(ASTContext &Ctx, + QualType Old, ---------------- I wonder if the simplest way to express this would be to follow the pattern of getFunctionTypeWithExceptionSpec and hasSameFunctionTypeIgnoringExceptionSpec, i.e. make a function that strips pointer sized address spaces off of pointer typed arguments, returns it, and then compare them. ASTContext would be a natural place for that kind of type adjustment. ================ Comment at: clang/lib/Sema/SemaOverload.cpp:2890 +static QualType RemovePtrSizeAddrSpace(ASTContext &Ctx, QualType T) { + if (const PointerType *Ptr = T->getAs<PointerType>()) { ---------------- I think it would be fair to raise this method up to ASTContext, next to getAddrSpaceQualType, removeAddrSpaceQualType, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71039/new/ https://reviews.llvm.org/D71039 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits