Skip to content

Commit

Permalink
Unsuppress unchecked warnings
Browse files Browse the repository at this point in the history
And check it with a pos checkfile.
  • Loading branch information
dwijnand committed Aug 10, 2023
1 parent c5adafc commit fbdf7c6
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 89 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/Message.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,15 @@ class CompilationTests {
).times(2).checkCompile()
}

// Warning tests ------------------------------------------------------------

@Test def warn: Unit = {
implicit val testGroup: TestGroup = TestGroup("compileWarn")
aggregateTests(
compileFilesInDir("tests/warn", defaultOptions),
).checkWarnings()
}

// Negative tests ------------------------------------------------------------

@Test def negAll: Unit = {
Expand Down
98 changes: 49 additions & 49 deletions compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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] =
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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) = {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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).
Expand All @@ -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")
}

private 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

private 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 = {
Expand Down
24 changes: 0 additions & 24 deletions tests/pending/neg/i16451.check

This file was deleted.

72 changes: 72 additions & 0 deletions tests/warn/i16451.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
-- [E029] Pattern Match Exhaustivity Warning: tests/warn/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/warn/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/warn/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/warn/i16451.scala:34:11 ------------------------------------------------
34 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| Unreachable case
-- [E030] Match case Unreachable Warning: tests/warn/i16451.scala:39:11 ------------------------------------------------
39 | case x: Wrapper[Color.Red.type] => x // error: unreachable // error: unchecked
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| Unreachable case
-- [E092] Pattern Match Unchecked Warning: tests/warn/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/warn/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/warn/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/warn/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/warn/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/warn/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/warn/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/warn/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`
Loading

0 comments on commit fbdf7c6

Please sign in to comment.