From e7e1c77092ec0d64a6ecbb8b920d1b4cca74837f Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Fri, 29 Jan 2016 17:56:21 -0800 Subject: Improved outer ref checking in pattern matches: The old algorithm omitted necessary outer ref checks in some places. This new one is more conservative. It only omits outer ref checks when the expected type and the scrutinee type match up, or when the expected type is defined in a static location. For this specific purpose the top level of a method or other code block (which is not a trait or class definition) is also considered static because it does not have a prefix. This change comes with a spec update to clarify the prefix rule for type patterns. The new wording makes it clear that the presence of a prefix is to be interpreted in a *semantic* way, i.e. the existence of a prefix determines the necessity for an outer ref check, no matter if the prefix is actually spelled out *syntactically*. Note that the old outer ref check implementation did not use the alternative interpretation of requiring prefixes to be given syntactically. It never created an outer ref check for a local class `C`, no matter if the pattern was `_: C` or `_: this.C`, thus violating both interpretations of the spec. There is now explicit support for unchecked matches (like `case _: (T @unchecked) =>`) to suppress warnings for unchecked outer refs. `@unchecked` worked before and was used for this purpose in `neg/t7721` but never actually existed as a feature. It was a result of a bug that prevented an outer ref check from being generated in the first place if *any* annotation was used on an expected type in a type pattern. This new version will still generate the outer ref check if an outer ref is available but suppress the warning otherwise. Other annotations on type patterns are ignored. New tests are in `neg/outer-ref-checks`. The expected results of tests `neg/t7171` and `neg/t7171b` have changed because the compiler now tries to generate additional outer ref checks that were not present before (which was a bug). --- test/files/neg/outer-ref-checks.check | 24 ++++++++ test/files/neg/outer-ref-checks.flags | 1 + test/files/neg/outer-ref-checks.scala | 106 ++++++++++++++++++++++++++++++++++ test/files/neg/t7171.check | 5 +- test/files/neg/t7171b.check | 8 ++- test/files/run/t7171.check | 3 + 6 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/outer-ref-checks.check create mode 100644 test/files/neg/outer-ref-checks.flags create mode 100644 test/files/neg/outer-ref-checks.scala (limited to 'test/files') diff --git a/test/files/neg/outer-ref-checks.check b/test/files/neg/outer-ref-checks.check new file mode 100644 index 0000000000..bba7118d79 --- /dev/null +++ b/test/files/neg/outer-ref-checks.check @@ -0,0 +1,24 @@ +outer-ref-checks.scala:5: warning: The outer reference in this type test cannot be checked at run time. + final case class Inner(val s: String) // unchecked warning + ^ +outer-ref-checks.scala:8: warning: The outer reference in this type test cannot be checked at run time. + case Inner(s) => // unchecked warning + ^ +outer-ref-checks.scala:18: warning: The outer reference in this type test cannot be checked at run time. + case Inner(s) => // unchecked warning + ^ +outer-ref-checks.scala:19: warning: The outer reference in this type test cannot be checked at run time. + case O.Inner(s) => // unchecked warning + ^ +outer-ref-checks.scala:41: warning: The outer reference in this type test cannot be checked at run time. + case Inner(s) => // unchecked warning + ^ +outer-ref-checks.scala:46: warning: The outer reference in this type test cannot be checked at run time. + case _: Inner => // unchecked warning + ^ +outer-ref-checks.scala:56: warning: The outer reference in this type test cannot be checked at run time. + case _: (Inner @uncheckedVariance) => // unchecked warning + ^ +error: No warnings can be incurred under -Xfatal-warnings. +7 warnings found +one error found diff --git a/test/files/neg/outer-ref-checks.flags b/test/files/neg/outer-ref-checks.flags new file mode 100644 index 0000000000..464cc20ea6 --- /dev/null +++ b/test/files/neg/outer-ref-checks.flags @@ -0,0 +1 @@ +-Xfatal-warnings -unchecked \ No newline at end of file diff --git a/test/files/neg/outer-ref-checks.scala b/test/files/neg/outer-ref-checks.scala new file mode 100644 index 0000000000..35983fe92b --- /dev/null +++ b/test/files/neg/outer-ref-checks.scala @@ -0,0 +1,106 @@ +import scala.annotation.unchecked.uncheckedVariance + +class Outer { + // A final class gets no outer ref, so we expect to see warnings where an outer ref check should be performed + final case class Inner(val s: String) // unchecked warning + + def belongs(a: Any): Unit = a match { + case Inner(s) => // unchecked warning + case _ => + } + + def belongsStaticSameOuter(a: Inner): Unit = a match { + case Inner(s) => // no need for outer check + // match is exhaustive, no default case needed + } + + def belongsOtherOuter(a: Outer#Inner): Unit = a match { + case Inner(s) => // unchecked warning + case O.Inner(s) => // unchecked warning + case _ => + } +} + +object O extends Outer { + def belongsStaticSameOuter2(a: Inner): Unit = a match { + case Inner(s) => // no need for outer check + // match is exhaustive, no default case needed + } + + def belongsStaticSameOuter3(a: Inner): Unit = a match { + case _: Inner => // no need for outer check + // match is exhaustive, no default case needed + } + + def belongsStaticSameOuter4(a: Inner): Unit = a match { + case _: (Inner @uncheckedVariance) => // no need for outer check + // match is exhaustive, no default case needed + } + + def belongsOtherOuter2(a: Outer#Inner): Unit = a match { + case Inner(s) => // unchecked warning + case _ => + } + + def belongsOtherOuter3(a: Outer#Inner): Unit = a match { + case _: Inner => // unchecked warning + case _ => + } + + def belongsOtherOuter4(a: Outer#Inner): Unit = a match { + case _: (Inner @unchecked) => // warning supressed + case _ => + } + + def belongsOtherOuter5(a: Outer#Inner): Unit = a match { + case _: (Inner @uncheckedVariance) => // unchecked warning + case _ => + } + + def nested: Unit = { + final case class I(s: String) + + def check1(a: Any): Unit = a match { + case I(s) => // no need for outer check + case _ => + } + + def check2(a: I): Unit = a match { + case I(s) => // no need for outer check + // match is exhaustive, no default case needed + } + } +} + +class O2 { + def nested: Unit = { + final case class I(s: String) + + def check1(a: Any): Unit = a match { + case I(s) => // no need for outer check (is this correct?) + case _ => + } + + def check2(a: I): Unit = a match { + case I(s) => // no need for outer check (is this correct?) + // match is exhaustive, no default case needed + } + } +} + +package p { + object T { + case class C(x: Int) + } +} + +object U { + val T = p.T +} + +class Test { + def m(a: Any) = a match { + case U.T.C(1) => 1 // used to warn + case _ => 1 + } +} diff --git a/test/files/neg/t7171.check b/test/files/neg/t7171.check index ecd768afda..2de9151483 100644 --- a/test/files/neg/t7171.check +++ b/test/files/neg/t7171.check @@ -1,6 +1,9 @@ t7171.scala:2: warning: The outer reference in this type test cannot be checked at run time. final case class A() ^ +t7171.scala:9: warning: The outer reference in this type test cannot be checked at run time. + case _: A => true; case _ => false + ^ error: No warnings can be incurred under -Xfatal-warnings. -one warning found +two warnings found one error found diff --git a/test/files/neg/t7171b.check b/test/files/neg/t7171b.check index bf695afea7..6b05b6fa63 100644 --- a/test/files/neg/t7171b.check +++ b/test/files/neg/t7171b.check @@ -1,6 +1,12 @@ t7171b.scala:2: warning: The outer reference in this type test cannot be checked at run time. final case class A() ^ +t7171b.scala:8: warning: The outer reference in this type test cannot be checked at run time. + case _: A => true; case _ => false + ^ +t7171b.scala:13: warning: The outer reference in this type test cannot be checked at run time. + case _: A => true; case _ => false + ^ error: No warnings can be incurred under -Xfatal-warnings. -one warning found +three warnings found one error found diff --git a/test/files/run/t7171.check b/test/files/run/t7171.check index d826f6cb94..5454142882 100644 --- a/test/files/run/t7171.check +++ b/test/files/run/t7171.check @@ -1,3 +1,6 @@ t7171.scala:2: warning: The outer reference in this type test cannot be checked at run time. final case class A() ^ +t7171.scala:9: warning: The outer reference in this type test cannot be checked at run time. + case _: A => true; case _ => false + ^ -- cgit v1.2.3