Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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
WojciechMazur wants to merge 3 commits into release-3.7.4 from release-3.7.4_backport-23792

Conversation

Copy link
Contributor

@WojciechMazur WojciechMazur commented Sep 20, 2025

Backports #23792 to the 3.7.4.

PR submitted by the release tooling.
[skip ci]

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]
Base automatically changed from release-3.7.4_backport-23927 to release-3.7.4 September 22, 2025 10:18
Copy link
Member

Seems like this one was not included in 3.7.4-RC1, does that mean we are going to have 3.7.4-RC2?

Copy link
Contributor Author

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.

hamzaremmal reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /