Skip to content

Commit

Permalink
Backport "Unsuppress unchecked warnings" to LTS (#20669)
Browse files Browse the repository at this point in the history
Backports #18377 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
  • Loading branch information
WojciechMazur committed Jun 20, 2024
2 parents 77b782e + dcbc291 commit c0da8f9
Show file tree
Hide file tree
Showing 24 changed files with 323 additions and 155 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 @@ -377,7 +377,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
4 changes: 2 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,9 @@ 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
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
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ object TypeTestsCasts {
}.apply(tp)

/** Returns true if the type arguments of `P` can be determined from `X` */
def typeArgsTrivial(X: Type, P: AppliedType)(using Context) = inContext(ctx.fresh.setExploreTyperState().setFreshGADTBounds) {
def typeArgsDeterminable(X: Type, P: AppliedType)(using Context) = inContext(ctx.fresh.setExploreTyperState().setFreshGADTBounds) {
val AppliedType(tycon, _) = P

def underlyingLambda(tp: Type): TypeLambda = tp.ensureLambdaSub match {
Expand Down Expand Up @@ -155,7 +155,7 @@ object TypeTestsCasts {
case x =>
// always false test warnings are emitted elsewhere
TypeComparer.provablyDisjoint(x, tpe.derivedAppliedType(tycon, targs.map(_ => WildcardType)))
|| typeArgsTrivial(X, tpe)
|| typeArgsDeterminable(X, tpe)
||| i"its type arguments can't be determined from $X"
}
case AndType(tp1, tp2) => recur(X, tp1) && recur(X, tp2)
Expand Down 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
19 changes: 11 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ object SpaceEngine {
project(pat)

case Typed(_, tpt) =>
Typ(erase(tpt.tpe.stripAnnots, isValue = true), decomposed = false)
Typ(erase(tpt.tpe.stripAnnots, isValue = true, isTyped = true), decomposed = false)

case This(_) =>
Typ(pat.tpe.stripAnnots, decomposed = false)
Expand Down Expand Up @@ -458,28 +458,31 @@ object SpaceEngine {
*
* @param inArray whether `tp` is a type argument to `Array`
* @param isValue whether `tp` is the type which match against values
* @param isTyped whether `tp` is the type from a `Typed` tree
*
* If `isValue` is true, then pattern-bound symbols are erased to its upper bound.
* This is needed to avoid spurious unreachable warnings. See tests/patmat/i6197.scala.
*/
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false)(using Context): Type =
trace(i"erase($tp${if inArray then " inArray" else ""}${if isValue then " isValue" else ""})", debug)(tp match {
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false, isTyped: Boolean = false)(using Context): Type =
trace(i"erase($tp${if inArray then " inArray" else ""}${if isValue then " isValue" else ""}${if isTyped then " isTyped" else ""})", debug)(tp match {
case tp @ AppliedType(tycon, args) if tycon.typeSymbol.isPatternBound =>
WildcardType

case tp @ AppliedType(tycon, args) =>
val inArray = tycon.isRef(defn.ArrayClass)
val args2 = args.map(arg => erase(arg, inArray = inArray, isValue = false))
val inArray = tycon.isRef(defn.ArrayClass) || tp.translucentSuperType.isRef(defn.ArrayClass)
val args2 =
if isTyped && !inArray then args.map(_ => WildcardType)
else args.map(arg => erase(arg, inArray = inArray, isValue = false))
tp.derivedAppliedType(erase(tycon, inArray, isValue = false), args2)

case tp @ OrType(tp1, tp2) =>
OrType(erase(tp1, inArray, isValue), erase(tp2, inArray, isValue), tp.isSoft)
OrType(erase(tp1, inArray, isValue, isTyped), erase(tp2, inArray, isValue, isTyped), tp.isSoft)

case AndType(tp1, tp2) =>
AndType(erase(tp1, inArray, isValue), erase(tp2, inArray, isValue))
AndType(erase(tp1, inArray, isValue, isTyped), erase(tp2, inArray, isValue, isTyped))

case tp @ RefinedType(parent, _, _) =>
erase(parent, inArray, isValue)
erase(parent, inArray, isValue, isTyped)

case tref: TypeRef if tref.symbol.isPatternBound =>
if inArray then tref.underlying
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 @@ -108,6 +108,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
16 changes: 9 additions & 7 deletions compiler/test/dotty/tools/dotc/reporting/TestReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import interfaces.Diagnostic.{ERROR, WARNING}

import scala.io.Codec

class TestReporter protected (outWriter: PrintWriter, filePrintln: String => Unit, logLevel: Int)
class TestReporter protected (outWriter: PrintWriter, logLevel: Int)
extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with MessageRendering {

protected final val _errorBuf = mutable.ArrayBuffer.empty[Diagnostic]
final def errors: Iterator[Diagnostic] = _errorBuf.iterator
protected final val _diagnosticBuf = mutable.ArrayBuffer.empty[Diagnostic]
final def diagnostics: Iterator[Diagnostic] = _diagnosticBuf.iterator
final def errors: Iterator[Diagnostic] = diagnostics.filter(_.level >= ERROR)

protected final val _messageBuf = mutable.ArrayBuffer.empty[String]
final def messages: Iterator[String] = _messageBuf.iterator
Expand Down Expand Up @@ -79,8 +80,9 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
case _ => ""
}

if dia.level >= ERROR then _errorBuf.append(dia)
if dia.level >= WARNING then _consoleReporter.doReport(dia)
if dia.level >= WARNING then
_diagnosticBuf.append(dia)
_consoleReporter.doReport(dia)
printMessageAndPos(dia, extra)
}
}
Expand Down Expand Up @@ -125,10 +127,10 @@ object TestReporter {
}

def reporter(ps: PrintStream, logLevel: Int): TestReporter =
new TestReporter(new PrintWriter(ps, true), logPrintln, logLevel)
new TestReporter(new PrintWriter(ps, true), logLevel)

def simplifiedReporter(writer: PrintWriter): TestReporter = {
val rep = new TestReporter(writer, logPrintln, WARNING) {
val rep = new TestReporter(writer, WARNING) {
/** Prints the message with the given position indication in a simplified manner */
override def printMessageAndPos(dia: Diagnostic, extra: String)(using Context): Unit = {
def report() = {
Expand Down
166 changes: 117 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,80 @@ 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)

override def maybeFailureMessage(testSource: TestSource, reporters: Seq[TestReporter]): Option[String] =
lazy val (map, expCount) = getWarnMapAndExpectedCount(testSource.sourceFiles.toIndexedSeq)
lazy val obtCount = reporters.foldLeft(0)(_ + _.warningCount)
lazy val (expected, unexpected) = getMissingExpectedWarnings(map, reporters.iterator.flatMap(_.diagnostics))
def hasMissingAnnotations = expected.nonEmpty || unexpected.nonEmpty
def showDiagnostics = "-> following the diagnostics:\n" +
reporters.flatMap(_.diagnostics.toSeq.sortBy(_.pos.line).map(e => s"${e.pos.line + 1}: ${e.message}")).mkString(" at ", "\n at ", "")
Option:
if reporters.exists(_.compilerCrashed) then s"Compiler crashed when compiling: ${testSource.title}"
else if reporters.exists(_.errorCount > 0) then
s"""Compilation failed for: ${testSource.title}
|$showDiagnostics
|""".stripMargin.trim.linesIterator.mkString("\n", "\n", "")
else if obtCount == 0 then s"\nNo warnings found when compiling warn test $testSource"
else if expCount == 0 then s"\nNo warning expected/defined in $testSource -- use // warn"
else if expCount != obtCount then
s"""|Wrong number of warnings encountered when compiling $testSource
|expected: $expCount, actual: $obtCount
|${expected.mkString("Unfulfilled expectations:\n", "\n", "")}
|${unexpected.mkString("Unexpected warnings:\n", "\n", "")}
|$showDiagnostics
|""".stripMargin.trim.linesIterator.mkString("\n", "\n", "")
else if hasMissingAnnotations then s"\nWarnings found on incorrect row numbers when compiling $testSource\n$showDiagnostics"
else if !map.isEmpty then s"\nExpected warnings(s) have {<warning position>=<unreported warning>}: $map"
else null
end maybeFailureMessage

def getWarnMapAndExpectedCount(files: Seq[JFile]): (HashMap[String, Integer], Int) =
val comment = raw"//( *)warn".r
val map = new HashMap[String, Integer]()
var count = 0
def bump(key: String): Unit =
map.get(key) match
case null => map.put(key, 1)
case n => map.put(key, n+1)
count += 1
files.filter(isSourceFile).foreach { file =>
Using(Source.fromFile(file, StandardCharsets.UTF_8.name)) { source =>
source.getLines.zipWithIndex.foreach { case (line, lineNbr) =>
comment.findAllMatchIn(line).foreach { _ =>
bump(s"${file.getPath}:${lineNbr+1}")
}
}
}.get
}
(map, count)

def getMissingExpectedWarnings(map: HashMap[String, Integer], reporterWarnings: Iterator[Diagnostic]): (List[String], List[String]) =
val unexpected, unpositioned = ListBuffer.empty[String]
def relativize(path: String): String = path.split(JFile.separatorChar).dropWhile(_ != "tests").mkString(JFile.separator)
def seenAt(key: String): Boolean =
map.get(key) match
case null => false
case 1 => map.remove(key) ; true
case n => map.put(key, n - 1) ; true
def sawDiagnostic(d: Diagnostic): Unit =
val srcpos = d.pos.nonInlined
if srcpos.exists then
val key = s"${relativize(srcpos.source.file.toString())}:${srcpos.line + 1}"
if !seenAt(key) then unexpected += key
else
unpositioned += relativize(srcpos.source.file.toString())

reporterWarnings.foreach(sawDiagnostic)

(map.asScala.keys.toList, (unexpected ++ unpositioned).toList)
end getMissingExpectedWarnings

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 +888,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 +1091,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()
def checkCompile()(implicit summaryReport: SummaryReporting): this.type =
checkPass(new PosTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput), "Pos")

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")
}

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 +1115,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 +1149,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 !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 test.didFail then
fail("Rewrite test failed")
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
Loading

0 comments on commit c0da8f9

Please sign in to comment.