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

inline needed to resolve weird compilation issue #14442

Closed
informarte opened this issue Feb 9, 2022 · 2 comments · Fixed by #14480
Closed

inline needed to resolve weird compilation issue #14442

informarte opened this issue Feb 9, 2022 · 2 comments · Fixed by #14480

Comments

@informarte
Copy link

informarte commented Feb 9, 2022

Compiler version

3.1.2-RC1

Problem description

I am having a weird compilation issue that can be resolved by inlining a seemingly unrelated method.

Minimized code

class Foo(val id: Int) {
    inline def ==(that: Foo): Boolean = true
}
case class FooWrapper(foo: Foo)

yields

[error] 4 |    case class FooWrapper(foo: Foo)
[error]   |               ^
[error]   |               method == is declared as `inline`, but was not inlined
[error]   |
[error]   |               Try increasing `-Xmax-inlines` above 32

To make the problem go away, inline the method Bar.f.

Real-world problem

git clone https://github.com/informarte/yuck.git
cd yuck
git checkout spikes/scala-3-inlining
make unit-tests

works fine. However, rolling back the last commit (which inlines the unrelated method), breaks the compilation:

git checkout HEAD~1
make unit-tests

yields

/mill yuck.dev.test.run yuck.test.UnitTestSuite
[43/80] yuck.dev.compile 
[info] compiling 2 Scala sources to /tmp/yuck/out/yuck/dev/compile.dest/classes ...
[info] done compiling
[info] compiling 2 Scala sources to /tmp/yuck/out/yuck/dev/compile.dest/classes ...
[info] done compiling
[67/80] yuck.dev.test.compile 
[info] compiling 2 Scala sources to /tmp/yuck/out/yuck/dev/test/compile.dest/classes ...
[error] -- Error: /tmp/yuck/src/test/yuck/flatzinc/test/util/DotExporter.scala:18:23 ---
[error] 18 |    private case class VariableVertex(x: AnyVariable) extends Vertex
[error]    |                       ^
[error]    |                  method == is declared as `inline`, but was not inlined
[error]    |
[error]    |                  Try increasing `-Xmax-inlines` above 32
[error] -- Error: /tmp/yuck/src/test/yuck/flatzinc/test/util/DotExporter.scala:19:23 ---
[error] 19 |    private case class ConstraintVertex(constraint: Constraint) extends Vertex
[error]    |                       ^
[error]    |                  method == is declared as `inline`, but was not inlined
[error]    |
[error]    |                  Try increasing `-Xmax-inlines` above 32
[error] two errors found
1 targets failed
yuck.dev.test.compile Compilation failed

In fact, both AnyVariable and Constraint come with custom, inline == operators - I added those for efficiency and type safety - and they never caused any trouble with Scala 2.13.

As suggested, I increased max-inlines (up to 40) but it did not help.

Expectation

Assuming that my code is correct, I expect that it should compile without the need to inline an unrelated method.

@informarte informarte added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 9, 2022
@nicolasstucki nicolasstucki added stat:needs minimization Needs a self contained minimization area:inline and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 9, 2022
@informarte
Copy link
Author

@nicolasstucki, I extended my report with minimized code.

@nicolasstucki nicolasstucki removed the stat:needs minimization Needs a self contained minimization label Feb 14, 2022
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Feb 14, 2022

PostTyper (SyntheticMembers) creates the equals methods with a call to the inline == but does not set the ctx.compilationUnit.needsInlining.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Feb 14, 2022
olsdavis pushed a commit to olsdavis/dotty that referenced this issue Apr 4, 2022
@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants