[DebugInfo][CodeView] Use <lambda_n> as the display name for lambdas.

[akhuang]

Summary

Currently (for codeview) lambdas have a string like <lambda_0> in
their mangled name, and don't have any display name. This change uses the
<lambda_0> as the display name, which helps distinguish between lambdas
in -gline-tables-only, since there are no linkage names there.
It also changes how we display lambda names; previously we used
<unnamed-tag>; now it will show <lambda_0>.

I added a function to the mangling context code to create this string;
for Itanium it just returns an empty string.

Bug: https://bugs.llvm.org/show_bug.cgi?id=48432

[rnk]

I see a variety of presubmit failures in Harbormaster that seem related, so make sure to check those.

clang/include/clang/AST/Mangle.h

92

I think I would prefer to have this return a number. I think CGDebugInfo should be responsible for the formatting of the display names, not the mangler. Both the MS and Itanium manglers have private methods for numbering internal linkage lambdas, and I think it would be reasonable to hook those up to a public virtual method.

clang/lib/CodeGen/CGDebugInfo.cpp

319–322

I think this comment could be improved, and I think it would help address some of David's concerns in the previous patch about the format requirements of codeview.

clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp

18

Please add tests for a lambda in an inline function and one in a non-inline function. The inline lambda will be externally visible and have a mangling number, and the non-inline one will be internal, and will not have a mangling number. They should both get <lambda_n> names, though.

[akhuang]

-add to comment
-Add externally visible lambda to test case

[akhuang]

change to function returning a number, fill in ItaniumMangle function

[rnk]

I see a potential issue, but I think this looks good otherwise.

clang/lib/AST/ItaniumMangle.cpp

210

This has the potential to create a situation where enabling debug info changes the internal mangled names that clang uses. To be *extra* safe, you could add a method like getAnonymousStructIdForDebugInfo that does .find instead of .insert, and use it here.

clang/lib/AST/MicrosoftMangle.cpp

236

This has a similar concern, this isn't a readonly operation.

[akhuang]

clang/include/clang/AST/Mangle.h

92

I used a string because we mangle some lambdas like <lambda_1_1> (when they're used as function parameters).
Maybe we don't need to differentiate between these lambdas though?

clang/lib/AST/ItaniumMangle.cpp

210

Oh, right, yeah. I guess I'll just let it return 0 if it's not found.

[akhuang]

Avoid using getLambdaId function

[rnk]

clang/include/clang/AST/Mangle.h

92

I see. So, the situation we are worried about would look like:

struct Foo {
  static volatile int arg1_defaults, arg2_defaults;
  static void foo(
      int arg1 = []() { return ++arg1_defaults; }(),
      int arg2 = []() { return ++arg2_defaults; }());
};
void bar() { Foo::foo(); }

In this case, if we use numbers here, we'd end up using the names Foo::<lambda_1> Foo::<lambda_1>, and those would end up being duplicate LF_FUNC_IDs. Darn.

You were right, we better go back to using strings. Sorry about that. That makes a good test, though. :)

[akhuang]

clang/include/clang/AST/Mangle.h

92

Yep. I don't know what it should output for the Itanium version though. I guess it could just do the same thing, even though we don't use that string anywhere there.

[akhuang]

make function return lambda string, and add test for lambda in function parameters

[rnk]

lgtm, thanks!

[akhuang]

Fix test / string code after test failures

clang/include/clang/AST/Mangle.h

public:
   }
 
   uint64_t getAnonymousStructId(const NamedDecl *D) {
     std::pair<llvm::DenseMap<const NamedDecl *, uint64_t>::iterator, bool>
         Result = AnonStructIds.insert(std::make_pair(D, AnonStructIds.size()));
     return Result.first->second;
   }
 
+  uint64_t getAnonymousStructIdForDebugInfo(const NamedDecl *D) {
+    llvm::DenseMap<const NamedDecl *, uint64_t>::iterator Result =
+        AnonStructIds.find(D);
+    // The decl should already be inserted, but return 0 in case it is not.
+    if (Result == AnonStructIds.end())
+      return 0;
+    return Result->second;
+  }
+
+  virtual std::string getLambdaString(const CXXRecordDecl *Lambda) = 0;
+
   /// @name Mangler Entry Points
   /// @{
 
   bool shouldMangleDeclName(const NamedDecl *D);
   virtual bool shouldMangleCXXName(const NamedDecl *D) = 0;
   virtual bool shouldMangleStringLiteral(const StringLiteral *SL) = 0;
 
   // FIXME: consider replacing raw_ostream & with something like SmallString &.

clang/lib/AST/ItaniumMangle.cpp

if (!discriminator) {
       const DeclContext *DC = getEffectiveDeclContext(ND);
       discriminator = ++Discriminator[std::make_pair(DC, ND->getIdentifier())];
     }
     if (discriminator == 1)
       return false;
     disc = discriminator-2;
     return true;
   }
+
+  std::string getLambdaString(const CXXRecordDecl *Lambda) override {
+    // This function matches the one in MicrosoftMangle, which returns
+    // the string that is used in lambda mangled names.
+    assert(Lambda->isLambda() && "RD must be a lambda!");
+    std::string Name("<lambda");
+    Decl *LambdaContextDecl = Lambda->getLambdaContextDecl();
+    unsigned LambdaManglingNumber = Lambda->getLambdaManglingNumber();
+    unsigned LambdaId;
+    const ParmVarDecl *Parm = dyn_cast_or_null<ParmVarDecl>(LambdaContextDecl);
+    const FunctionDecl *Func =
+        Parm ? dyn_cast<FunctionDecl>(Parm->getDeclContext()) : nullptr;
+
+    if (Func) {
+      unsigned DefaultArgNo =
+          Func->getNumParams() - Parm->getFunctionScopeIndex();
+      Name += llvm::utostr(DefaultArgNo);
+      Name += "_";
+    }
+
+    if (LambdaManglingNumber)
+      LambdaId = LambdaManglingNumber;
+    else
+      LambdaId = getAnonymousStructIdForDebugInfo(Lambda);
+
+    Name += llvm::utostr(LambdaId);
+    Name += '>';
+    return Name;
+  }
+
   /// @}
 };
 
 /// Manage the mangling of a single name.
 class CXXNameMangler {
   ItaniumMangleContextImpl &Context;
   raw_ostream &Out;
   bool NullOut = false;

clang/lib/AST/MicrosoftMangle.cpp

bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) {
     // Make up a reasonable number for internal decls.
     unsigned &discriminator = Uniquifier[ND];
     if (!discriminator)
       discriminator = ++Discriminator[std::make_pair(DC, ND->getIdentifier())];
     disc = discriminator + 1;
     return true;
   }
 
+  std::string getLambdaString(const CXXRecordDecl *Lambda) override {
+    assert(Lambda->isLambda() && "RD must be a lambda!");
+    std::string Name("<lambda_");
+
+    Decl *LambdaContextDecl = Lambda->getLambdaContextDecl();
+    unsigned LambdaManglingNumber = Lambda->getLambdaManglingNumber();
+    unsigned LambdaId;
+    const ParmVarDecl *Parm = dyn_cast_or_null<ParmVarDecl>(LambdaContextDecl);
+    const FunctionDecl *Func =
+        Parm ? dyn_cast<FunctionDecl>(Parm->getDeclContext()) : nullptr;
+
+    if (Func) {
+      unsigned DefaultArgNo =
+          Func->getNumParams() - Parm->getFunctionScopeIndex();
+      Name += llvm::utostr(DefaultArgNo);
+      Name += "_";
+    }
+
+    if (LambdaManglingNumber)
+      LambdaId = LambdaManglingNumber;
+    else
+      LambdaId = getLambdaIdForDebugInfo(Lambda);
+
+    Name += llvm::utostr(LambdaId);
+    Name += ">";
+    return Name;
+  }
+
   unsigned getLambdaId(const CXXRecordDecl *RD) {
     assert(RD->isLambda() && "RD must be a lambda!");
     assert(!RD->isExternallyVisible() && "RD must not be visible!");
     assert(RD->getLambdaManglingNumber() == 0 &&
            "RD must not have a mangling number!");
     std::pair<llvm::DenseMap<const CXXRecordDecl *, unsigned>::iterator, bool>
         Result = LambdaIds.insert(std::make_pair(RD, LambdaIds.size()));
     return Result.first->second;
   }
 
+  unsigned getLambdaIdForDebugInfo(const CXXRecordDecl *RD) {
+    assert(RD->isLambda() && "RD must be a lambda!");
+    assert(!RD->isExternallyVisible() && "RD must not be visible!");
+    assert(RD->getLambdaManglingNumber() == 0 &&
+           "RD must not have a mangling number!");
+    llvm::DenseMap<const CXXRecordDecl *, unsigned>::iterator Result =
+        LambdaIds.find(RD);
+    // The lambda should exist, but return 0 in case it doesn't.
+    if (Result == LambdaIds.end())
+      return 0;
+    return Result->second;
+  }
+
   /// Return a character sequence that is (somewhat) unique to the TU suitable
   /// for mangling anonymous namespaces.
   StringRef getAnonymousNamespaceHash() const {
     return AnonymousNamespaceHash;
   }
 
 private:
   void mangleInitFiniStub(const VarDecl *D, char CharCode, raw_ostream &Out);

clang/lib/CodeGen/CGDebugInfo.cpp

if (isa<ClassTemplateSpecializationDecl>(RD)) {
     // Copy this name on the side and use its reference.
     return internString(Name);
   }
 
   // quick optimization to avoid having to intern strings that are already
   // stored reliably elsewhere
   if (const IdentifierInfo *II = RD->getIdentifier())
     return II->getName();
 
-  // The CodeView printer in LLVM wants to see the names of unnamed types: it is
-  // used to reconstruct the fully qualified type names.
+  // The CodeView printer in LLVM wants to see the names of unnamed types
+  // because they need to have a unique identifier.
+  // These names are used to reconstruct the fully qualified type names.
   if (CGM.getCodeGenOpts().EmitCodeView) {
     if (const TypedefNameDecl *D = RD->getTypedefNameForAnonDecl()) {
       assert(RD->getDeclContext() == D->getDeclContext() &&
              "Typedef should not be in another decl context!");
       assert(D->getDeclName().getAsIdentifierInfo() &&
              "Typedef was not named!");
       return D->getDeclName().getAsIdentifierInfo()->getName();
     }
 
     if (CGM.getLangOpts().CPlusPlus) {
       StringRef Name;
 
       ASTContext &Context = CGM.getContext();
       if (const DeclaratorDecl *DD = Context.getDeclaratorForUnnamedTagDecl(RD))
         // Anonymous types without a name for linkage purposes have their
         // declarator mangled in if they have one.
         Name = DD->getName();
       else if (const TypedefNameDecl *TND =
                    Context.getTypedefNameForUnnamedTagDecl(RD))
         // Anonymous types without a name for linkage purposes have their
         // associate typedef mangled in if they have one.
         Name = TND->getName();
 
+      // Give lambdas a display name based on their name mangling.
+      if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+        if (CXXRD->isLambda())
+          return internString(
+              CGM.getCXXABI().getMangleContext().getLambdaString(CXXRD));
+
       if (!Name.empty()) {
         SmallString<256> UnnamedType("<unnamed-type-");
         UnnamedType += Name;
         UnnamedType += '>';
         return internString(UnnamedType);
       }
     }
   }

clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp

int main(int argc, char* argv[], char* arge[]) {
   // LINUX-NOT:      identifier:
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "four"
   // MSVC-SAME:      type: [[TYPE_OF_FOUR:![0-9]+]]
   // MSVC-SAME:      )
   // MSVC:       [[TYPE_OF_FOUR]] = distinct !DICompositeType
   // MSVC-SAME:      tag: DW_TAG_class_type
-  // MSVC-NOT:       name:
+  // MSVC-SAME:      name: "<lambda_0>"
   // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
   // MSVC-SAME:      )
 
   return 0;
 }

clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp

-// RUN: %clang_cc1 %s -gcodeview -debug-info-kind=line-tables-only -S \
-// RUN:   -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-windows-msvc -gcodeview
+// RUN: -debug-info-kind=line-tables-only -emit-llvm -o - | FileCheck %s
 // Checks that clang with "-gline-tables-only" with CodeView emits some debug
 // info for variables and types when they appear in function scopes.
 
 namespace NS {
 struct C {
   void m() {}
+  // Test externally visible lambda.
+  void lambda2() { []() {}(); }
+ 
+  // Test naming for function parameters.
+  void lambda_params(int x = [](){ return 0; }(), int y = [](){ return 1; }()) {}
 };
 void f() {}
 }
 
+// Test non- externally visible lambda.
+auto lambda1 = []() { return 1; };
+
 NS::C c;
 
+
 void test() {
   // CHECK: !DISubprogram(name: "f", scope: ![[NS:[0-9]+]],
   // CHECK-SAME:          type: ![[F:[0-9]+]]
   // CHECK: ![[NS]] = !DINamespace(name: "NS", scope: null)
   // CHECK: ![[F]] = !DISubroutineType(types: ![[FTYPE:[0-9]+]])
   // CHECK: ![[FTYPE]] = !{null}
   NS::f();
 
   // CHECK: ![[M:[0-9]+]] = distinct !DISubprogram(name: "m", scope: ![[C:[0-9]+]],
-  // CHECK-SAME:                                   type: ![[MTYPE:[0-9]+]]
+  // CHECK-SAME:                                   type: ![[MTYPE:[0-9]+]],
   // CHECK: ![[C]] = !DICompositeType(tag: DW_TAG_structure_type, name: "C",
   // CHECK-SAME:                      flags: DIFlagFwdDecl
   // CHECK-NOT: identifier
   // CHECK: ![[MTYPE]] = !DISubroutineType({{.*}}types: !{{.*}})
   c.m();
+
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA0:[0-9]+]],
+  // CHECK: ![[LAMBDA0]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK-SAME:                            name: "<lambda_0>",
+  // CHECK-SAME:                            flags: DIFlagFwdDecl
+  lambda1();
+
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1_1:[0-9]+]],
+  // CHECK: ![[LAMBDA1_1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK-SAME:                              name: "<lambda_1_1>",
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA2_1:[0-9]+]],
+  // CHECK: ![[LAMBDA2_1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK-SAME:                              name: "<lambda_2_1>",
+  c.lambda_params();
+
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1:[0-9]+]],
+  // CHECK: ![[LAMBDA1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK-SAME:                            name: "<lambda_1>",
+  // CHECK-SAME:                            flags: DIFlagFwdDecl
+  c.lambda2();
 }