-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Backport "Prevent opaque types leaking from transparent inline methods" to 3.7.4 #24002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
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
@WojciechMazur
WojciechMazur
force-pushed
the
release-3.7.4_backport-23927
branch
from
September 21, 2025 16:00
7074f7a
to
e78c54c
Compare
Before this PR, every transparent inline call returning an opaque type would actually be typed with an intersection type `DECLARED & ACTUAL`, where `DECLARED` was the declared return type of the transparent method, and `ACTUAL` was the type actually returned in the expansion, with every opaque type alias then dealiased. There was no way to guard against this dealiasing. With the changes in this PR, users are now able to manually ensure that they receive the types they want, although they might have to manually annotate the returned type inside of the transparent inline method body (as described in the added documentation section). The previous dealiasing was caused by the proxy mechanism in inlining, which would effectively deals every opaque type, that is transparent from the perspective of the original method declaration. Now, we try to map the results of the transparent inline back to the original (opaque) types. However all of this is only true for the outermost transparent inline method calls. Nested calls will not be affected by this change. This is because the type checker in the original context of the method will see the opaque type as transparent (so it will type the rest of the method according to that), and that typing must still hold after inlining the method e.g.: ``` object Time: opaque type Time = String transparent inline makeTime(): Time = "1h" transparent inline listTime(): List[Time] = List[String](makeTime()) // mapping the results of makeTime() back into opaque types outside // of the scope of Time will cause an inlining compilation error // (which we are generally trying to avoid, and which would be // considered a bug in the compiler). ``` This might cause the aliased type to still leak in a manner that may feel unexpected. In the above example, even if the List does not have an explicit type parameter, the type inference will still decide on `String`, causing any call to listTime to leak that type. This is also touched upon in the added docs. This PR might cause some source/library incompatibilities connected to the changed returned types (but I doubt it’s many, considering the additional required effort of ignoring type inference if we want the outputted type to be different). [Cherry-picked c6245ed]
Certain macros could return nodes typed with incorrect ThisTypes, which would reference module types outside of their scope. We remap those problematic nodes to TermRefs pointing to objects, and then possibly manually cast the returned node to the remapped type, as the ensureConforms method would just leave the previous incorrect type after confirming that the remapped type is a subtype of the previous incorrect one. [Cherry-picked 6771a79]
[Cherry-picked e611110]
@WojciechMazur
WojciechMazur
force-pushed
the
release-3.7.4_backport-23792
branch
from
September 21, 2025 16:01
07c0e16
to
503232a
Compare
This change introduced regression in dfianthdl/dfhdl
- it fails for different reasons:
- due to Backport "Only check seen for LazyRef for TypeSizeAccumulator" to 3.7.4 #24003 - https://github.com/VirtusLab/community-build3/actions/runs/17892555871/job/50875048278
- this PR crash - https://github.com/VirtusLab/community-build3/actions/runs/17899484520/job/50890813020
- in nightly - https://github.com/VirtusLab/community-build3/actions/runs/17871701239/job/50827359406
Base automatically changed from
release-3.7.4_backport-23927
to
release-3.7.4
September 22, 2025 10:18
Seems like this one was not included in 3.7.4-RC1
, does that mean we are going to have 3.7.4-RC2
?
It was not included becouse it introduced a regression. We might have RC2 anyway later - I'd like to backport some changes around -Wunused
that are not yet merged.
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.
Backports #23792 to the 3.7.4.
PR submitted by the release tooling.
[skip ci]