[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
akhuang requested review of this revision.

Basically, the code was checking if there were no constructors at all,
but didn't check that there were any non copy/move constructors.

I _think_ the code now reflects what the logic for ctor homing was intended to 
be.
It would be nice to have more test cases but I guess I also don't have a great
understanding of what sorts of situations can happen with constructors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87808

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -31,3 +31,16 @@
   F(int) {}
   int i;
 } TestF;
+
+// CHECK-DAG: ![[G:.*]] ={{.*}}!DICompositeType({{.*}}name: 
"G"{{.*}}DIFlagTypePassByValue
+// CHECK-DAG: !DICompositeType({{.*}}scope: ![[G]], {{.*}}DIFlagTypePassByValue
+// The inner struct only has copy/move constructors so it shouldn't be subject
+// to constructor homing.
+struct G {
+  G() : g_(0) {}
+  struct {
+    int g_;
+  };
+} TestG;
+
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2289,14 +2289,15 @@
       isClassOrMethodDLLImport(RD))
     return false;
 
-  if (RD->ctors().empty())
-    return false;
-
-  for (const auto *Ctor : RD->ctors())
+  bool hasCtor = false;
+  for (const auto *Ctor : RD->ctors()) {
     if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
       return false;
+    if (!Ctor->isCopyOrMoveConstructor())
+      hasCtor = true;
+  }
 
-  return true;
+  return hasCtor;
 }
 
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,



Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -31,3 +31,16 @@
   F(int) {}
   int i;
 } TestF;
+
+// CHECK-DAG: ![[G:.*]] ={{.*}}!DICompositeType({{.*}}name: "G"{{.*}}DIFlagTypePassByValue
+// CHECK-DAG: !DICompositeType({{.*}}scope: ![[G]], {{.*}}DIFlagTypePassByValue
+// The inner struct only has copy/move constructors so it shouldn't be subject
+// to constructor homing.
+struct G {
+  G() : g_(0) {}
+  struct {
+    int g_;
+  };
+} TestG;
+
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2289,14 +2289,15 @@
       isClassOrMethodDLLImport(RD))
     return false;
 
-  if (RD->ctors().empty())
-    return false;
-
-  for (const auto *Ctor : RD->ctors())
+  bool hasCtor = false;
+  for (const auto *Ctor : RD->ctors()) {
     if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
       return false;
+    if (!Ctor->isCopyOrMoveConstructor())
+      hasCtor = true;
+  }
 
-  return true;
+  return hasCtor;
 }
 
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits