From d82dfcc0f57b7693b185e68f1457f70cc4b25133 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Mon, 4 Apr 2022 12:07:56 -0700 Subject: [PATCH 1/3] fix vulpix rewrite testing previously, the test run would complete with success despite the rewrite test itself failing. --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index cf051e5f01ad..5a57ede4ceb5 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -1064,7 +1064,13 @@ trait ParallelTesting extends RunnerOrchestration { self => target.copy(dir = copyToDir(outDir, dir)) } - new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite() + val test = new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite() + + cleanup() + + if test.didFail then + fail("Rewrite test failed") + this } From b481090fb9ce833fffb8304b34dbc752f674bd40 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Wed, 30 Mar 2022 18:29:01 -0700 Subject: [PATCH 2/3] fix rewrites for filtering for generators the test added here used to fail because the `case` was patched inside the parens, e.g. `for (case x: String) ...` which is not valid --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- compiler/test/dotty/tools/dotc/CompilationTests.scala | 1 + tests/rewrites/filtering-fors.check | 6 ++++++ tests/rewrites/filtering-fors.scala | 6 ++++++ 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 tests/rewrites/filtering-fors.check create mode 100644 tests/rewrites/filtering-fors.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6c1767f6ef83..c83bb569c7bb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1603,7 +1603,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef if (!checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration`) if (isPatDef) patch(Span(tree.selector.span.end), ": @unchecked") - else patch(Span(pat.span.start), "case ") + else patch(Span(tree.span.start), "case ") // skip exhaustivity check in later phase // TODO: move the check above to patternMatcher phase diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 6b2af4c1ff85..5bafac7686e5 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -73,6 +73,7 @@ class CompilationTests { aggregateTests( compileFile("tests/rewrites/rewrites.scala", scala2CompatMode.and("-rewrite", "-indent")), compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")), + compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "future-migration")), compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")), diff --git a/tests/rewrites/filtering-fors.check b/tests/rewrites/filtering-fors.check new file mode 100644 index 000000000000..f6cac26d8629 --- /dev/null +++ b/tests/rewrites/filtering-fors.check @@ -0,0 +1,6 @@ +val xs: List[Any] = ??? +val as = for case (x: String) <- xs yield x +val bs = + for + case (x: String) <- xs + yield x diff --git a/tests/rewrites/filtering-fors.scala b/tests/rewrites/filtering-fors.scala new file mode 100644 index 000000000000..e3b312186a14 --- /dev/null +++ b/tests/rewrites/filtering-fors.scala @@ -0,0 +1,6 @@ +val xs: List[Any] = ??? +val as = for (x: String) <- xs yield x +val bs = + for + (x: String) <- xs + yield x From c818e0a158a3ce97a444c983db78f5c094fe9294 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Thu, 31 Mar 2022 11:36:27 -0700 Subject: [PATCH 3/3] fix rewrites for refutable pattern bindings needing `@unchecked` over compound expressions --- .../src/dotty/tools/dotc/typer/Typer.scala | 24 +++++++++++++++--- .../dotty/tools/dotc/CompilationTests.scala | 1 + .../rewrites/refutable-pattern-bindings.check | 25 +++++++++++++++++++ .../rewrites/refutable-pattern-bindings.scala | 25 +++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/rewrites/refutable-pattern-bindings.check create mode 100644 tests/rewrites/refutable-pattern-bindings.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index c83bb569c7bb..9ac83c570f11 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1596,14 +1596,32 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer typedMatchFinish(tree, sel1, selType, tree.cases, pt) } + /** Are some form of brackets necessary to annotate the tree `sel` as `@unchecked`? + * If so, return a Some(opening bracket, closing bracket), otherwise None. + */ + def uncheckedBrackets(sel: untpd.Tree): Option[(String, String)] = sel match + case _: untpd.If + | _: untpd.Match + | _: untpd.ForYield + | _: untpd.ParsedTry + | _: untpd.Try => Some("(", ")") + case _: untpd.Block => Some("{", "}") + case _ => None + result match { case result @ Match(sel, CaseDef(pat, _, _) :: _) => tree.selector.removeAttachment(desugar.CheckIrrefutable) match { case Some(checkMode) if !sel.tpe.hasAnnotation(defn.UncheckedAnnot) => val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef - if (!checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration`) - if (isPatDef) patch(Span(tree.selector.span.end), ": @unchecked") - else patch(Span(tree.span.start), "case ") + if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration` then + if isPatDef then uncheckedBrackets(tree.selector) match + case None => + patch(Span(tree.selector.span.end), ": @unchecked") + case Some(bl, br) => + patch(Span(tree.selector.span.start), s"$bl") + patch(Span(tree.selector.span.end), s"$br: @unchecked") + else + patch(Span(tree.span.start), "case ") // skip exhaustivity check in later phase // TODO: move the check above to patternMatcher phase diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 5bafac7686e5..1fb14ec1d858 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -74,6 +74,7 @@ class CompilationTests { compileFile("tests/rewrites/rewrites.scala", scala2CompatMode.and("-rewrite", "-indent")), compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")), compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "future-migration")), + compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "future-migration")), compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")), diff --git a/tests/rewrites/refutable-pattern-bindings.check b/tests/rewrites/refutable-pattern-bindings.check new file mode 100644 index 000000000000..5acea5dc0ead --- /dev/null +++ b/tests/rewrites/refutable-pattern-bindings.check @@ -0,0 +1,25 @@ +val xs: List[Any] = ??? + +val hd :: tl = (xs match + case Nil => null :: xs + case _ => xs): @unchecked + +val h :: t = xs: @unchecked + +val a :: b = + (if xs.isEmpty then null :: xs + else xs): @unchecked + +val c :: d = + (try xs.head :: xs + catch case _: NoSuchElementException => null :: xs): @unchecked + +val e :: f = + {val zero = null :: Nil + if xs.isEmpty then zero + else xs}: @unchecked + +val j :: k = + (for + case (x: String) <- xs + yield x): @unchecked diff --git a/tests/rewrites/refutable-pattern-bindings.scala b/tests/rewrites/refutable-pattern-bindings.scala new file mode 100644 index 000000000000..e724c9564bd2 --- /dev/null +++ b/tests/rewrites/refutable-pattern-bindings.scala @@ -0,0 +1,25 @@ +val xs: List[Any] = ??? + +val hd :: tl = xs match + case Nil => null :: xs + case _ => xs + +val h :: t = xs + +val a :: b = + if xs.isEmpty then null :: xs + else xs + +val c :: d = + try xs.head :: xs + catch case _: NoSuchElementException => null :: xs + +val e :: f = + val zero = null :: Nil + if xs.isEmpty then zero + else xs + +val j :: k = + for + (x: String) <- xs + yield x