[clang] Revert "[DebugInfo] Ignore undefined constexpr constructors in constructor homing." (PR #221566)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Avi Kivity (avikivity)

<details>
<summary>Changes</summary>


This reverts commit 6ba0802b406e0963720a698702b9136578dde149 (#<!-- -->218165).

canUseCtorHoming() used to bail out on hasConstexprNonCopyMoveConstructor(). That commit narrowed the exemption to *defined* constexpr constructors, on the grounds that "declared constexpr constructors are not callable in a TU that doesn't see their definition". That premise does not hold:

  - A constexpr constructor that is only declared can be called outside a constant-expression context, with the definition supplied by another translation unit; clang emits an ordinary external call for it.

  - More importantly, whether the class can be constructed here is not the right question. A class whose constructor is never odr-used at all need not have that constructor defined anywhere in the program, yet the type can still be required to be complete:

        struct box {
            unsigned long v;
            constexpr box(unsigned long t);   // declared, never defined
        };
        unsigned long f(box* p) { return p->v; }

    Nothing homes box, so no translation unit emits its definition and the member is unreachable from a debugger, even though f() reads through it.

This showed up in ScyllaDB as gdb reporting "<incomplete type>" for std::pair<const K, V> inside absl::container_internal::map_slot_type - every libstdc++ std::pair constructor is constexpr, and a pair that only ever appears as a union member never has one instantiated - which broke the flat_hash_map readers in its gdb pretty-printing scripts.

Keep the test coverage the reverted commit added for cases whose expected output is unchanged, correct the DeclaredConstexpr expectation back to full debug info, and add the case above, which needs no odr-use of the constructor at all.

Fixes https://github.com/llvm/llvm-project/issues/221560

---
Full diff: https://github.com/llvm/llvm-project/pull/221566.diff


2 Files Affected:

- (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+5-15) 
- (modified) clang/test/DebugInfo/CXX/limited-ctor.cpp (+13-2) 


``````````diff
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 15080e5e47b3e..02864621d60a3 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3239,17 +3239,11 @@ static bool canUseCtorHoming(const CXXRecordDecl *RD) {
   if (isClassOrMethodDLLImport(RD))
     return false;
 
-  if (RD->isLambda() || RD->isAggregate() || RD->hasTrivialDefaultConstructor())
+  if (RD->isLambda() || RD->isAggregate() ||
+      RD->hasTrivialDefaultConstructor() ||
+      RD->hasConstexprNonCopyMoveConstructor())
     return false;
 
-  // Skip this optimization if the class has an implicit constexpr default
-  // constructor, since those constructors can be invoked without emitting type
-  // information for the constructor.
-  if (RD->needsImplicitDefaultConstructor() &&
-      RD->defaultedDefaultConstructorIsConstexpr())
-    return false;
-
-  bool HasNonDeletedCtor = false;
   for (const CXXConstructorDecl *Ctor : RD->ctors()) {
     if (Ctor->isCopyOrMoveConstructor())
       continue;
@@ -3260,15 +3254,11 @@ static bool canUseCtorHoming(const CXXRecordDecl *RD) {
       // copy/move constructor, which does not enable homing.
       if (CtorDef->isDelegatingConstructor())
         continue;
-      // Skip this optimization if we see a defined constexpr constructor, which
-      // can be invoked without emitting type info.
-      if (Ctor->isConstexpr() && !Ctor->isDeleted())
-        return false;
     }
     if (!Ctor->isDeleted())
-      HasNonDeletedCtor = true;
+      return true;
   }
-  return HasNonDeletedCtor;
+  return false;
 }
 
 static bool shouldOmitDefinition(llvm::codegenoptions::DebugInfoKind DebugKind,
diff --git a/clang/test/DebugInfo/CXX/limited-ctor.cpp b/clang/test/DebugInfo/CXX/limited-ctor.cpp
index 613faa11ffad8..04e2986e9daef 100644
--- a/clang/test/DebugInfo/CXX/limited-ctor.cpp
+++ b/clang/test/DebugInfo/CXX/limited-ctor.cpp
@@ -27,12 +27,23 @@ struct E {
   constexpr E(){};
 } TestE;
 
-// Declared but not defined constexpr constructor should not emit full debug info..
-// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "DeclaredConstexpr"{{.*}}flags: DIFlagFwdDecl
+// A constexpr constructor that is only declared here may still be defined
+// elsewhere and called, so it cannot be relied on to home the type.
+// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "DeclaredConstexpr"{{.*}}DIFlagTypePassByValue
 struct DeclaredConstexpr {
   constexpr DeclaredConstexpr();
 } TestDeclaredConstexpr;
 
+// A declared-only constexpr constructor that is never odr-used need not be
+// defined anywhere in the program, so nothing would ever emit the definition
+// of the type - but the type is still required to be complete here.
+// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "ConstexprDeclaredOnly"{{.*}}DIFlagTypePassByValue
+struct ConstexprDeclaredOnly {
+  unsigned long v;
+  constexpr ConstexprDeclaredOnly(unsigned long t);
+};
+unsigned long ReadConstexprDeclaredOnly(ConstexprDeclaredOnly *p) { return p->v; }
+
 // Defined out-of-line constexpr constructor should emit full debug info.
 // CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "OutOfLineConstexpr"{{.*}}DIFlagTypePassByValue
 struct OutOfLineConstexpr {

``````````

</details>


https://github.com/llvm/llvm-project/pull/221566