Skip to content

Conversation

@smeenai
Copy link
Collaborator

@smeenai smeenai commented Oct 29, 2024

We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both CIRGenerator::HandleInlineFunctionDefinition and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes #991

We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both `CIRGenerator::HandleInlineFunctionDefinition` and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes llvm#991
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@bcardosolopes bcardosolopes merged commit b4d4611 into llvm:main Oct 30, 2024
6 checks passed
lanza pushed a commit that referenced this pull request Nov 5, 2024
We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both `CIRGenerator::HandleInlineFunctionDefinition` and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes #991
lanza pushed a commit that referenced this pull request Mar 18, 2025
We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both `CIRGenerator::HandleInlineFunctionDefinition` and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes #991
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both `CIRGenerator::HandleInlineFunctionDefinition` and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes llvm#991
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.

error: definition with same mangled name on method in namespace

2 participants