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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/lib/AST/Expr.cpp:693
         else
-          MC->mangleName(ND, Out);
+          GD = GlobalDecl(ND);
+        MC->mangleName(GD, Out);
----------------
rjmccall wrote:
> 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.
Added extension for kernel reference here and other places in the other patch. Also added comments to GlobalDecl.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+    GD = GlobalDecl(dyn_cast<FunctionDecl>(DC));
+  return GD;
----------------
rjmccall wrote:
> `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)`.
Original code is `cast<FunctionDecl>(DC)` since `ObjCMethodDecl` and `BlockDecl` are handled separately. Since we dot have GlobalDecl ctor for Decl*, I will cast to FunctionDecl.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+    GD = GlobalDecl(dyn_cast<FunctionDecl>(DC));
+  return GD;
----------------
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > `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)`.
> > Original code is `cast<FunctionDecl>(DC)` since `ObjCMethodDecl` and `BlockDecl` are handled separately. Since we dot have GlobalDecl ctor for Decl*, I will cast to FunctionDecl.
> I guess we just never enter this for local names within ObjC methods or blocks?
Yes.


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

https://reviews.llvm.org/D75700