Skip to content
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

Fix callTrace of inlined methods #18738

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Oct 20, 2023

We need to keep the reference to the called method, not only the symbol
of the to level class. This is important for the traces of the assert
method that is defined in a different file. This might also be useful
for macro annotations.

This is also a solution to the awkward Select vs. Ident distinction to
identify macros in YCheckPositions.

@nicolasstucki nicolasstucki changed the title Fix callTrace if inlined methods Fix callTrace of inlined methods Oct 20, 2023
@nicolasstucki nicolasstucki self-assigned this Oct 20, 2023
@nicolasstucki nicolasstucki force-pushed the fix-source-file-of-inlined-assert-with-scala2-library-tasty branch 7 times, most recently from 26669c9 to 2134515 Compare October 24, 2023 07:43
@nicolasstucki nicolasstucki added the needs-minor-release This PR cannot be merged until the next minor release label Oct 24, 2023
@nicolasstucki nicolasstucki marked this pull request as ready for review October 24, 2023 12:03
@nicolasstucki nicolasstucki force-pushed the fix-source-file-of-inlined-assert-with-scala2-library-tasty branch 2 times, most recently from 5ad2eaf to a46bed4 Compare October 24, 2023 12:20
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Oct 31, 2023
This test exists for historical reasons. Now we compile the Scala 2
library in the `scala2-library-bootstrapped` project. This one checks
that it compiles (with `-Ycheck:all`) and is used to package the TASTy
files in the `scala2-library-tasty` project. Furthermore, now there is
a `scala/scala` test that checks that the library can be compiled with
Scala 3.

This test is fundamentally broken but we have not noticed it because we
do not use `-Ycheck:all`. The reason is that when we compile `scala.Predef`
we get a conflict with `scala.runtime.stdLibPatches.Predef` when we patch
the symbols. This also surfaced in scala#18738.
nicolasstucki added a commit that referenced this pull request Nov 1, 2023
This test exists for historical reasons. Now we compile the Scala 2
library in the `scala2-library-bootstrapped` project. This one checks
that it compiles (with `-Ycheck:all`) and is used to package the TASTy
files in the `scala2-library-tasty` project. Furthermore, now there is a
`scala/scala` test that checks that the library can be compiled with
Scala 3.

This test is fundamentally broken but we have not noticed it because we
do not use `-Ycheck:all`. The reason is that when we compile
`scala.Predef` we get a conflict with
`scala.runtime.stdLibPatches.Predef` when we patch the symbols. This
also surfaced in #18738.
We need to keep the reference to the called method, not only the symbol
of the to level class. This is important for the traces of the `assert`
method that is defined in a different file. This might also be useful
for macro annotations.

This is also a solution to the awkward Select vs. Ident distinction to
identify macros in `YCheckPositions`.
@nicolasstucki nicolasstucki force-pushed the fix-source-file-of-inlined-assert-with-scala2-library-tasty branch from a46bed4 to eb0c408 Compare November 1, 2023 08:58
@odersky odersky merged commit f61026d into scala:main Nov 6, 2023
18 checks passed
@odersky odersky deleted the fix-source-file-of-inlined-assert-with-scala2-library-tasty branch November 6, 2023 20:48
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants