Skip to content

Backport "Unsuppress unchecked warnings" to LTS #20669

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 7 commits into from
Jun 20, 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
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
Loading