[PATCH] D80153: [AST] Mangle LambdaContextDecl for top level decl

[Reid "Away June-Sep" Kleckner via Phabricator via cfe-commits]

rnk added a comment.

After looking at the code more, I'm more convinced that your fix is in the right place.



================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:954
             if ((isa<VarDecl>(LambdaContextDecl) ||
-                 isa<FieldDecl>(LambdaContextDecl)) &&
-                LambdaContextDecl->getDeclContext()->isRecord()) {
+                 isa<FieldDecl>(LambdaContextDecl))) {
               mangleUnqualifiedName(cast<NamedDecl>(LambdaContextDecl));
----------------
I think this code is supposed to be analogous to the ItaniumMangle.cpp code here:
https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ItaniumMangle.cpp#L1829

Can you add in `&& !isa<ParmVarDecl>(LambdaContextDecl)` to this condition? I believe it will prevent changing the mangling in some of the test cases you have.


================
Comment at: clang/test/CodeGenCXX/mangle-ms-cxx11.cpp:329
     static int white;
     // CHECK-DAG: @"?white@?1???R<lambda_1>@x at A@PR31197@@QBE at XZ@4HA"
     return &white;
----------------
I see, this incorporate the field name `x` here, explaining the FieldDecl check.


================
Comment at: clang/test/CodeGenCXX/mangle-ms-cxx11.cpp:340
   static void default_args(FPtrTy x = [] {}, FPtrTy y = [] {}, int z = [] { return 1; }() + [] { return 2; }()) {}
-  // CHECK-DAG: @"??R<lambda_1_1>@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
-  // CHECK-DAG: @"??R<lambda_1_2>@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
-  // CHECK-DAG: @"??R<lambda_2_1>@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
-  // CHECK-DAG: @"??R<lambda_3_1>@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
+  // CHECK-DAG: @"??R<lambda_1_1>@z@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
+  // CHECK-DAG: @"??R<lambda_1_2>@z@?0??default_args at A@PR31197@@SAXP6AXXZ0H at Z@QBE at XZ"(
----------------
I think these should not change, because they are parameters, and in this particular area we are attempting to follow the Itanium mangling strategy. Even if clang is not ABI compatible with MSVC for this feature, I would prefer it if we didn't break ABI compatibility with old versions of clang when possible.


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

https://reviews.llvm.org/D80153