Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to fix C++ code generation for default(T)/sequence initialization of importcpp types by avoiding untyped {} assignment expressions that can trigger ambiguous C++ assignment overload resolution.
Changes:
- Adds
genCppConstructorExprfor type-prefixed C++ construction expressions. - Updates
resetLocto use the new constructor expression for imported C++ types. - Adds a C++ regression test with an imported type that has ambiguous assignment overloads.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
compiler/cgen.nim |
Uses the new constructor-expression helper when resetting imported C++ types. |
compiler/ccgtypes.nim |
Adds genCppConstructorExpr alongside existing C++ initializer generation. |
tests/cpp/tcpp_default_ctor_assignment.nim |
Adds a regression test compiling newSeq for an imported C++ type. |
tests/cpp/tcpp_default_ctor_assignment.h |
Defines the C++ type with ambiguous assignment overloads used by the test. |
Comments suppressed due to low confidence (2)
compiler/ccgtypes.nim:715
- Using a parenthesized constructor expression here changes C++ overload resolution from the braced initialization that
genCppInitializeruses elsewhere. A typed braced construction expression would still disambiguate the RHS of assignments, while preserving initializer-list/default-list initialization semantics for imported C++ types that rely onstd::initializer_listoverloads or are only list-initializable.
let k = lastSon(n[i])
compiler/ccgtypes.nim:714
- The assertion message contains a typo: “doesnt” should be “doesn't”.
of nkOfBranch, nkElse:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #25803
This pull request introduces a new approach for generating C++ constructor expressions in the Nim compiler's C++ backend, ensuring that type-prefixed construction is used when needed (such as in assignments), rather than just braced initializer lists. It also adds a new test case (with corresponding C++ header) to verify correct behavior when assigning to types with overloaded assignment operators and constructors.
C++ code generation improvements:
genCppConstructorExprinccgtypes.nim, which generates a full type-prefixed constructor expression (e.g.,Foo(a, b)) for C++ code generation, as opposed to just a braced initializer list. This is important for contexts like assignments where the type must be explicit.resetLocincgen.nimto usegenCppConstructorExprinstead ofgenCppInitializerwhen initializing imported C++ types, ensuring correct code generation for assignments.Testing:
tcpp_default_ctor_assignment.h, defining a structAmbiguousAssignwith overloaded assignment operators and constructors to test ambiguous assignment scenarios.tcpp_default_ctor_assignment.nim, which exercises construction and assignment for the imported C++ type, ensuring the new code generation logic works as intended.