[PATCH] D71039: Add support for the MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.


================
Comment at: clang/include/clang/AST/Type.h:477-479
+           ((isPtrSizeAddressSpace(A) && B == LangAS::Default) ||
+            (isPtrSizeAddressSpace(B) && A == LangAS::Default) ||
+            (isPtrSizeAddressSpace(A) && isPtrSizeAddressSpace(B)));
----------------
rnk wrote:
> 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.
Yes -- I think I considered doing this and then forgot that we already return true when A and B are both default. 


================
Comment at: clang/lib/Sema/SemaDecl.cpp:3156
+
+static bool HasSameFunctionTypeIgnoringPointerSizes(ASTContext &Ctx,
+                                                    QualType Old,
----------------
rnk wrote:
> 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.
Done, this does make the code a bit shorter. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71039/new/

https://reviews.llvm.org/D71039