[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/AST/Expr.cpp:693
         else
-          MC->mangleName(ND, Out);
+          GD = GlobalDecl(ND);
+        MC->mangleName(GD, Out);
----------------
This will need an extension for your case, right?  Maybe there should be comments in `GlobalDecl` pointing out all the places where we need to go from an arbitrary `Decl*` to a `GlobalDecl` so that someone adding a new kind of discriminated declaration will know to update them all.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+    GD = GlobalDecl(dyn_cast<FunctionDecl>(DC));
+  return GD;
----------------
`cast`?  But I'm not sure this is true, local entities can be in non-function declarations: blocks, ObjC methods, and captured statements.  You can just `cast<Decl>(DC)`.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1028
     else
-      MC.mangleName(ND, Out);
+      MC.mangleName(GD, Out);
   } else {
----------------
yaxunl wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > What would be necessary in order for this to turn into just `mangleName(GD, Out)`?   I suspect there are a lot of subtle assumptions between the different mangling methods today that could probably be broken down a bit and we'd end up with something much cleaner.
> > It seems to be just API changes. I will try changing them.
> I got one issue. When I tried to create `GlobalDecl(CD, Ctor_DefaultClosure)` I got assertion
> 
> include/llvm/ADT/PointerIntPair.h:186: static intptr_t llvm::PointerIntPairInfo<const clang::Decl *, 2, llvm::PointerLikeTypeTraits<const clang::Decl *> >::updateInt(intptr_t, intptr_t) [PointerT = const clang::Decl *, IntBits = 2, PtrTraits = llvm::PointerLikeTypeTraits<const clang::Decl *>]: Assertion `(IntWord & ~IntMask) == 0 && "Integer too large for field"' failed.
> CXXConstructor=v:7:25 (default constructor) (converting constructor)
> 
> This is because Ctor_DefaultClosure has value 4, which exceeds the value that can be put into PointerIntPairInfo
It looks like `Decl` is declared with alignment 8, so yeah, it's safe to make that a 3-bit field.


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

https://reviews.llvm.org/D75700