[Indexing] Fixing inconsistencies between FUNCDNAME and generated code by improving ASTContext's API for MangleContext

[pacxx]

Summary

The MangleContext API from ASTContext is used incorrectly in the handling of the FUNCDNAME resulting in inconsistencies between the results of FUNCDNAME and the mangled function names in CodeGen.

This fix simplifies the ASTContext API so that it owns the mangling context what removes the inconsistencies.

[rnk]

Can you update the description to clarify that this is fixing a bug in the indexing library? From the description it sounds like we have a serious bug in FUNCDNAME codegen, which is not the case. CodeGen does the right thing. The ASTContext API is just crappy, so the Index library used it incorrectly. The fix is to simplify the ASTContext API so that it owns the mangling context.

include/clang/AST/ASTContext.h

2161–2163

Please refactor all the other code that uses createMangleContext to use getMangleContext, and then make createMangleContext private.

[pacxx]

Refactored as suggested.

[rnk]

Code looks good, but we should test it. Can you add a CodeGenCXX test that shows the issue with FUNCDNAME? I see the issue with this program:

#include <stdio.h>
int main() {
  const char *s1 = ([]() { return __FUNCDNAME__; })();
  const char *s2 = ([]() { return __FUNCDNAME__; })();
  printf("%s\n%s\n", s1, s2);
}

lib/AST/Expr.cpp

500–502

Oh, I see, FUNCDNAME was really broken. Ignore my previous comment.

include/clang/AST/ASTContext.h

public:
   uint64_t lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
                                 const ObjCImplementationDecl *ID,
                                 const ObjCIvarDecl *Ivar) const;
 
   bool isNearlyEmpty(const CXXRecordDecl *RD) const;
 
   VTableContextBase *getVTableContext();
 
+private:
   MangleContext *createMangleContext();
+public:
+  MangleContext &getMangleContext();
 
   void DeepCollectObjCIvars(const ObjCInterfaceDecl *OI, bool leafClass,
                             SmallVectorImpl<const ObjCIvarDecl*> &Ivars) const;
 
   unsigned CountNonClassIvars(const ObjCInterfaceDecl *OI) const;
   void CollectInheritedProtocols(const Decl *CDecl,
                           llvm::SmallPtrSet<ObjCProtocolDecl*, 8> &Protocols);
 
private:
   // by DeclContext objects.  This probably should not be in ASTContext,
   // but we include it here so that ASTContext can quickly deallocate them.
   llvm::PointerIntPair<StoredDeclsMap *, 1> LastSDM;
 
   std::unique_ptr<ParentMapPointers> PointerParents;
   std::unique_ptr<ParentMapOtherNodes> OtherParents;
 
   std::unique_ptr<VTableContextBase> VTContext;
+  std::unique_ptr<MangleContext> MContext;
 
   void ReleaseDeclContextMaps();
   void ReleaseParentMapEntries();
 
 public:
   enum PragmaSectionFlag : unsigned {
     PSF_None = 0,
     PSF_Read = 0x1,

lib/AST/ASTContext.cpp

MangleContext *ASTContext::createMangleContext() {
   case TargetCXXABI::WatchOS:
     return ItaniumMangleContext::create(*this, getDiagnostics());
   case TargetCXXABI::Microsoft:
     return MicrosoftMangleContext::create(*this, getDiagnostics());
   }
   llvm_unreachable("Unsupported ABI");
 }
 
+MangleContext &ASTContext::getMangleContext() {
+  if (!MContext)
+    MContext.reset(createMangleContext());
+  return *MContext.get();
+}
+
 CXXABI::~CXXABI() = default;
 
 size_t ASTContext::getSideTableAllocatedMemory() const {
   return ASTRecordLayouts.getMemorySize() +
          llvm::capacity_in_bytes(ObjCLayouts) +
          llvm::capacity_in_bytes(KeyFunctions) +
          llvm::capacity_in_bytes(ObjCImpls) +
          llvm::capacity_in_bytes(BlockVarCopyInits) +

lib/AST/Expr.cpp

 
 // FIXME: Maybe this should use DeclPrinter with a special "print predefined
 // expr" policy instead.
 std::string PredefinedExpr::ComputeName(IdentType IT, const Decl *CurrentDecl) {
   ASTContext &Context = CurrentDecl->getASTContext();
 
   if (IT == PredefinedExpr::FuncDName) {
     if (const NamedDecl *ND = dyn_cast<NamedDecl>(CurrentDecl)) {
-      std::unique_ptr<MangleContext> MC;
-      MC.reset(Context.createMangleContext());
+      MangleContext& MC = Context.getMangleContext();
 
-      if (MC->shouldMangleDeclName(ND)) {
+      if (MC.shouldMangleDeclName(ND)) {
         SmallString<256> Buffer;
         llvm::raw_svector_ostream Out(Buffer);
         if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(ND))
-          MC->mangleCXXCtor(CD, Ctor_Base, Out);
+          MC.mangleCXXCtor(CD, Ctor_Base, Out);
         else if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(ND))
-          MC->mangleCXXDtor(DD, Dtor_Base, Out);
+          MC.mangleCXXDtor(DD, Dtor_Base, Out);
         else
-          MC->mangleName(ND, Out);
+          MC.mangleName(ND, Out);
 
         if (!Buffer.empty() && Buffer.front() == '\01')
           return Buffer.substr(1);
         return Buffer.str();
       } else
         return ND->getIdentifier()->getName();
     }
     return "";

lib/CodeGen/CGCXXABI.h

 class CodeGenFunction;
 class CodeGenModule;
 struct CatchTypeInfo;
 
 /// \brief Implements C++ ABI-specific code generation functions.
 class CGCXXABI {
 protected:
   CodeGenModule &CGM;
-  std::unique_ptr<MangleContext> MangleCtx;
+  MangleContext &MangleCtx;
 
   CGCXXABI(CodeGenModule &CGM)
-    : CGM(CGM), MangleCtx(CGM.getContext().createMangleContext()) {}
+    : CGM(CGM), MangleCtx(CGM.getContext().getMangleContext()) {}
 
 protected:
   ImplicitParamDecl *getThisDecl(CodeGenFunction &CGF) {
     return CGF.CXXABIThisDecl;
   }
   llvm::Value *getThisValue(CodeGenFunction &CGF) {
     return CGF.CXXABIThisValue;
   }
protected:
   virtual bool isThisCompleteObject(GlobalDecl GD) const = 0;
 
 public:
 
   virtual ~CGCXXABI();
 
   /// Gets the mangle context.
   MangleContext &getMangleContext() {
-    return *MangleCtx;
+    return MangleCtx;
   }
 
   /// Returns true if the given constructor or destructor is one of the
   /// kinds that the ABI says returns 'this' (only applies when called
   /// non-virtually for destructors).
   ///
   /// There currently is no way to indicate if a destructor returns 'this'
   /// when called virtually, and code generation does not support the case.

lib/Index/CodegenNameGenerator.cpp

 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace clang::index;
 
 struct CodegenNameGenerator::Implementation {
-  std::unique_ptr<MangleContext> MC;
+  MangleContext& MC;
   llvm::DataLayout DL;
 
   Implementation(ASTContext &Ctx)
-    : MC(Ctx.createMangleContext()),
+    : MC(Ctx.getMangleContext()),
       DL(Ctx.getTargetInfo().getDataLayout()) {}
 
   bool writeName(const Decl *D, raw_ostream &OS) {
     // First apply frontend mangling.
     SmallString<128> FrontendBuf;
     llvm::raw_svector_ostream FrontendBufOS(FrontendBuf);
     if (auto *FD = dyn_cast<FunctionDecl>(D)) {
       if (FD->isDependentContext())
         return true;
       if (writeFuncOrVarName(FD, FrontendBufOS))
         return true;
     } else if (auto *VD = dyn_cast<VarDecl>(D)) {
       if (writeFuncOrVarName(VD, FrontendBufOS))
         return true;
     } else if (auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
-      MC->mangleObjCMethodNameWithoutSize(MD, OS);
+      MC.mangleObjCMethodNameWithoutSize(MD, OS);
       return false;
     } else if (auto *ID = dyn_cast<ObjCInterfaceDecl>(D)) {
       writeObjCClassName(ID, FrontendBufOS);
     } else {
       return true;
     }
 
     // Now apply backend mangling.
if (const auto *OCD = dyn_cast<ObjCContainerDecl>(D))
       return getAllManglings(OCD);
 
     if (!(isa<CXXRecordDecl>(D) || isa<CXXMethodDecl>(D)))
       return {};
 
     const NamedDecl *ND = cast<NamedDecl>(D);
 
     ASTContext &Ctx = ND->getASTContext();
-    std::unique_ptr<MangleContext> M(Ctx.createMangleContext());
 
     std::vector<std::string> Manglings;
 
     auto hasDefaultCXXMethodCC = [](ASTContext &C, const CXXMethodDecl *MD) {
       auto DefaultCC = C.getDefaultCallingConvention(/*IsVariadic=*/false,
                                                      /*IsCSSMethod=*/true);
       auto CC = MD->getType()->getAs<FunctionProtoType>()->getCallConv();
       return CC == DefaultCC;
if (const auto *CD = dyn_cast_or_null<CXXConstructorDecl>(ND)) {
             Manglings.emplace_back(getMangledThunk(MD, T));
     }
 
     return Manglings;
   }
 
 private:
   bool writeFuncOrVarName(const NamedDecl *D, raw_ostream &OS) {
-    if (MC->shouldMangleDeclName(D)) {
+    if (MC.shouldMangleDeclName(D)) {
       if (const auto *CtorD = dyn_cast<CXXConstructorDecl>(D))
-        MC->mangleCXXCtor(CtorD, Ctor_Complete, OS);
+        MC.mangleCXXCtor(CtorD, Ctor_Complete, OS);
       else if (const auto *DtorD = dyn_cast<CXXDestructorDecl>(D))
-        MC->mangleCXXDtor(DtorD, Dtor_Complete, OS);
+        MC.mangleCXXDtor(DtorD, Dtor_Complete, OS);
       else
-        MC->mangleName(D, OS);
+        MC.mangleName(D, OS);
       return false;
     } else {
       IdentifierInfo *II = D->getIdentifier();
       if (!II)
         return true;
       OS << II->getName();
       return false;
     }
static StringRef getClassSymbolPrefix(ObjCKind Kind, const ASTContext &Context) {
     return Kind == ObjCMetaclass ? "OBJC_METACLASS_$_" : "OBJC_CLASS_$_";
   }
 
   std::string getMangledStructor(const NamedDecl *ND, unsigned StructorType) {
     std::string FrontendBuf;
     llvm::raw_string_ostream FOS(FrontendBuf);
 
     if (const auto *CD = dyn_cast_or_null<CXXConstructorDecl>(ND))
-      MC->mangleCXXCtor(CD, static_cast<CXXCtorType>(StructorType), FOS);
+      MC.mangleCXXCtor(CD, static_cast<CXXCtorType>(StructorType), FOS);
     else if (const auto *DD = dyn_cast_or_null<CXXDestructorDecl>(ND))
-      MC->mangleCXXDtor(DD, static_cast<CXXDtorType>(StructorType), FOS);
+      MC.mangleCXXDtor(DD, static_cast<CXXDtorType>(StructorType), FOS);
 
     std::string BackendBuf;
     llvm::raw_string_ostream BOS(BackendBuf);
 
     llvm::Mangler::getNameWithPrefix(BOS, FOS.str(), DL);
 
     return BOS.str();
   }
 
   std::string getMangledThunk(const CXXMethodDecl *MD, const ThunkInfo &T) {
     std::string FrontendBuf;
     llvm::raw_string_ostream FOS(FrontendBuf);
 
-    MC->mangleThunk(MD, T, FOS);
+    MC.mangleThunk(MD, T, FOS);
 
     std::string BackendBuf;
     llvm::raw_string_ostream BOS(BackendBuf);
 
     llvm::Mangler::getNameWithPrefix(BOS, FOS.str(), DL);
 
     return BOS.str();
   }