Add add informative scope extrusion check

[nicolasstucki]

Some tests had to change because they temporarily extruded some references before using them in the correct scope. This showed places where we missed a given Quotes within a splice.

[nicolasstucki]

4 |  test2() // error
  |  ^^^^^^^
  |Exception occurred while executing macro expansion.
  |scala.quoted.runtime.impl.ScopeException: Expression created in a splice was used outside of that splice.
  |Created in: tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 55
  |Used in: tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 5
  |Expression: y
  |
  |
  |Creation stack:
  |     tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 55
  |     tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 5
  |     tests/neg-macros/scope-extrusion/Macro_1.scala:14 at column 27
  |
  |
  |Use stack:
  |     tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 5
  |     tests/neg-macros/scope-extrusion/Macro_1.scala:14 at column 27

[liufengyun]

Regarding the message:

4 |  test2() // error
  |  ^^^^^^^
  |Exception occurred while executing macro expansion.
  |scala.quoted.runtime.impl.ScopeException: Expression created in a splice was used outside of that splice.
  |Created in: tests/neg-macros/scope-extrusion/Macro_1.scala:17 at column 55

As the compiler detects some potential errors in user code, maybe avoid saying there are exceptions that occurred in the compiler. Might be straight-forward to point out what's possibly wrong in user's code.

[liufengyun]

It seems to me that the need to thread through Quotes might be a source of errors in practice. It's not obvious why it's necessary.

The need might come from staging. As macros are much more widely used than staging, I think it's worth optimizing more for macros.

Is it possible to achieve the same goal without complicating the user interface (I mean with only one Quotes)? That would be a big contribution.

[nicolasstucki]

It seems to me that the need to thread through Quotes might be a source of errors in practice. It's not obvious why it's necessary.

This would have been avoided if we could have made Expr path dependent on the instance of Quotes. Because we do not have this we need to do runtime checks to catch the cases that did leak an expression outside of a splice.

Even if we then un-extrude the quote, there is a performance penalty for doing it the wrong way as the owners might be wrong and would need to be changed. With this check we ensure that we have the correct ones at creation.

If we ensure that we have the correct context we will also be able to rely on it to perform stronger yChecks in the reflection API. Instead of letting the error pass and be detected by the global yCheck.

The need might come from staging. As macros are much more widely used than staging, I think it's worth optimizing more for macros.

It does not come from staging. Here it came due to a badly designed API. eqSumBody should have been a def from the start. Probably it shouldn't be a local method as well.

Is it possible to achieve the same goal without complicating the user interface (I mean with only one Quotes)? That would be a big contribution.

The point of adding the Quotes is to use a single Quotes instance to crate 'x/'y in ${eqSumBody('x, 'y)}) and use the same instance in eqSumBody to create '{ $ordx == $ordy && ... }. This would be clearer if the method was not defined locally.

Or simply write it without the method as

val elements = Expr.ofList(elemInstances)
'{
  eqSum((x: T, y: T) => {
    $m.ordinal(x) == $m.ordinal(y) && 
    $elements.apply($m.ordinal(x)).asInstanceOf[Eq[Any]].eqv(x, y)
  })
}

The original code looks messy because it was not well designed.

[nicolasstucki]

I also removed the old scopeId checks and they can be subsumed by these checks.

[nicolasstucki]

This runtime check also found that this abstraction that we have used quite a few times was fundamentally broken. We put the Quotes in the wrong place which caused a potential change of owners after the creation of the quote.

[liufengyun]

Might be good to think

• how to teach this subtlety to avoid such errors, or
• eliminate such subtleties from the framework (if possible)

[nicolasstucki]

As the compiler detects some potential errors in user code, maybe avoid saying there are exceptions that occurred in the compiler.

As this is a yCheck failure it fails with an exception. This is intended for the macro authors when they are yChecking their implementation. The error states that it was an exception while executing macro expansion which should be clear to them when he get this error under yCheck.

Might be straight-forward to point out what's possibly wrong in user's code.

Not sure how much we can do at this stage. This is already the runtime of the macro and we do not have the code of the macro explicitly available to point errors. The best we can do is provide as much information as possible of the context where the quote was created and where it was used.

A common error is a missing quotes argument to a function. The error message could give a hint about that.

[liufengyun]

LGTM

The purpose of preventing scope extrusion is exactly avoiding accidental mistakes. The optional using Quotes parameter only defers it to the programmers -- but it's still error-prone.

I see this is a hard problem (safety & usability). As you mentioned, one possibility to fix the issue is to use path-dependent types.

Another possibility to fix the problem is to create a compiler plugin/phase to check that some types (such as Quotes) are never captured and have to be passed explicitly.

However, those improvements are beyond the scope of this PR, they can be discussed further and addressed in the future when better ideas emerge.

[liufengyun]

What about isOuterScopeOf -> contains?

[nicolasstucki]

I avoided contains because it can be misinterpreted. Even I made a couple of mistakes and inverted the arguments when it was called contains.