[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This looks great, thanks!  Skimmed through the changes pretty quickly.



================
Comment at: clang/lib/AST/Mangle.cpp:237
   }
-  Out << ((TI.getPointerWidth(0) / 8) * ArgWords);
+  Out << ((TI.getPointerWidth(LangAS::Default) / 8) * ArgWords);
 }
----------------
Since you're changing this, please hoist this call out so that we do it once.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917
+    EffectiveFieldSize = FieldSize = TI.Width;
+    FieldAlign = TI.Align;
   } else {
----------------
This is a nice cleanup, but I actually can't figure out why we can't just fall into the code below.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1706
+    assert(ThisPtrTy->getPointeeType().getAddressSpace() == LangAS::Default &&
+           "this pointer not in default address space?");
     auto Align = getTypeAlignIfRequired(ThisPtrTy, CGM.getContext());
----------------
I'm pretty sure we have language modes that support methods in different address spaces, and your code doesn't seem to require this assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295