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.

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

fix: disallow toplevel infix definitions for vals, vars, givens, methods and implicits #17994

Merged
merged 10 commits into from
Jul 13, 2023
Merged

fix: disallow toplevel infix definitions for vals, vars, givens, methods and implicits #17994

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
91515d6
fix: disallow toplevel infix definitions for vals, vars, givens, meth…
arainko Jun 17, 2023
1dd1af8
actually check if a symbol is infix before failing with ToplevelDefCa…
arainko Jun 17, 2023
08ddbac
allow infix extension methods
arainko Jun 17, 2023
2dcf133
add newline at the end of 'messages.scala'
arainko Jun 20, 2023
03a2ae0
another newline adjustment
arainko Jun 20, 2023
3f3bd01
change error code from 183 to 186 in checkfile
arainko Jul 1, 2023
2b391c8
delete ToplevelDefCantBeInfix (along with its MessageId) and use Modi…
arainko Jul 8, 2023
46a1403
use Symbol#showKind to print a nicer error message, make the explanat…
arainko Jul 12, 2023
000cdd4
Update tests/neg/i17738-toplevel-infix.scala
mbovel Jul 13, 2023
ef786ff
Update tests/neg/i17738-toplevel-infix.check
mbovel Jul 13, 2023
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
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2551,10 +2551,10 @@ class AnonymousInstanceCannotBeEmpty(impl: untpd.Template)(using Context)
|"""
}

class ModifierNotAllowedForDefinition(flag: Flag)(using Context)
class ModifierNotAllowedForDefinition(flag: Flag, explanation: String = "")(using Context)
extends SyntaxMsg(ModifierNotAllowedForDefinitionID) {
def msg(using Context) = i"Modifier ${hl(flag.flagsString)} is not allowed for this definition"
def explain(using Context) = ""
def explain(using Context) = explanation
}

class RedundantModifier(flag: Flag)(using Context)
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,13 @@ object Checking {
fail(CannotHaveSameNameAs(sym, cls, CannotHaveSameNameAs.CannotBeOverridden))
sym.setFlag(Private) // break the overriding relationship by making sym Private
}
if sym.isWrappedToplevelDef && !sym.isType && sym.flags.is(Infix, butNot = Extension) then
val defKind =
mbovel marked this conversation as resolved.
Show resolved Hide resolved
if sym.flags.is(Method) then "def"
else if sym.flags.is(Mutable) then "var"
else if sym.flags.is(Given) then "given"
else "val"
fail(ModifierNotAllowedForDefinition(Flags.Infix, s"a toplevel $defKind cannot be infix"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val defKind =
if sym.flags.is(Method) then "def"
else if sym.flags.is(Mutable) then "var"
else if sym.flags.is(Given) then "given"
else "val"
fail(ModifierNotAllowedForDefinition(Flags.Infix, s"a toplevel $defKind cannot be infix"))
fail(ModifierNotAllowedForDefinition(Flags.Infix, s"A top-level ${sym.showKind} cannot be infix."))

Looks like sym.showKind could be a good fit here.

Its implemented there: https://github.com/lampepfl/dotty/blob/2b391c82de1861d8ab0196e78dd2975539b8082a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala#L477-L502

Note that the check file would need to be updated as well as the names are not exactly the same as what you currently have.

Copy link
Member

Choose a reason for hiding this comment

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

Minor: my suggestion also makes the explanation a full sentence (we don't have very coherent formatting for error messages, but it seems like full sentences for explanations is more common), and using top-level instead of toplevel as it is more common.

checkApplicable(Erased,
!sym.isOneOf(MutableOrLazy, butNot = Given) && !sym.isType || sym.isClass)
checkCombination(Final, Open)
Expand Down
45 changes: 45 additions & 0 deletions tests/neg/i17738-toplevel-infix.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
-- [E156] Syntax Error: tests/neg/i17738-toplevel-infix.scala:14:10 ----------------------------------------------------
14 |infix val toplevelVal = ??? // error
| ^
| Modifier infix is not allowed for this definition
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| a toplevel val cannot be infix
--------------------------------------------------------------------------------------------------------------------
-- [E156] Syntax Error: tests/neg/i17738-toplevel-infix.scala:15:10 ----------------------------------------------------
15 |infix var toplevelVar = ??? // error
| ^
| Modifier infix is not allowed for this definition
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| a toplevel var cannot be infix
--------------------------------------------------------------------------------------------------------------------
-- [E156] Syntax Error: tests/neg/i17738-toplevel-infix.scala:16:10 ----------------------------------------------------
16 |infix def toplevelDef = ??? // error
| ^
| Modifier infix is not allowed for this definition
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| a toplevel def cannot be infix
--------------------------------------------------------------------------------------------------------------------
-- [E156] Syntax Error: tests/neg/i17738-toplevel-infix.scala:17:12 ----------------------------------------------------
17 |infix given toplevelGiven: Int = ??? // error
| ^
| Modifier infix is not allowed for this definition
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| a toplevel given cannot be infix
--------------------------------------------------------------------------------------------------------------------
-- [E156] Syntax Error: tests/neg/i17738-toplevel-infix.scala:18:19 ----------------------------------------------------
18 |infix implicit val topevelImplicit: Int = ??? // error
mbovel marked this conversation as resolved.
Show resolved Hide resolved
| ^
| Modifier infix is not allowed for this definition
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| a toplevel val cannot be infix
--------------------------------------------------------------------------------------------------------------------
18 changes: 18 additions & 0 deletions tests/neg/i17738-toplevel-infix.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// scalac: -explain
mbovel marked this conversation as resolved.
Show resolved Hide resolved
infix type A[b, a] = Nothing

infix type B[b, a] = b match {
case Int => a
}

infix class C[A, B]
infix trait D[A, B]

extension (x: Boolean)
infix def or (y: => Boolean) = x || y

infix val toplevelVal = ??? // error
infix var toplevelVar = ??? // error
infix def toplevelDef = ??? // error
infix given toplevelGiven: Int = ??? // error
infix implicit val topevelImplicit: Int = ??? // error
mbovel marked this conversation as resolved.
Show resolved Hide resolved