Skip to content

Collect checks to run in CrossVersionChecks #16037

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

som-snytt
Copy link
Contributor

Check for deprecation when inlining or constant folding, but avoid cyclic errors by deferring the exclusion check, which walks the owner chain, until CrossVersionChecks, where the check normally happens.

Fixes #2968

Check for deprecation when inlining or constant folding,
but avoid cyclic errors by deferring the exclusion check,
which walks the owner chain, until CrossVersionChecks,
where the check normally happens.
@som-snytt som-snytt force-pushed the issue/2968-deprecated-folded-ops branch from 6b65711 to 6cdeb38 Compare September 17, 2022 20:34
@som-snytt som-snytt marked this pull request as ready for review September 17, 2022 21:56
@odersky
Copy link
Contributor

odersky commented Sep 19, 2022

Is it reallly #2968 that is fixed here?

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

I'd like to understand the problem better before passing judgment.

At first glance, this looks like a lot of complications for a relatively small gain, so overall I am pretty down on this.


private val initialStore = store10
private val initialStore = store12
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding something to the store is an expensive operation. It charges every time we copy a store with two additional slots to copy. if we look up the property not very often it's better to use a Property.

@odersky odersky assigned som-snytt and unassigned odersky Sep 19, 2022
@som-snytt
Copy link
Contributor Author

Scala 2 has a similar but simpler mechanism just to run functions after typer, with the same goal of avoiding cycles.

This could just be tacked onto CrossVersionChecks, more tightly coupled but also sufficient.

@odersky
Copy link
Contributor

odersky commented Sep 19, 2022

Yes, why don't we do this in cross version checks? That's where the other tests are done, right?

@som-snytt
Copy link
Contributor Author

som-snytt commented Sep 19, 2022

Constant folded ops currently disappear. Instead of trees preserved and constant types assigned.

Edit: also inlining.

@odersky
Copy link
Contributor

odersky commented Sep 19, 2022

What is an op that is constant folded and deprecated? Is there anything that's realistic in practice?

@odersky
Copy link
Contributor

odersky commented Sep 19, 2022

The other possibility is to order phases differently. We can move CrossVersionChecks earlier (with a bit of massaging) or drop trees of constant types later (currently done in FirstTransform).

@som-snytt
Copy link
Contributor Author

The int-shift-long is deprecated; and the trick is just to avoid warning if an enclosing element is deprecated. Inlining opens the door to anything might be deprecated. (I also managed to trigger deprecation at the "inlined body retainer" member. There is a test that would normally trigger on experimental that way.)

This use case is not urgent, so I'll take some time to explore options.

(The ability to defer arbitrary checks is useful in Scala 2 to break out of the "ball of mud", but probably less useful here. I see Scala 2 has a DeferredRefCheck tree.)

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.

print deprecation warning for bitshift Int by a Long
2 participants