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

Show inlined positions with source code #14427

Merged
merged 4 commits into from
Feb 8, 2022

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Feb 7, 2022

Re-enable #14002. Fix issue when inlined position sources are not available.

Fixes #13991
Also fixes #14428

It might have been disabled by mistake because bootstrapCmdTests was
assumed to be the set of tests for the bootstrapped compiler.
The second is an extension for the second with tests that can only
be run on the bootstrapped compiler. It is now renamed to
bootstrappedOnlyCmdTests to make sure we avoid this potential confusion.

Fixes scala#14428
@deusaquilus
Copy link
Contributor

deusaquilus commented Feb 7, 2022

I'm just curious. Is this further work needed to resolve #13991?

@smarter
Copy link
Member

smarter commented Feb 7, 2022

@deusaquilus the fix to #13991 had to be reverted because it broke the CI (cf #14406), this PR re-enables it.

@nicolasstucki
Copy link
Contributor Author

The CI issue is fixed in 733af41 and we ensure it won't pass the PR CI tests again in 20e7269.

@nicolasstucki nicolasstucki marked this pull request as ready for review February 8, 2022 06:53
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

@@ -56,6 +56,7 @@ cp tests/neg/i6371/B_2.scala $OUT/B.scala
"$SBT" "scalac $OUT/A.scala -d $OUT1"
rm $OUT/A.scala
"$SBT" "scalac -classpath $OUT1 -d $OUT1 $OUT/B.scala" > "$tmp" 2>&1 || echo "ok"
cat "$tmp" # for debugging
Copy link
Member

Choose a reason for hiding this comment

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

Could you comment that out or only display it in case the test doesn't pass? Otherwise we get some error output in the CI that could be misinterpreted as a test failure.

Copy link
Contributor Author

@nicolasstucki nicolasstucki Feb 8, 2022

Choose a reason for hiding this comment

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

Commented out

@smarter smarter assigned nicolasstucki and unassigned smarter Feb 8, 2022
@nicolasstucki nicolasstucki merged commit 48d5747 into scala:main Feb 8, 2022
@nicolasstucki nicolasstucki deleted the reenable-inline-positions branch February 8, 2022 14:09
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cmdTests are not executed on test Incorrect position of error reported from macro in inline methods chain
4 participants