Skip to content

Backport "Add the -Wall option that enables all warnings (Plan B)" to LTS #22107

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 5 commits into from
Dec 4, 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
65 changes: 39 additions & 26 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,46 +165,50 @@ private sealed trait WarningSettings:

val Whelp: Setting[Boolean] = BooleanSetting("-W", "Print a synopsis of warning options.")
val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings"))
val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
val Wall: Setting[Boolean] = BooleanSetting("-Wall", "Enable all warning settings.")
private val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
private val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
private val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
name = "-Wunused",
helpArg = "warning",
descr = "Enable or disable specific `unused` warnings",
choices = List(
choices = List(
ChoiceWithHelp("nowarn", ""),
ChoiceWithHelp("all",""),
ChoiceWithHelp("all", ""),
ChoiceWithHelp(
name = "imports",
description = "Warn if an import selector is not referenced.\n" +
"NOTE : overrided by -Wunused:strict-no-implicit-warn"),
ChoiceWithHelp("privates","Warn if a private member is unused"),
ChoiceWithHelp("locals","Warn if a local definition is unused"),
ChoiceWithHelp("explicits","Warn if an explicit parameter is unused"),
ChoiceWithHelp("implicits","Warn if an implicit parameter is unused"),
ChoiceWithHelp("params","Enable -Wunused:explicits,implicits"),
ChoiceWithHelp("linted","Enable -Wunused:imports,privates,locals,implicits"),
ChoiceWithHelp(
name = "strict-no-implicit-warn",
description = "Same as -Wunused:import, only for imports of explicit named members.\n" +
"NOTE : This overrides -Wunused:imports and NOT set by -Wunused:all"
),
// ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"),
ChoiceWithHelp(
name = "unsafe-warn-patvars",
description = "(UNSAFE) Warn if a variable bound in a pattern is unused.\n" +
"This warning can generate false positive, as warning cannot be\n" +
"suppressed yet."
)
ChoiceWithHelp("privates", "Warn if a private member is unused"),
ChoiceWithHelp("locals", "Warn if a local definition is unused"),
ChoiceWithHelp("explicits", "Warn if an explicit parameter is unused"),
ChoiceWithHelp("implicits", "Warn if an implicit parameter is unused"),
ChoiceWithHelp("params", "Enable -Wunused:explicits,implicits"),
ChoiceWithHelp("linted", "Enable -Wunused:imports,privates,locals,implicits"),
ChoiceWithHelp(
name = "strict-no-implicit-warn",
description = "Same as -Wunused:import, only for imports of explicit named members.\n" +
"NOTE : This overrides -Wunused:imports and NOT set by -Wunused:all"
),
// ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"),
ChoiceWithHelp(
name = "unsafe-warn-patvars",
description = "(UNSAFE) Warn if a variable bound in a pattern is unused.\n" +
"This warning can generate false positive, as warning cannot be\n" +
"suppressed yet."
)
),
default = Nil
)
object WunusedHas:
def isChoiceSet(s: String)(using Context) = Wunused.value.pipe(us => us.contains(s))
def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s))
def allOr(s: String)(using Context) = Wall.value || Wunused.value.pipe(us => us.contains("all") || us.contains(s))
def nowarn(using Context) = allOr("nowarn")

// Is any choice set for -Wunused?
def any(using Context): Boolean = Wall.value || Wunused.value.nonEmpty

// overrided by strict-no-implicit-warn
def imports(using Context) =
(allOr("imports") || allOr("linted")) && !(strictNoImplicitWarn)
Expand Down Expand Up @@ -278,6 +282,16 @@ private sealed trait WarningSettings:
|to prevent the shell from expanding patterns.""".stripMargin,
)

val YcheckInit: Setting[Boolean] = BooleanSetting("-Ysafe-init", "Ensure safe initialization of objects.")

object Whas:
def allOr(s: Setting[Boolean])(using Context): Boolean =
Wall.value || s.value
def valueDiscard(using Context): Boolean = allOr(WvalueDiscard)
def nonUnitStatement(using Context): Boolean = allOr(WNonUnitStatement)
def enumCommentDiscard(using Context): Boolean = allOr(WenumCommentDiscard)
def checkInit(using Context): Boolean = allOr(YcheckInit)

/** -X "Extended" or "Advanced" settings */
private sealed trait XSettings:
self: SettingGroup =>
Expand Down Expand Up @@ -415,7 +429,6 @@ private sealed trait YSettings:
// Experimental language features
val YnoKindPolymorphism: Setting[Boolean] = BooleanSetting("-Yno-kind-polymorphism", "Disable kind polymorphism.")
val YexplicitNulls: Setting[Boolean] = BooleanSetting("-Yexplicit-nulls", "Make reference types non-nullable. Nullable types can be expressed with unions: e.g. String|Null.")
val YcheckInit: Setting[Boolean] = BooleanSetting("-Ysafe-init", "Ensure safe initialization of objects.")
val YrequireTargetName: Setting[Boolean] = BooleanSetting("-Yrequire-targetName", "Warn if an operator is defined without a @targetName annotation.")
val YrecheckTest: Setting[Boolean] = BooleanSetting("-Yrecheck-test", "Run basic rechecking (internal test only).")
val YccDebug: Setting[Boolean] = BooleanSetting("-Ycc-debug", "Used in conjunction with captureChecking language import, debug info for captured references.")
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ object Symbols extends SymUtils {
ctx.settings.YretainTrees.value ||
denot.owner.isTerm || // no risk of leaking memory after a run for these
denot.isOneOf(InlineOrProxy) || // need to keep inline info
ctx.settings.YcheckInit.value // initialization check
ctx.settings.Whas.checkInit // initialization check

/** The last denotation of this symbol */
private var lastDenot: SymDenotation = _
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3923,7 +3923,7 @@ object Parsers {
if (in.token == COMMA) {
in.nextToken()
val ids = commaSeparated(() => termIdent())
if ctx.settings.WenumCommentDiscard.value then
if ctx.settings.Whas.enumCommentDiscard then
in.getDocComment(start).foreach: comm =>
warning(
em"""Ambiguous Scaladoc comment on multiple cases is ignored.
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke

override def isRunnable(using Context): Boolean =
super.isRunnable &&
ctx.settings.Wunused.value.nonEmpty &&
ctx.settings.WunusedHas.any &&
!ctx.isJava

// ========== SETUP ============
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Checker extends Phase:
override val runsAfter = Set(Pickler.name)

override def isEnabled(using Context): Boolean =
super.isEnabled && ctx.settings.YcheckInit.value
super.isEnabled && ctx.settings.Whas.checkInit

def traverse(traverser: InitTreeTraverser)(using Context): Boolean = monitor(phaseName):
val unit = ctx.compilationUnit
Expand All @@ -46,7 +46,8 @@ class Checker extends Phase:
cancellable {
val classes = traverser.getClasses()

Semantic.checkClasses(classes)(using checkCtx)
if ctx.settings.Whas.checkInit then
Semantic.checkClasses(classes)(using checkCtx)
}

units0
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4196,7 +4196,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
// so will take the code path that decides on inlining
val tree1 = adapt(tree, WildcardType, locked)
checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.Whas.valueDiscard && !isThisTypeResult(tree)) {
report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
}
return tpd.Block(tree1 :: Nil, unitLiteral)
Expand Down Expand Up @@ -4498,7 +4498,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
&& !isJavaApplication(t) // Java methods are inherently side-effecting
// && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression?
)
if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then
if ctx.settings.Whas.nonUnitStatement && !ctx.isAfterTyper && checkInterestingShapes(t) then
val where = t match {
case Block(_, res) => res
case If(_, thenpart, Literal(Constant(()))) =>
Expand Down
2 changes: 1 addition & 1 deletion tests/pos-with-compiler-cc/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ object Symbols {
ctx.settings.YretainTrees.value ||
denot.owner.isTerm || // no risk of leaking memory after a run for these
denot.isOneOf(InlineOrProxy) || // need to keep inline info
ctx.settings.YcheckInit.value // initialization check
ctx.settings.Whas.checkInit // initialization check

/** The last denotation of this symbol */
private var lastDenot: SymDenotation = _
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Checker extends Phase:
override val runsAfter = Set(Pickler.name)

override def isEnabled(using Context): Boolean =
super.isEnabled && ctx.settings.YcheckInit.value
super.isEnabled && ctx.settings.Whas.checkInit

override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] =
val checkCtx = ctx.fresh.setPhase(this.start)
Expand Down
12 changes: 12 additions & 0 deletions tests/warn/i18559a.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:4:28 ---------------------------------------------------------
4 | import collection.mutable.Set // warn
| ^^^
| unused import
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:8:8 ----------------------------------------------------------
8 | val x = 1 // warn
| ^
| unused local definition
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:11:26 --------------------------------------------------------
11 | import SomeGivenImports.given // warn
| ^^^^^
| unused import
15 changes: 15 additions & 0 deletions tests/warn/i18559a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//> using options -Wall
// This test checks that -Wall turns on -Wunused:all if -Wunused is not set
object FooImportUnused:
import collection.mutable.Set // warn

object FooUnusedLocal:
def test(): Unit =
val x = 1 // warn

object FooGivenUnused:
import SomeGivenImports.given // warn

object SomeGivenImports:
given Int = 0
given String = "foo"
12 changes: 12 additions & 0 deletions tests/warn/i18559b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- Warning: tests/warn/i18559b.scala:8:6 -------------------------------------------------------------------------------
8 | val localFile: String = s"${url.##}.tmp" // warn
| ^
| Access non-initialized value localFile. Calling trace:
| -> class RemoteFile(url: String) extends AbstractFile: [ i18559b.scala:7 ]
| ^
| -> abstract class AbstractFile: [ i18559b.scala:3 ]
| ^
| -> val extension: String = name.substring(4) [ i18559b.scala:5 ]
| ^^^^
| -> def name: String = localFile [ i18559b.scala:9 ]
| ^^^^^^^^^
9 changes: 9 additions & 0 deletions tests/warn/i18559b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//> using options -Wall
// This test checks that -Wall turns on -Wsafe-init
abstract class AbstractFile:
def name: String
val extension: String = name.substring(4)

class RemoteFile(url: String) extends AbstractFile:
val localFile: String = s"${url.##}.tmp" // warn
def name: String = localFile
12 changes: 12 additions & 0 deletions tests/warn/i18559c.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:4:28 ---------------------------------------------------------
4 | import collection.mutable.Set // warn
| ^^^
| unused import
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:8:8 ----------------------------------------------------------
8 | val x = 1 // warn
| ^
| unused local definition
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:11:26 --------------------------------------------------------
11 | import SomeGivenImports.given // warn
| ^^^^^
| unused import
15 changes: 15 additions & 0 deletions tests/warn/i18559c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//> using options -Wall -Wunused:locals
// This test checks that -Wall overrides -Wunused:... if it is already set
object FooImportUnused:
import collection.mutable.Set // warn

object FooUnusedLocal:
def test(): Unit =
val x = 1 // warn

object FooGivenUnused:
import SomeGivenImports.given // warn

object SomeGivenImports:
given Int = 0
given String = "foo"
Loading