Skip to content

Detect InsertedApply on Apply to break synthetic apply loops, not just Select #25463

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.

Sign up for GitHub

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

Open
SolalPirelli wants to merge 3 commits into
base: main
Choose a base branch
from
+14 −3
Open

Detect InsertedApply on Apply to break synthetic apply loops, not just Select #25463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9c66ff
Regression test
SolalPirelli Mar 9, 2026
4b7b8f6
Fix
SolalPirelli Mar 9, 2026
338092a
add checkfile
SolalPirelli Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ object Typer {
* apply of a function class?
*/
private[typer] def isSyntheticApply(tree: tpd.Tree): Boolean = tree match {
case tree: tpd.Select => tree.hasAttachment(InsertedApply)
case _: (tpd.Select | tpd.Apply) => tree.hasAttachment(InsertedApply)
case TypeApply(fn, targs) => isSyntheticApply(fn) && targs.forall(_.isInstanceOf[tpd.InferredTypeTree])
case _ => false
}
Expand Down
6 changes: 6 additions & 0 deletions tests/neg/24782.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- [E050] Type Error: tests/neg/24782.scala:4:28 -----------------------------------------------------------------------
4 | def f(using pure: Pure) = S(s => pure(s)) // error (but no compiler crash / stack overflow)
| ^
| expression does not take more parameters
|
| longer explanation available when compiling with `-explain`
5 changes: 5 additions & 0 deletions tests/neg/24782.scala
View file
Open in desktop
Copy link
Member

@hamzaremmal hamzaremmal Mar 11, 2026

Choose a reason for hiding this comment

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

checkfile is missing :)

Copy link
Contributor Author

@SolalPirelli SolalPirelli Mar 11, 2026

Choose a reason for hiding this comment

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

oops, thanks, added

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object S {
trait F[A]
type Pure = [a] => a => F[a]
def f(using pure: Pure) = S(s => pure(s)) // error (but no compiler crash / stack overflow)
}
4 changes: 2 additions & 2 deletions tests/pos/i14907.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ object Module {
case 1 => Any *: Fill[0]
}
extension[N <: Int] (f: Fun[N])
def apply: Fill[N] => Any = ???
def apply(): Fill[N] => Any = ???
Copy link
Member

@hamzaremmal hamzaremmal Mar 11, 2026

Choose a reason for hiding this comment

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

https://github.com/scala/scala3/pull/25463/changes#r2906064294

So defining def apply is not allowed?

Copy link
Contributor Author

@SolalPirelli SolalPirelli Mar 11, 2026

Choose a reason for hiding this comment

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

per conversation on Slack, no, parameterless apply should not be used since the language reference only talks about parameterized insertions of apply, see the recent thread in compiler-development

Copy link
Member

@hamzaremmal hamzaremmal Mar 12, 2026

Choose a reason for hiding this comment

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

Okay, then can we have a clear test that shows explicitly this. Something like:

object A:
  def apply: Int = ???

val x: Int = A // error: must call apply explicitly

This test and the other have too many elements in them that it is not clear what their purpose is.

Copy link
Contributor Author

@SolalPirelli SolalPirelli Mar 12, 2026

Choose a reason for hiding this comment

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

It would be nice to have some systematic tests of language features, yeah, but for that we'd need an unambiguous language reference first, the slack thread already mentions some corner cases like "what if apply is of a custom function-like type". :/

Copy link
Member

@hamzaremmal hamzaremmal Mar 12, 2026

Choose a reason for hiding this comment

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

but for that we'd need an unambiguous language reference first

I agree. If that is intended to be the case, and if you find the wording to be ambiguous, you can also update the wording to at least cover this case?

Copy link
Contributor Author

@SolalPirelli SolalPirelli Mar 12, 2026

Choose a reason for hiding this comment

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

created #25504


Fun[1]()(???)
Fun[1]()()(???)
Copy link
Contributor Author

@SolalPirelli SolalPirelli Mar 9, 2026

Choose a reason for hiding this comment

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

As discussed on Slack, per §6.6 of the language ref, apply should only be used if there are parameters. This is the only positive test I could find in which this was happening.

}
Loading