Skip to content

Check macro expansion type - #25756

Merged
mbovel merged 2 commits into
scala:mainfrom
mbovel:mb/macros-expected-type
Jul 14, 2026
Merged

Check macro expansion type#25756
mbovel merged 2 commits into
scala:mainfrom
mbovel:mb/macros-expected-type

Conversation

@mbovel

@mbovel mbovel commented Apr 10, 2026

Copy link
Copy Markdown
Member

Macro-generated trees are type-checked internally but not with respect to the expected type, such that you can write the following and get a runtime exception:

// tests/neg-macros/expr-asInstanceOf/Macro_1.scala
import scala.quoted.*

object Macro:
  inline def foo(): Int =
    ${fooImpl()}

  def fooImpl()(using Quotes): Expr[Int] =
    import quotes.reflect.*
    Expr("hello").asInstanceOf[Expr[Int]]
// tests/neg-macros/expr-asInstanceOf/Test_2.scala
@main def test =
  println(Macro.foo()) // runtime error

This PR attempts to fix that by checking the generated-tree type with respect to the parent splice type.

@mbovel mbovel changed the title Check macro expansions with respect with an expected type Check macro expansions with an expected type Apr 10, 2026
Comment thread tests/pos-macros/i25692/Macro_1.scala Outdated
@mbovel
mbovel force-pushed the mb/macros-expected-type branch from 2309b50 to c2ac5fd Compare April 10, 2026 14:56
Comment thread compiler/src/dotty/tools/dotc/inlines/Inliner.scala
@mbovel mbovel changed the title Check macro expansions with an expected type Check macro expansion type Apr 10, 2026
@mbovel

mbovel commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Error in Monocle:

[error] -- Error: /home/runner/work/scala3/scala3/community-build/community-projects/Monocle/core/shared/src/test/scala-3/monocle/internal/IsoFieldsTest.scala:10:41 
[error] 10 |    val iso: Iso[Foo.type, EmptyTuple] = Iso.fields[Foo.type]
[error]    |                                         ^^^^^^^^^^^^^^^^^^^^
[error]    |Macro expansion has type monocle.Iso[Foo.type, EmptyTuple.type], which does not conform to the expected type monocle.PIso[Foo.type, Foo.type, Tuple, Tuple]
[error]    |----------------------------------------------------------------------------
[error]    |Inline stack trace
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |This location contains code that was inlined from IsoFields.scala:9
[error]  9 |    ${ IsoFieldsImpl.apply[S]('mirror) }
[error]    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |This location contains code that was inlined from IsoFields.scala:9
[error] 17 |      IsoFields[S]
[error]    |      ^^^^^^^^^^^^
[error]     ----------------------------------------------------------------------------
[error] one error found
[error] (coreJVM / Test / compileIncremental) Compilation failed
[error] Total time: 29 s, completed Apr 10, 2026, 4:11:55 PM

@mbovel

mbovel commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Right: type Iso[S, A] = PIso[S, S, A, A], but PIso is non-variant, so monocle.PIso[Foo.type, Foo.type, EmptyTuple.type, EmptyTuple.type] is not related to monocle.PIso[Foo.type, Foo.type, Tuple, Tuple].

See also test, def fields, object IsoField.

@mbovel

mbovel commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

The culprit is whitebox: https://github.com/optics-dev/Monocle/blob/d49981e4623f2100d2671b6ff47bc2aaa29e7af4/core/shared/src/main/scala-3/monocle/internal/IsoFields.scala#L16-L25. This is exactly the pattern we want to prevent, so the error is legitimate.

@mbovel
mbovel force-pushed the mb/macros-expected-type branch 4 times, most recently from ecee1ad to 7bb2c25 Compare April 10, 2026 21:56
@mbovel
mbovel force-pushed the mb/macros-expected-type branch from 7bb2c25 to 2b47054 Compare April 11, 2026 13:19
@mbovel

mbovel commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

Pragmatically, maybe we should first introduce this check only when-Xcheck-macros is passed, until all community-build libraries are fixed. I changed the code to do that.

@mbovel

mbovel commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

Ah, we do pass -Xcheck-macros to community build projects by default:

object SbtCommunityProject:
def scalacOptions = List(
"-Xcheck-macros",
"-Wsafe-init",
)

bishabosha added a commit that referenced this pull request Apr 14, 2026
Discussing with @bishabosha and further investigating, I realized the
warning I introduced yesterday in #25750 is wrong.

Code generated by macros is actually checked, even without
`-Xcheck-macros`, here:


https://github.com/scala/scala3/blob/20fc4c757617742af469c925d48beaf888f7ebfe/compiler/src/dotty/tools/dotc/inlines/Inliner.scala#L966-L967

The errors I am missing are due to these checks being run without an
expected type, which I'll try to address separately (#25756).

This PR reverts #25750, but still rephrase to avoid saying these are
_runtime_ checks.
@mbovel
mbovel force-pushed the mb/macros-expected-type branch from 2b47054 to 8e2dfc1 Compare May 6, 2026 12:36
@mbovel

mbovel commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Monocle PR: optics-dev/Monocle#1574.

@mbovel
mbovel force-pushed the mb/macros-expected-type branch from cd0c6c7 to 637ce99 Compare July 13, 2026 21:34
Bump the Monocle submodule to optics-dev/Monocle@2de8183 ("Remove
whitebox cast from GenIso.fields", scala#1602), which drops the whitebox
cast that is rejected once macro expansion types are checked.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mbovel
mbovel force-pushed the mb/macros-expected-type branch from 637ce99 to 323de5a Compare July 13, 2026 21:38
@mbovel
mbovel marked this pull request as ready for review July 14, 2026 08:56
@mbovel

mbovel commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@bishabosha the CI is now green. Monocle was the only error; I updated it upstream and in the community build. Do you still approve? 😄

@bishabosha
bishabosha self-requested a review July 14, 2026 09:29

@bishabosha bishabosha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks goood

@mbovel
mbovel merged commit f3355d6 into scala:main Jul 14, 2026
50 checks passed
@mbovel
mbovel deleted the mb/macros-expected-type branch July 14, 2026 09:40
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.

2 participants