Skip to content

Fix Issue #22752: Force semantic analysis of fields before is() conversion check#22760

Merged
dkorpel merged 1 commit intodlang:masterfrom
oCHIKIo:fix-22752
Mar 20, 2026
Merged

Fix Issue #22752: Force semantic analysis of fields before is() conversion check#22760
dkorpel merged 1 commit intodlang:masterfrom
oCHIKIo:fix-22752

Conversation

@oCHIKIo
Copy link
Contributor

@oCHIKIo oCHIKIo commented Mar 18, 2026

Fix Issue #22752:
is() expressions should force semantic analysis on forward-referenced structs before conversion checks.

The Problem: Currently, is(const(S) : S) can incorrectly return true if it is evaluated before S's fields have been populated (while the struct is still "opaque" or in a forward-reference state). In this state, the compiler has not yet "seen" any fields, so it assumes the struct is empty and decides that an implicit conversion is valid. This leads to static assert failures later in the compilation when the same condition is re-evaluated after the fields are actually read.

The Fix: I have updated implicitConvToWithoutAliasThis in dmd.typesem to explicitly call from.sym.size(Loc.initial). This forces the compiler to perform field analysis and determine the struct's layout before making a final decision on the implicit conversion.

Verification:

Bug Reproduction: The test case provided in the issue now compiles and passes successfully.
Internal Consistency: All modules passed dmd-unittest, ensuring no regressions in the compiler's core logic.
ABI Safety: High. This change only affects the internal reasoning of the compiler during semantic analysis and does not change the generated mangling or binary layout.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 18, 2026

Please add the test case to the test suite

@oCHIKIo
Copy link
Contributor Author

oCHIKIo commented Mar 18, 2026

Please add the test case to the test suite

Added the requested runnable test case to the test suite and amended the commit.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 18, 2026

Can you make it a compilable test? Runnable tests are significantly slower, and the main function doesn't do anything here.

@oCHIKIo
Copy link
Contributor Author

oCHIKIo commented Mar 18, 2026

done

@dkorpel
Copy link
Contributor

dkorpel commented Mar 18, 2026

Also it should be the exact code that caused the issue, the current test case already worked before. I think you can copy-paste the snippet from the issue into the compilable folder.

@oCHIKIo
Copy link
Contributor Author

oCHIKIo commented Mar 18, 2026

I have updated implicitConvToWithoutAliasThis in dmd.typesem to call from.sym.size(Loc.initial). this forces the compiler to finish field analysis and determine the struct's layout before checking the implicit conversion, which fixes the issues with is() expressions on forward-referenced structs.
My initial test case passed even without the fix, but I have now confirmed this one correctly fails without it and passes with it.

Comment on lines +9227 to +9228
import dmd.dsymbolsem : size;
from.sym.size(Loc.initial);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the return result be checked in case an error occurred and it returns invalid size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch. I've updated the code to check the return value if size() returns SIZE_INVALID, we now return MATCH.nomatch early

@oCHIKIo
Copy link
Contributor Author

oCHIKIo commented Mar 20, 2026

The two failing checks are both macOS 14 x64 runners. All other platforms (Windows x86/x64, Ubuntu x86/x64, FreeBSD, Alpine, CircleCI, Buildkite) passed successfully. This looks like a macOS CI infrastructure issue unrelated to my change. Could a maintainer re-run the macOS jobs?

@dkorpel dkorpel merged commit 3ad7803 into dlang:master Mar 20, 2026
53 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants