Skip to content

Conversation

@lanza
Copy link
Member

@lanza lanza commented Nov 22, 2025

Stack from ghstack (oldest at bottom):

This patch fixes a critical assertion failure that occurred when accessing
fields in records that use Empty Base Optimization (EBO), particularly
affecting std::unique_ptr, std::shared_ptr, and other standard library types.

Problem:

When processing classes with empty member types combined with inheriting
constructors, CIRGenRecordLayout::getCIRFieldNo() would assert with:
"Assertion `FieldInfo.count(FD) && "Invalid field for record!"' failed"

This occurred because EBO transforms empty-type members into base classes
rather than fields, so they don't exist in the FieldInfo map. The bug was
triggered by the combination of three conditions:

  1. A member with an empty struct type (e.g., std::default_delete)
  2. Inheriting constructors (using Base::Base)
  3. Instantiation of the inherited constructor

Root Cause:

std::unique_ptr internally uses std::tuple<T*, default_delete>, where
default_delete is an empty struct. When std::tuple applies EBO, the empty
deleter is stored as a base class instead of a member field. During code
generation for constructors, CIR tried to initialize this "field" but it
didn't exist in the FieldInfo map, causing an assertion failure.

Solution:

Following the traditional CodeGen pattern from clang/lib/CodeGen, this patch:

  1. Adds containsFieldDecl() method to CIRGenRecordLayout (matching
    CGRecordLayout::containsFieldDecl) to check if a field exists in the
    layout before attempting to access it.

  2. Adds guard checks before getCIRFieldNo() calls in:

    • CIRGenExpr.cpp::emitLValueForField() - handles field access
    • CIRGenExpr.cpp::emitLValueForFieldInitialization() - handles
      initialization of reference fields
    • CIRGenExprConst.cpp::emitNullConstant() - handles null initialization
    • CIRGenExpr.cpp::emitAddrOfFieldStorage() - removes redundant call
  3. When a field doesn't exist in the layout (EBO case), returns appropriate
    fallback values instead of asserting.

Impact:

This fix enables the following previously-broken STL types:

  • std::unique_ptr
  • std::shared_ptr
  • std::function
  • std::array<T, N>
  • std::any
  • std::thread

Testing:

Added clang/test/CIR/CodeGen/ebo-tuple.cpp which tests the specific
scenario that triggered this bug: empty member types with inheriting
constructors.

Verified with:

  • Minimal 24-line reproduction case
  • std::unique_ptr default construction
  • std::shared_ptr default construction

This implementation strictly follows the traditional CodeGen approach from
clang/lib/CodeGen/CGRecordLayout.h and clang/lib/CodeGen/CGExpr.cpp to
maintain consistency with LLVM CodeGen.

[ghstack-poisoned]
lanza added a commit that referenced this pull request Nov 22, 2025
…mization

This patch fixes a critical assertion failure that occurred when accessing
fields in records that use Empty Base Optimization (EBO), particularly
affecting std::unique_ptr, std::shared_ptr, and other standard library types.

**Problem:**

When processing classes with empty member types combined with inheriting
constructors, CIRGenRecordLayout::getCIRFieldNo() would assert with:
  "Assertion `FieldInfo.count(FD) && \"Invalid field for record!\"' failed"

This occurred because EBO transforms empty-type members into base classes
rather than fields, so they don't exist in the FieldInfo map. The bug was
triggered by the combination of three conditions:
1. A member with an empty struct type (e.g., std::default_delete<T>)
2. Inheriting constructors (using Base::Base)
3. Instantiation of the inherited constructor

**Root Cause:**

std::unique_ptr internally uses std::tuple<T*, default_delete<T>>, where
default_delete is an empty struct. When std::tuple applies EBO, the empty
deleter is stored as a base class instead of a member field. During code
generation for constructors, CIR tried to initialize this "field" but it
didn't exist in the FieldInfo map, causing an assertion failure.

**Solution:**

Following the traditional CodeGen pattern from clang/lib/CodeGen, this patch:

1. Adds containsFieldDecl() method to CIRGenRecordLayout (matching
   CGRecordLayout::containsFieldDecl) to check if a field exists in the
   layout before attempting to access it.

2. Adds guard checks before getCIRFieldNo() calls in:
   - CIRGenExpr.cpp::emitLValueForField() - handles field access
   - CIRGenExpr.cpp::emitLValueForFieldInitialization() - handles
     initialization of reference fields
   - CIRGenExprConst.cpp::emitNullConstant() - handles null initialization
   - CIRGenExpr.cpp::emitAddrOfFieldStorage() - removes redundant call

3. When a field doesn't exist in the layout (EBO case), returns appropriate
   fallback values instead of asserting.

**Impact:**

This fix enables the following previously-broken STL types:
- std::unique_ptr<T>
- std::shared_ptr<T>
- std::function<Sig>
- std::array<T, N>
- std::any
- std::thread

**Testing:**

Added clang/test/CIR/CodeGen/ebo-tuple.cpp which tests the specific
scenario that triggered this bug: empty member types with inheriting
constructors.

Verified with:
- Minimal 24-line reproduction case
- std::unique_ptr<int> default construction
- std::shared_ptr<int> default construction

This implementation strictly follows the traditional CodeGen approach from
clang/lib/CodeGen/CGRecordLayout.h and clang/lib/CodeGen/CGExpr.cpp to
maintain consistency with LLVM CodeGen.


ghstack-source-id: f433d7e
Pull-Request: #1999
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@lanza lanza closed this Nov 23, 2025
lanza added a commit that referenced this pull request Nov 24, 2025
…mization

This patch fixes a critical assertion failure that occurred when accessing
fields in records that use Empty Base Optimization (EBO), particularly
affecting std::unique_ptr, std::shared_ptr, and other standard library types.

**Problem:**

When processing classes with empty member types combined with inheriting
constructors, CIRGenRecordLayout::getCIRFieldNo() would assert with:
  "Assertion `FieldInfo.count(FD) && \"Invalid field for record!\"' failed"

This occurred because EBO transforms empty-type members into base classes
rather than fields, so they don't exist in the FieldInfo map. The bug was
triggered by the combination of three conditions:
1. A member with an empty struct type (e.g., std::default_delete<T>)
2. Inheriting constructors (using Base::Base)
3. Instantiation of the inherited constructor

**Root Cause:**

std::unique_ptr internally uses std::tuple<T*, default_delete<T>>, where
default_delete is an empty struct. When std::tuple applies EBO, the empty
deleter is stored as a base class instead of a member field. During code
generation for constructors, CIR tried to initialize this "field" but it
didn't exist in the FieldInfo map, causing an assertion failure.

**Solution:**

Following the traditional CodeGen pattern from clang/lib/CodeGen, this patch:

1. Adds containsFieldDecl() method to CIRGenRecordLayout (matching
   CGRecordLayout::containsFieldDecl) to check if a field exists in the
   layout before attempting to access it.

2. Adds guard checks before getCIRFieldNo() calls in:
   - CIRGenExpr.cpp::emitLValueForField() - handles field access
   - CIRGenExpr.cpp::emitLValueForFieldInitialization() - handles
     initialization of reference fields
   - CIRGenExprConst.cpp::emitNullConstant() - handles null initialization
   - CIRGenExpr.cpp::emitAddrOfFieldStorage() - removes redundant call

3. When a field doesn't exist in the layout (EBO case), returns appropriate
   fallback values instead of asserting.

**Impact:**

This fix enables the following previously-broken STL types:
- std::unique_ptr<T>
- std::shared_ptr<T>
- std::function<Sig>
- std::array<T, N>
- std::any
- std::thread

**Testing:**

Added clang/test/CIR/CodeGen/ebo-tuple.cpp which tests the specific
scenario that triggered this bug: empty member types with inheriting
constructors.

Verified with:
- Minimal 24-line reproduction case
- std::unique_ptr<int> default construction
- std::shared_ptr<int> default construction

This implementation strictly follows the traditional CodeGen approach from
clang/lib/CodeGen/CGRecordLayout.h and clang/lib/CodeGen/CGExpr.cpp to
maintain consistency with LLVM CodeGen.


ghstack-source-id: 5d2b410
Pull-Request: #1999
@lanza lanza reopened this Nov 24, 2025
[ghstack-poisoned]
@lanza lanza marked this pull request as draft November 24, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants