Skip to content

Add support for JEP-409 (sealed classes) + Add javacOpt directive #19080

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

Merged
merged 4 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,18 @@ object JavaParsers {
addAnnot(scalaDot(jtpnme.VOLATILEkw))
case SYNCHRONIZED | STRICTFP =>
in.nextToken()
case SEALED =>
flags |= Flags.Sealed
in.nextToken()
// JEP-409: Special trick for the 'non-sealed' java keyword
case IDENTIFIER if in.name.toString == "non" =>
val lookahead = in.LookaheadScanner()
({lookahead.nextToken(); lookahead.token}, {lookahead.nextToken(); lookahead.name.toString}) match
case (MINUS, "sealed") =>
in.nextToken(); in.nextToken() // skip '-' and 'sealed'. Nothing more to do
case _ =>
syntaxError(em"Identifier '${in.name}' is not allowed here")
in.nextToken()
case _ =>
val privateWithin: TypeName =
if (isPackageAccess && !inInterface) thisPackageName
Expand Down Expand Up @@ -812,6 +824,17 @@ object JavaParsers {
else
List()


def permittedSubclassesOpt(isSealed: Boolean) : List[Tree] =
if in.token == PERMITS && !isSealed then
syntaxError(em"A type declaration that has a permits clause should have a sealed modifier")
if in.token == PERMITS then
in.nextToken()
repsep(() => typ(), COMMA)
else
// JEP-409: Class/Interface may omit the permits clause
Nil

def classDecl(start: Offset, mods: Modifiers): List[Tree] = {
accept(CLASS)
val nameOffset = in.offset
Expand All @@ -825,6 +848,7 @@ object JavaParsers {
else
ObjectTpt()
val interfaces = interfacesOpt()
val permittedSubclasses = permittedSubclassesOpt(mods.is(Flags.Sealed))
val (statics, body) = typeBody(CLASS, name)
val cls = atSpan(start, nameOffset) {
TypeDef(name, makeTemplate(superclass :: interfaces, body, tparams, needsDummyConstr = true)).withMods(mods)
Expand Down Expand Up @@ -889,6 +913,7 @@ object JavaParsers {
}
else
List(ObjectTpt())
val permittedSubclasses = permittedSubclassesOpt(mods is Flags.Sealed)
val (statics, body) = typeBody(INTERFACE, name)
val iface = atSpan(start, nameOffset) {
TypeDef(
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/parsing/JavaScanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ object JavaScanners {
'5' | '6' | '7' | '8' | '9' =>
putChar(ch)
nextChar()

case '_' =>
putChar(ch)
nextChar()
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/parsing/JavaTokens.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ object JavaTokens extends TokensCommon {

final val javaOnlyKeywords: TokenSet = tokenRange(INSTANCEOF, ASSERT)
final val sharedKeywords: BitSet = BitSet( IF, FOR, ELSE, THIS, NULL, NEW, SUPER, ABSTRACT, FINAL, PRIVATE, PROTECTED,
EXTENDS, TRUE, FALSE, CLASS, IMPORT, PACKAGE, DO, THROW, TRY, CATCH, FINALLY, WHILE, RETURN )
EXTENDS, TRUE, FALSE, CLASS, IMPORT, PACKAGE, DO, THROW, TRY, CATCH, FINALLY, WHILE, RETURN, SEALED)
final val primTypes: TokenSet = tokenRange(VOID, DOUBLE)
final val keywords: BitSet = sharedKeywords | javaOnlyKeywords | primTypes

Expand All @@ -22,6 +22,7 @@ object JavaTokens extends TokensCommon {
inline val INTERFACE = 105; enter(INTERFACE, "interface")
inline val ENUM = 106; enter(ENUM, "enum")
inline val IMPLEMENTS = 107; enter(IMPLEMENTS, "implements")
inline val PERMITS = 108; enter(PERMITS, "permits")

/** modifiers */
inline val PUBLIC = 110; enter(PUBLIC, "public")
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/parsing/Tokens.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ abstract class TokensCommon {
//inline val YIELD = 48; enter(YIELD, "yield")
inline val DO = 49; enter(DO, "do")
//inline val TRAIT = 50; enter(TRAIT, "trait")
//inline val SEALED = 51; enter(SEALED, "sealed")
inline val SEALED = 51; enter(SEALED, "sealed")
inline val THROW = 52; enter(THROW, "throw")
inline val TRY = 53; enter(TRY, "try")
inline val CATCH = 54; enter(CATCH, "catch")
Expand Down Expand Up @@ -169,7 +169,7 @@ object Tokens extends TokensCommon {
inline val OBJECT = 44; enter(OBJECT, "object")
inline val YIELD = 48; enter(YIELD, "yield")
inline val TRAIT = 50; enter(TRAIT, "trait")
inline val SEALED = 51; enter(SEALED, "sealed")
//inline val SEALED = 51; enter(SEALED, "sealed")
inline val MATCH = 58; enter(MATCH, "match")
inline val LAZY = 59; enter(LAZY, "lazy")
inline val THEN = 60; enter(THEN, "then")
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,7 @@ class Namer { typer: Typer =>
* (2) If may not derive from itself
* (3) The class is not final
* (4) If the class is sealed, it is defined in the same compilation unit as the current class
* (unless defined in Java. See JEP-409)
*
* @param isJava If true, the parent type is in Java mode, and we do not require a stable prefix
*/
Expand All @@ -1569,7 +1570,7 @@ class Namer { typer: Typer =>
if pclazz.is(Final) then
report.error(ExtendFinalClass(cls, pclazz), cls.srcPos)
else if pclazz.isEffectivelySealed && pclazz.associatedFile != cls.associatedFile then
if pclazz.is(Sealed) then
if pclazz.is(Sealed) && !pclazz.is(JavaDefined) then
Copy link
Member

@bishabosha bishabosha Feb 27, 2024

Choose a reason for hiding this comment

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

this would seem to allow extending a sealed Java class in a Scala file, perhaps use !isJava instead which tests the compilation unit. Or could it be legitimate to define a permitted sub class in Scala?

Copy link
Member

@bishabosha bishabosha Feb 27, 2024

Choose a reason for hiding this comment

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

Actually it is allowed, If I compile with scalac Cat.scala and Pet.java in run 1, then javac on Pet.java in run 2 with classpath of run 1 outputs, then it works

Copy link
Member

@bishabosha bishabosha Feb 27, 2024

Choose a reason for hiding this comment

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

maybe it could be interesting to add this as a test case in a follow up

report.error(UnableToExtendSealedClass(pclazz), cls.srcPos)
else if sourceVersion.isAtLeast(future) then
checkFeature(nme.adhocExtensions,
Expand Down
18 changes: 15 additions & 3 deletions compiler/test/dotty/tools/utils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,14 @@ def toolArgsFor(tool: ToolName, filename: Option[String])(lines: List[String]):
// groups are (name, args)
// note: ideally we would replace everything that requires this to use directive syntax, however scalajs: --skip has no directive equivalent yet.
private val toolArg = raw"(?://|/\*| \*) ?(?i:(${ToolName.values.mkString("|")})):((?:[^*]|\*(?!/))*)".r.unanchored

// ================================================================================================
// =================================== VULPIX DIRECTIVES ==========================================
// ================================================================================================

/** Directive to specify to vulpix the options to pass to Dotty */
private val directiveOptionsArg = raw"//> using options (.*)".r.unanchored
private val directiveJavacOptions = raw"//> using javacOpt (.*)".r.unanchored
Copy link
Member Author

@hamzaremmal hamzaremmal Feb 14, 2024

Choose a reason for hiding this comment

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


// Inspect the lines for compiler options of the form
// `//> using options args`, `// scalajs: args`, `/* scalajs: args`, ` * scalajs: args` etc.
Expand All @@ -90,10 +97,15 @@ private val directiveOptionsArg = raw"//> using options (.*)".r.unanchored
def toolArgsParse(lines: List[String], filename: Option[String]): List[(String,String)] =
lines.flatMap {
case toolArg("scalac", _) => sys.error(s"`// scalac: args` not supported. Please use `//> using options args`${filename.fold("")(f => s" in file $f")}")
case toolArg("javac", _) => sys.error(s"`// javac: args` not supported. Please use `//> using javacOpt args`${filename.fold("")(f => s" in file $f")}")
case toolArg(name, args) => List((name, args))
case _ => Nil
} ++
lines.flatMap { case directiveOptionsArg(args) => List(("scalac", args)) case _ => Nil }
lines.flatMap {
case directiveOptionsArg(args) => List(("scalac", args))
case directiveJavacOptions(args) => List(("javac", args))
case _ => Nil
}

import org.junit.Test
import org.junit.Assert._
Expand All @@ -104,6 +116,6 @@ class ToolArgsTest:
@Test def `tool is present`: Unit = assertEquals("-hey" :: Nil, toolArgsFor(ToolName.Test, None)("// test: -hey" :: Nil))
@Test def `missing tool is absent`: Unit = assertEquals(Nil, toolArgsFor(ToolName.Javac, None)("// test: -hey" :: Nil))
@Test def `multitool is present`: Unit =
assertEquals("-hey" :: Nil, toolArgsFor(ToolName.Test, None)("// test: -hey" :: "// javac: -d /tmp" :: Nil))
assertEquals("-d" :: "/tmp" :: Nil, toolArgsFor(ToolName.Javac, None)("// test: -hey" :: "// javac: -d /tmp" :: Nil))
assertEquals("-hey" :: Nil, toolArgsFor(ToolName.Test, None)("// test: -hey" :: "// java: -d /tmp" :: Nil))
assertEquals("-d" :: "/tmp" :: Nil, toolArgsFor(ToolName.Java, None)("// test: -hey" :: "// java: -d /tmp" :: Nil))
end ToolArgsTest
6 changes: 3 additions & 3 deletions compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
case None => true

def scalacOptions = toolArgs.getOrElse(ToolName.Scalac, Nil)
def javacOptions = toolArgs.getOrElse(ToolName.Javac, Nil)

val flags = flags0
.and(scalacOptions*)
Expand All @@ -512,11 +513,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>

def compileWithJavac(fs: Array[String]) = if (fs.nonEmpty) {
val fullArgs = Array(
"javac",
"-encoding", StandardCharsets.UTF_8.name,
) ++ flags.javacFlags ++ fs
) ++ flags.javacFlags ++ javacOptions++ fs

val process = Runtime.getRuntime.exec(fullArgs)
val process = Runtime.getRuntime.exec("javac" +: fullArgs)
val output = Source.fromInputStream(process.getErrorStream).mkString

if waitForJudiciously(process) != 0 then Some(output)
Expand Down
8 changes: 8 additions & 0 deletions tests/neg/i18533.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Error: tests/neg/i18533/Pet_SCALA_ONLY.java:3:10 --------------------------------------------------------------------
3 |class Pet permits Cat { // error
| ^^^^^^^
| A type declaration that has a permits clause should have a sealed modifier
-- Error: tests/neg/i18533/non-SCALA_ONLY.java:4:7 ---------------------------------------------------------------------
4 |public non class Test { // error
| ^^^
| Identifier 'non' is not allowed here
5 changes: 5 additions & 0 deletions tests/neg/i18533/Cat_SCALA_ONLY.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package i18533;

public final class Cat extends Pet {

}
5 changes: 5 additions & 0 deletions tests/neg/i18533/Pet_SCALA_ONLY.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package i18533;

class Pet permits Cat { // error

}
6 changes: 6 additions & 0 deletions tests/neg/i18533/non-SCALA_ONLY.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package i18533;

// Special test for the non-sealed trick (See JavaParsers.scala::modifiers)
public non class Test { // error

}
8 changes: 8 additions & 0 deletions tests/pos/i18533/Cat.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//> using javacOpt --enable-preview --source 17
Copy link
Member Author

@hamzaremmal hamzaremmal Feb 14, 2024

Choose a reason for hiding this comment

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

These options should be removed when CI is bumped to Java 21 (See #19701)

//> test: -jvm 17+

package i18533;

public final class Cat extends Pet {

}
8 changes: 8 additions & 0 deletions tests/pos/i18533/Dog.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//> using javacOpt --enable-preview --source 17
//> test: -jvm 17+

package i18533;

public non-sealed class Dog extends Pet {

}
8 changes: 8 additions & 0 deletions tests/pos/i18533/Pet.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//> using javacOpt --enable-preview --source 17
//> test: -jvm 17+

package i18533;

public sealed class Pet permits Cat, Dog {

}
3 changes: 0 additions & 3 deletions tests/run/t9915/C_1.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/*
* javac: -encoding UTF-8
*/
public class C_1 {
public static final String NULLED = "X\000ABC";
public static final String SUPPED = "𐒈𐒝𐒑𐒛𐒐𐒘𐒕𐒖";
Expand Down