-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Allow discarding "Discarded non-Unit" warnings with : Unit
#21927
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,6 +81,9 @@ object Typer { | |
/** Indicates that a definition was copied over from the parent refinements */ | ||
val RefinementFromParent = new Property.StickyKey[Unit] | ||
|
||
/** Indicates that an expression is explicitly ascribed to [[Unit]] type. */ | ||
val AscribedToUnit = new Property.StickyKey[Unit] | ||
|
||
/** An attachment on a Select node with an `apply` field indicating that the `apply` | ||
* was inserted by the Typer. | ||
*/ | ||
|
@@ -1193,7 +1196,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
else tpt | ||
val expr1 = | ||
if isWildcard then tree.expr.withType(underlyingTreeTpe.tpe) | ||
else typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) | ||
else | ||
if underlyingTreeTpe.tpe.isRef(defn.UnitClass) then | ||
untpd.unsplice(tree.expr).putAttachment(AscribedToUnit, ()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) | ||
assignType(cpy.Typed(tree)(expr1, tpt), underlyingTreeTpe) | ||
.withNotNullInfo(expr1.notNullInfo) | ||
} | ||
|
@@ -3377,7 +3383,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
else if (ctx.mode.is(Mode.Pattern)) | ||
typedUnApply(cpy.Apply(tree)(op, l :: r :: Nil), pt) | ||
else { | ||
val app = typedApply(desugar.binop(l, op, r), pt) | ||
val app = typedApply(desugar.binop(l, op, r).withAttachmentsFrom(tree), pt) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed to add |
||
if op.name.isRightAssocOperatorName && !ctx.mode.is(Mode.QuotedExprPattern) then | ||
val defs = new mutable.ListBuffer[Tree] | ||
def lift(app: Tree): Tree = (app: @unchecked) match | ||
|
@@ -4581,9 +4587,14 @@ 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.Whas.valueDiscard && !isThisTypeResult(tree)) { | ||
|
||
if ctx.settings.Whas.valueDiscard | ||
&& !ctx.isAfterTyper | ||
&& !tree.isInstanceOf[Inlined] | ||
&& !isThisTypeResult(tree) | ||
&& !tree.hasAttachment(AscribedToUnit) then | ||
report.warning(ValueDiscarding(tree.tpe), tree.srcPos) | ||
} | ||
|
||
return tpd.Block(tree1 :: Nil, unitLiteral) | ||
} | ||
|
||
|
@@ -4839,6 +4850,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
// sometimes we do not have the original anymore and use the transformed tree instead. | ||
// But taken together, the two criteria are quite accurate. | ||
missingArgs(tree, tree.tpe.widen) | ||
case _ if tree.hasAttachment(AscribedToUnit) => | ||
// The tree was ascribed to `Unit` explicitly to silence the warning. | ||
() | ||
case _ if isUnitExpr => | ||
report.warning(PureUnitExpression(original, tree.tpe), original.srcPos) | ||
case _ => | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,9 @@ | ||
-- [E190] Potential Issue Warning: tests/neg/i13091.scala:7:17 --------------------------------------------------------- | ||
7 |def test: Unit = new Foo // error: class Foo is marked @experimental ... | ||
| ^^^^^^^ | ||
| Discarded non-Unit value of type Foo. You may want to use `()`. | ||
-- Error: tests/neg/i13091.scala:7:16 ---------------------------------------------------------------------------------- | ||
7 |def test = (new Foo): Unit // error: class Foo is marked @experimental ... | ||
| ^^^ | ||
| class Foo is marked @experimental | ||
| | ||
| longer explanation available when compiling with `-explain` | ||
-- Error: tests/neg/i13091.scala:7:21 ---------------------------------------------------------------------------------- | ||
7 |def test: Unit = new Foo // error: class Foo is marked @experimental ... | ||
| ^^^ | ||
| class Foo is marked @experimental | ||
| | ||
| Experimental definition may only be used under experimental mode: | ||
| 1. in a definition marked as @experimental, or | ||
| 2. an experimental feature is imported at the package level, or | ||
| 3. compiling with the -experimental compiler flag. | ||
| Experimental definition may only be used under experimental mode: | ||
| 1. in a definition marked as @experimental, or | ||
| 2. an experimental feature is imported at the package level, or | ||
| 3. compiling with the -experimental compiler flag. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,4 +4,4 @@ import annotation.experimental | |
|
||
@experimental class Foo | ||
|
||
def test: Unit = new Foo // error: class Foo is marked @experimental ... | ||
def test = (new Foo): Unit // error: class Foo is marked @experimental ... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoids the "non-Unit" warning, that is not directly related to this test. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,6 @@ object Test: | |
object Test2: | ||
|
||
if true then | ||
1 | ||
() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoids the "non-Unit" warning, that is not directly related to this test. |
||
else 2 // error | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
-- [E190] Potential Issue Warning: tests/warn/21557.scala:9:16 --------------------------------------------------------- | ||
9 | val x: Unit = 1 + 1 // warn | ||
| ^^^^^ | ||
| Discarded non-Unit value of type Int. Add `: Unit` to discard silently. | ||
| | ||
| longer explanation available when compiling with `-explain` | ||
-- [E176] Potential Issue Warning: tests/warn/21557.scala:10:2 --------------------------------------------------------- | ||
10 | 1 + 1 // warn | ||
| ^^^^^ | ||
| unused value of type (2 : Int) | ||
-- [E175] Potential Issue Warning: tests/warn/21557.scala:15:52 -------------------------------------------------------- | ||
15 | val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type Assertion. Add `: Unit` to discard silently. | ||
-- [E176] Potential Issue Warning: tests/warn/21557.scala:16:41 -------------------------------------------------------- | ||
16 | new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| unused value of type Assertion |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
//> using options -Wvalue-discard -Wnonunit-statement | ||
|
||
class Assertion(assert: => Any): | ||
def shouldPass(): Assertion = ??? | ||
|
||
def test: Unit = | ||
1 + 1: Unit | ||
(1 + 1): Unit | ||
val x: Unit = 1 + 1 // warn | ||
1 + 1 // warn | ||
val y: Int = 1 + 1 | ||
|
||
new Assertion("").shouldPass(): Unit | ||
(new Assertion("").shouldPass()): Unit | ||
val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard) | ||
new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement) | ||
val y1: Assertion = new Assertion("another other").shouldPass() | ||
|
||
() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,20 @@ | ||
-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:27:36 ------------------------------------------- | ||
27 | mutable.Set.empty[String].remove("") // warn | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type Boolean | ||
| discarded non-Unit value of type Boolean. Add `: Unit` to discard silently. | ||
-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:39:41 ------------------------------------------- | ||
39 | mutable.Set.empty[String].subtractOne("") // warn | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type scala.collection.mutable.Set[String] | ||
| discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. | ||
-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:59:4 -------------------------------------------- | ||
59 | mutable.Set.empty[String] += "" // warn | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type scala.collection.mutable.Set[String] | ||
| discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. | ||
-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:15:35 ------------------------------------------- | ||
15 | firstThing().map(_ => secondThing()) // warn | ||
| ^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type Either[Failed, Unit] | ||
| discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently. | ||
-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:18:35 ------------------------------------------- | ||
18 | firstThing().map(_ => secondThing()) // warn | ||
| ^^^^^^^^^^^^^ | ||
| discarded non-Unit value of type Either[Failed, Unit] | ||
| discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's create actionable diagnostics in the next spree for both diagnostics