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

Reset comparersInUse to zero in ContextState.reset #18915

Merged
merged 2 commits into from
Nov 14, 2023

Conversation

Linyxus
Copy link
Contributor

@Linyxus Linyxus commented Nov 14, 2023

Fixes #18855

The code in #18855 used to crash the compiler with an out-of-bound access exception. It was because, previously, ContextState.reset clears the comparers, the cached pool of TypeComparers, while keeping the variable comparersInUse intact. However, the comparer method of a context assumes comparers has a size greater than or equal to comparersInUse. So the invariant is clearly violated here and thus triggers the crash.

This PR proposes a straightforward fix. Yet, the reason why only under both macros and cc this code path will be triggered remains to be investigated.

def impl()(using Quotes): Expr[Unit] = '{()}
inline def run(): Unit = ${impl()}


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@@ -1063,6 +1063,7 @@ object Contexts {
sources.clear()
files.clear()
comparers.clear() // forces re-evaluation of top and bottom classes in TypeComparer
comparersInUse = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

For additional context, this needs to be reset because the compilation unit was suspended due to the macro dependency. We need to set it back to 0 each time we restart compilation after the suspension.

@@ -0,0 +1,3 @@
import scala.language.experimental.captureChecking
val x = run()

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change


def impl()(using Quotes): Expr[Unit] = '{()}
inline def run(): Unit = ${impl()}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

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.

Well spotted!

@odersky odersky merged commit 0cd94fc into scala:main Nov 14, 2023
18 checks passed
@odersky odersky deleted the fix-18855 branch November 14, 2023 09:46
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Even simple macros do not expand with capture checking
4 participants