From ab5a8665b62b0f8c9c6f902210c8fd6a1a960d78 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 9 Aug 2023 19:01:09 +0100 Subject: [PATCH] Unsuppress unchecked warnings And check it with a pos checkfile. --- .../dotty/tools/dotc/reporting/Message.scala | 2 +- .../dotty/tools/dotc/reporting/messages.scala | 5 +- .../tools/dotc/transform/TypeTestsCasts.scala | 2 +- .../dotty/tools/vulpix/ParallelTesting.scala | 98 +++++++++---------- tests/pending/neg/i16451.check | 24 ----- tests/warn/i16451.check | 72 ++++++++++++++ tests/{pending/neg => warn}/i16451.scala | 28 +++--- 7 files changed, 142 insertions(+), 89 deletions(-) delete mode 100644 tests/pending/neg/i16451.check create mode 100644 tests/warn/i16451.check rename tests/{pending/neg => warn}/i16451.scala (54%) diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala index 2ef839b5b402..11d86386d630 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Message.scala @@ -408,7 +408,7 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self => override def canExplain = true /** Override with `true` for messages that should always be shown even if their - * position overlaps another messsage of a different class. On the other hand + * position overlaps another message of a different class. On the other hand * multiple messages of the same class with overlapping positions will lead * to only a single message of that class to be issued. */ diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 5848d9e7cba1..c77f9b4fe2e1 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -895,9 +895,10 @@ extends Message(PatternMatchExhaustivityID) { } } -class UncheckedTypePattern(msgFn: => String)(using Context) +class UncheckedTypePattern(argType: Type, whyNot: String)(using Context) extends PatternMatchMsg(UncheckedTypePatternID) { - def msg(using Context) = msgFn + override def showAlways = true + def msg(using Context) = i"the type test for $argType cannot be checked at runtime because $whyNot" def explain(using Context) = i"""|Type arguments and type refinements are erased during compile time, thus it's |impossible to check them at run-time. diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index f5cb8eab73a4..9eaf92a83b7a 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -363,7 +363,7 @@ object TypeTestsCasts { if !isTrusted && !isUnchecked then val whyNot = whyUncheckable(expr.tpe, argType, tree.span) if whyNot.nonEmpty then - report.uncheckedWarning(em"the type test for $argType cannot be checked at runtime because $whyNot", expr.srcPos) + report.uncheckedWarning(UncheckedTypePattern(argType, whyNot), expr.srcPos) transformTypeTest(expr, argType, flagUnrelated = enclosingInlineds.isEmpty) // if test comes from inlined code, dont't flag it even if it always false } diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index bccbcbee29e1..322207140129 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -226,14 +226,14 @@ trait ParallelTesting extends RunnerOrchestration { self => Try(testSource match { case testSource @ JointCompilationSource(name, files, flags, outDir, fromTasty, decompilation) => val reporter = - if (fromTasty) compileFromTasty(flags, suppressErrors, outDir) - else compile(testSource.sourceFiles, flags, suppressErrors, outDir) + if (fromTasty) compileFromTasty(flags, outDir) + else compile(testSource.sourceFiles, flags, outDir) List(reporter) case testSource @ SeparateCompilationSource(_, dir, flags, outDir) => testSource.compilationGroups.map { (group, files) => if group.compiler.isEmpty then - compile(files, flags, suppressErrors, outDir) + compile(files, flags, outDir) else compileWithOtherCompiler(group.compiler, files, flags, outDir) } @@ -469,7 +469,7 @@ trait ParallelTesting extends RunnerOrchestration { self => registerCompletion() throw e - protected def compile(files0: Array[JFile], flags0: TestFlags, suppressErrors: Boolean, targetDir: JFile): TestReporter = { + protected def compile(files0: Array[JFile], flags0: TestFlags, targetDir: JFile): TestReporter = { import scala.util.Properties.* def flattenFiles(f: JFile): Array[JFile] = @@ -634,7 +634,7 @@ trait ParallelTesting extends RunnerOrchestration { self => reporter - protected def compileFromTasty(flags0: TestFlags, suppressErrors: Boolean, targetDir: JFile): TestReporter = { + protected def compileFromTasty(flags0: TestFlags, targetDir: JFile): TestReporter = { val tastyOutput = new JFile(targetDir.getPath + "_from-tasty") tastyOutput.mkdir() val flags = flags0 and ("-d", tastyOutput.getPath) and "-from-tasty" @@ -653,6 +653,12 @@ trait ParallelTesting extends RunnerOrchestration { self => private def mkLogLevel = if suppressErrors || suppressAllOutput then ERROR + 1 else ERROR private def mkReporter = TestReporter.reporter(realStdout, logLevel = mkLogLevel) + protected def diffCheckfile(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable) = + checkFile(testSource).foreach(diffTest(testSource, _, reporterOutputLines(reporters), reporters, logger)) + + private def reporterOutputLines(reporters: Seq[TestReporter]): List[String] = + reporters.flatMap(_.consoleOutput.split("\n")).toList + private[ParallelTesting] def executeTestSuite(): this.type = { assert(testSourcesCompleted == 0, "not allowed to re-use a `CompileRun`") if filteredSources.nonEmpty then @@ -717,6 +723,12 @@ trait ParallelTesting extends RunnerOrchestration { self => private final class PosTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting) extends Test(testSources, times, threadLimit, suppressAllOutput) + private final class WarnTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting) + extends Test(testSources, times, threadLimit, suppressAllOutput): + override def suppressErrors = true + override def onSuccess(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable): Unit = + diffCheckfile(testSource, reporters, logger) + private final class RewriteTest(testSources: List[TestSource], checkFiles: Map[JFile, JFile], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting) extends Test(testSources, times, threadLimit, suppressAllOutput) { private def verifyOutput(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable) = { @@ -808,10 +820,7 @@ trait ParallelTesting extends RunnerOrchestration { self => end maybeFailureMessage override def onSuccess(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable): Unit = - checkFile(testSource).foreach(diffTest(testSource, _, reporterOutputLines(reporters), reporters, logger)) - - def reporterOutputLines(reporters: Seq[TestReporter]): List[String] = - reporters.flatMap(_.consoleOutput.split("\n")).toList + diffCheckfile(testSource, reporters, logger) // In neg-tests we allow two or three types of error annotations. // Normally, `// error` must be annotated on the correct line number. @@ -1014,20 +1023,11 @@ trait ParallelTesting extends RunnerOrchestration { self => * compilation without generating errors and that they do not crash the * compiler */ - def checkCompile()(implicit summaryReport: SummaryReporting): this.type = { - val test = new PosTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite() - - cleanup() - - if (!shouldFail && test.didFail) { - fail(s"Expected no errors when compiling, failed for the following reason(s):\n${reasonsForFailure(test)}\n") - } - else if (shouldFail && !test.didFail && test.skipCount == 0) { - fail("Pos test should have failed, but didn't") - } + def checkCompile()(implicit summaryReport: SummaryReporting): this.type = + checkPass(new PosTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput), "Pos") - this - } + def checkWarnings()(implicit summaryReport: SummaryReporting): this.type = + checkPass(new WarnTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput), "Warn") /** Creates a "neg" test run, which makes sure that each test generates the * correct number of errors at the correct positions. It also makes sure @@ -1047,35 +1047,16 @@ trait ParallelTesting extends RunnerOrchestration { self => end checkExpectedErrors /** Creates a "fuzzy" test run, which makes sure that each test compiles (or not) without crashing */ - def checkNoCrash()(implicit summaryReport: SummaryReporting): this.type = { - val test = new NoCrashTest(targets, times, threadLimit, shouldSuppressOutput).executeTestSuite() - - cleanup() - - if (test.didFail) { - fail("Fuzzy test shouldn't have crashed, but did") - } - - this - } + def checkNoCrash()(implicit summaryReport: SummaryReporting): this.type = + checkFail(new NoCrashTest(targets, times, threadLimit, shouldSuppressOutput), "Fuzzy") /** Creates a "run" test run, which is a superset of "pos". In addition to * making sure that all tests pass compilation and that they do not crash * the compiler; it also makes sure that all tests can run with the * expected output */ - def checkRuns()(implicit summaryReport: SummaryReporting): this.type = { - val test = new RunTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite() - - cleanup() - - if !shouldFail && test.didFail then - fail(s"Run test failed, but should not, reasons:\n${ reasonsForFailure(test) }") - else if shouldFail && !test.didFail && test.skipCount == 0 then - fail("Run test should have failed, but did not") - - this - } + def checkRuns()(implicit summaryReport: SummaryReporting): this.type = + checkPass(new RunTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput), "Run") /** Tests `-rewrite`, which makes sure that the rewritten files still compile * and agree with the expected result (if specified). @@ -1100,15 +1081,34 @@ trait ParallelTesting extends RunnerOrchestration { self => target.copy(dir = copyToDir(outDir, dir)) } - val test = new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite() + val test = new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput) + + checkFail(test, "Rewrite") + } + + def checkPass(test: Test, desc: String): this.type = + test.executeTestSuite() cleanup() - if test.didFail then - fail("Rewrite test failed") + if !shouldFail && test.didFail then + fail(s"$desc test failed, but should not, reasons:\n${reasonsForFailure(test)}") + else if shouldFail && !test.didFail && test.skipCount == 0 then + fail(s"$desc test should have failed, but didn't") + + this + + def checkFail(test: test, desc: String): this.type = + test.executeTestSuite() + + cleanup() + + if shouldFail && !test.didFail && test.skipCount == 0 then + fail(s"$desc test shouldn't have failed, but did. Reasons:\n${reasonsForFailure(test)}") + else if !shouldFail && test.didFail then + fail(s"$desc test failed") this - } /** Deletes output directories and files */ private def cleanup(): this.type = { diff --git a/tests/pending/neg/i16451.check b/tests/pending/neg/i16451.check deleted file mode 100644 index e53085e8eafa..000000000000 --- a/tests/pending/neg/i16451.check +++ /dev/null @@ -1,24 +0,0 @@ --- Error: tests/neg/i16451.scala:13:9 ---------------------------------------------------------------------------------- -13 | case x: Wrapper[Color.Red.type] => Some(x) // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] --- Error: tests/neg/i16451.scala:21:9 ---------------------------------------------------------------------------------- -21 | case x: Wrapper[Color.Red.type] => Some(x) // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Any --- Error: tests/neg/i16451.scala:25:9 ---------------------------------------------------------------------------------- -25 | case x: Wrapper[Color.Red.type] => Some(x) // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] --- Error: tests/neg/i16451.scala:29:9 ---------------------------------------------------------------------------------- -29 | case x: Wrapper[Color.Red.type] => Some(x) // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from A1 --- Error: tests/neg/i16451.scala:34:11 --------------------------------------------------------------------------------- -34 | case x: Wrapper[Color.Red.type] => x // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] --- Error: tests/neg/i16451.scala:39:11 --------------------------------------------------------------------------------- -39 | case x: Wrapper[Color.Red.type] => x // error - | ^ - |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] diff --git a/tests/warn/i16451.check b/tests/warn/i16451.check new file mode 100644 index 000000000000..040e1c83f8fe --- /dev/null +++ b/tests/warn/i16451.check @@ -0,0 +1,72 @@ +-- [E029] Pattern Match Exhaustivity Warning: tests/pos/i16451.scala:12:73 --------------------------------------------- +12 | def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match // error: inexhaustive + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Wrapper(_) + | + | longer explanation available when compiling with `-explain` +-- [E030] Match case Unreachable Warning: tests/pos/i16451.scala:25:9 -------------------------------------------------- +25 | case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/pos/i16451.scala:29:9 -------------------------------------------------- +29 | case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/pos/i16451.scala:34:11 ------------------------------------------------- +34 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/pos/i16451.scala:39:11 ------------------------------------------------- +39 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Unreachable case +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:13:9 ------------------------------------------------- +13 | case x: Wrapper[Color.Red.type] => Some(x) // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:14:9 ------------------------------------------------- +14 | case x: Wrapper[Color.Green.type] => None // error: unchecked + | ^ + |the type test for Wrapper[(Color.Green : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:21:9 ------------------------------------------------- +21 | case x: Wrapper[Color.Red.type] => Some(x) // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Any + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:22:9 ------------------------------------------------- +22 | case x: Wrapper[Color.Green.type] => None // error: unchecked + | ^ + |the type test for Wrapper[(Color.Green : Color)] cannot be checked at runtime because its type arguments can't be determined from Any + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:25:9 ------------------------------------------------- +25 | case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:29:9 ------------------------------------------------- +29 | case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from A1 + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:34:11 ------------------------------------------------ +34 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] + | + | longer explanation available when compiling with `-explain` +-- [E092] Pattern Match Unchecked Warning: tests/pos/i16451.scala:39:11 ------------------------------------------------ +39 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked + | ^ + |the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color] + | + | longer explanation available when compiling with `-explain` diff --git a/tests/pending/neg/i16451.scala b/tests/warn/i16451.scala similarity index 54% rename from tests/pending/neg/i16451.scala rename to tests/warn/i16451.scala index 49997d2bcf92..ce0a320a49c9 100644 --- a/tests/pending/neg/i16451.scala +++ b/tests/warn/i16451.scala @@ -1,38 +1,42 @@ -// scalac: -Werror +// enum Color: case Red, Green +//sealed trait Color +//object Color: +// case object Red extends Color +// case object Green extends Color case class Wrapper[A](value: A) object Test: - def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match - case x: Wrapper[Color.Red.type] => Some(x) // error - case null => None + def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match // error: inexhaustive + case x: Wrapper[Color.Red.type] => Some(x) // error: unchecked + case x: Wrapper[Color.Green.type] => None // error: unchecked def test_different(x: Wrapper[Color]): Option[Wrapper[Color]] = x match case x @ Wrapper(_: Color.Red.type) => Some(x) case x @ Wrapper(_: Color.Green.type) => None def test_any(x: Any): Option[Wrapper[Color.Red.type]] = x match - case x: Wrapper[Color.Red.type] => Some(x) // error - case _ => None + case x: Wrapper[Color.Red.type] => Some(x) // error: unchecked + case x: Wrapper[Color.Green.type] => None // error: unchecked def test_wrong(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match - case x: Wrapper[Color.Red.type] => Some(x) // error - case null => None + case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + case _ => None def t2[A1 <: Wrapper[Color]](x: A1): Option[Wrapper[Color.Red.type]] = x match - case x: Wrapper[Color.Red.type] => Some(x) // error - case null => None + case x: Wrapper[Color.Red.type] => Some(x) // error: unreachable // error: unchecked + case _ => None def test_wrong_seq(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] = xs.collect { - case x: Wrapper[Color.Red.type] => x // error + case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked } def test_wrong_seq2(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] = xs.collect { x => x match - case x: Wrapper[Color.Red.type] => x // error + case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked } def main(args: Array[String]): Unit =