From ef19bcdc830936e07dd9ae0e628c4b4e64a181ac Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 1 Oct 2020 16:45:56 +0200 Subject: [PATCH 1/4] Intrinsify StringContext.f Ported the current macro implementation --- .../dotty/tools/dotc/core/Definitions.scala | 6 +- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../localopt/StringContextChecker.scala | 112 ++++++++-------- .../localopt/StringInterpolatorOpt.scala | 9 +- .../src/dotty/tools/dotc/typer/Typer.scala | 15 +-- .../dotty/internal/StringContextMacro.scala | 13 -- project/Build.scala | 1 - tests/neg/f-interpolator-neg.scala | 83 ++++++++++++ .../f-interpolator-neg/Macros_1.scala | 73 ----------- .../f-interpolator-neg/Tests_2.scala | 121 ------------------ 10 files changed, 152 insertions(+), 282 deletions(-) rename library/src-bootstrapped/dotty/internal/StringContextMacro.scala => compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala (90%) delete mode 100644 library/src-non-bootstrapped/dotty/internal/StringContextMacro.scala create mode 100644 tests/neg/f-interpolator-neg.scala delete mode 100644 tests/run-macros/f-interpolator-neg/Macros_1.scala delete mode 100644 tests/run-macros/f-interpolator-neg/Tests_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 0894438a95cb..6da9cf131c32 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -553,6 +553,9 @@ class Definitions { @tu lazy val Seq_length : Symbol = SeqClass.requiredMethod(nme.length) @tu lazy val Seq_toSeq : Symbol = SeqClass.requiredMethod(nme.toSeq) + @tu lazy val StringOps: Symbol = requiredClass("scala.collection.StringOps") + @tu lazy val StringOps_format: Symbol = StringOps.requiredMethod(nme.format) + @tu lazy val ArrayType: TypeRef = requiredClassRef("scala.Array") def ArrayClass(using Context): ClassSymbol = ArrayType.symbol.asClass @tu lazy val Array_apply : Symbol = ArrayClass.requiredMethod(nme.apply) @@ -733,9 +736,6 @@ class Definitions { @tu lazy val StringContextModule_standardInterpolator: Symbol = StringContextModule.requiredMethod(nme.standardInterpolator) @tu lazy val StringContextModule_processEscapes: Symbol = StringContextModule.requiredMethod(nme.processEscapes) - @tu lazy val InternalStringContextMacroModule: Symbol = requiredModule("dotty.internal.StringContextMacro") - @tu lazy val InternalStringContextMacroModule_f: Symbol = InternalStringContextMacroModule.requiredMethod(nme.f) - @tu lazy val PartialFunctionClass: ClassSymbol = requiredClass("scala.PartialFunction") @tu lazy val PartialFunction_isDefinedAt: Symbol = PartialFunctionClass.requiredMethod(nme.isDefinedAt) @tu lazy val PartialFunction_applyOrElse: Symbol = PartialFunctionClass.requiredMethod(nme.applyOrElse) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 12c8d4ea817b..dc0bc7938255 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -477,6 +477,7 @@ object StdNames { val flagsFromBits : N = "flagsFromBits" val flatMap: N = "flatMap" val foreach: N = "foreach" + val format: N = "format" val fromDigits: N = "fromDigits" val fromProduct: N = "fromProduct" val genericArrayOps: N = "genericArrayOps" diff --git a/library/src-bootstrapped/dotty/internal/StringContextMacro.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala similarity index 90% rename from library/src-bootstrapped/dotty/internal/StringContextMacro.scala rename to compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala index d60da06cbd20..b91665986d68 100644 --- a/library/src-bootstrapped/dotty/internal/StringContextMacro.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala @@ -1,13 +1,20 @@ -// ALWAYS KEEP THIS FILE IN src-bootstrapped, DO NOT MOVE TO src - -package dotty.internal - -import scala.quoted._ - -object StringContextMacro { - - /** Implementation of scala.StringContext.f used in Dotty */ - inline def f(inline sc: StringContext)(inline args: Any*): String = ${ interpolate('sc, 'args) } +package dotty.tools.dotc +package transform.localopt + +import dotty.tools.dotc.ast.Trees._ +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.core.Decorators._ +import dotty.tools.dotc.core.Constants.Constant +import dotty.tools.dotc.core.Contexts._ +import dotty.tools.dotc.core.StdNames._ +import dotty.tools.dotc.core.NameKinds._ +import dotty.tools.dotc.core.Symbols._ +import dotty.tools.dotc.core.Types._ + +// Ported from old dotty.internal.StringContextMacro +// TODO: port Scala 2 logic? (see https://github.com/scala/scala/blob/2.13.x/src/compiler/scala/tools/reflect/FormatInterpolator.scala#L74) +object StringContextChecker { + import tpd._ /** This trait defines a tool to report errors/warnings that do not depend on Position. */ trait Reporter { @@ -51,24 +58,22 @@ object StringContextMacro { def restoreReported() : Unit } - /** Interpolates the arguments to the formatting String given inside a StringContext - * - * @param strCtxExpr the Expr that holds the StringContext which contains all the chunks of the formatting string - * @param args the Expr that holds the sequence of arguments to interpolate to the String in the correct format - * @return the Expr containing the formatted and interpolated String or an error/warning if the parameters are not correct - */ - private def interpolate(strCtxExpr: Expr[StringContext], argsExpr: Expr[Seq[Any]])(using qctx: QuoteContext): Expr[String] = { - import qctx.tasty._ - val sourceFile = strCtxExpr.unseal.pos.sourceFile - - val (partsExpr, parts) = strCtxExpr match { - case Expr.StringContext(p1 as Consts(p2)) => (p1.toList, p2.toList) - case _ => report.throwError("Expected statically known String Context", strCtxExpr) + /** Check the format of the parts of the f".." arguments and returns the string parts of the StringContext */ + def checkedParts(strContext_f: Tree, args0: Tree)(using Context): String = { + + val (partsExpr, parts) = strContext_f match { + case TypeApply(Select(Apply(_, (parts: SeqLiteral) :: Nil), _), _) => + (parts.elems, parts.elems.map { case Literal(Constant(str: String)) => str } ) + case _ => + report.error("Expected statically known String Context", strContext_f.srcPos) + return "" } - val args = argsExpr match { - case Varargs(args) => args - case _ => report.throwError("Expected statically known argument list", argsExpr) + val args = args0 match { + case args: SeqLiteral => args.elems + case _ => + report.error("Expected statically known argument list", args0.srcPos) + return "" } val reporter = new Reporter{ @@ -76,28 +81,29 @@ object StringContextMacro { private[this] var oldReported = false def partError(message : String, index : Int, offset : Int) : Unit = { reported = true - val positionStart = partsExpr(index).unseal.pos.start + offset - error(message, sourceFile, positionStart, positionStart) + val pos = partsExpr(index).sourcePos + val posOffset = pos.withSpan(pos.span.shift(offset)) + report.error(message, posOffset) } def partWarning(message : String, index : Int, offset : Int) : Unit = { reported = true - val positionStart = partsExpr(index).unseal.pos.start + offset - warning(message, sourceFile, positionStart, positionStart) + val pos = partsExpr(index).sourcePos + val posOffset = pos.withSpan(pos.span.shift(offset)) + report.warning(message, posOffset) } def argError(message : String, index : Int) : Unit = { reported = true - error(message, args(index).unseal.pos) + report.error(message, args(index).srcPos) } def strCtxError(message : String) : Unit = { reported = true - val positionStart = strCtxExpr.unseal.pos.start - error(message, sourceFile, positionStart, positionStart) + report.error(message, strContext_f.srcPos) } def argsError(message : String) : Unit = { reported = true - error(message, argsExpr.unseal.pos) + report.error(message, args0.srcPos) } def hasReported() : Boolean = { @@ -114,18 +120,11 @@ object StringContextMacro { } } - interpolate(parts, args, argsExpr, reporter) + checked(parts, args, reporter) } - /** Helper function for the interpolate function above - * - * @param partsExpr the list of parts enumerated as Expr - * @param args the list of arguments enumerated as Expr - * @param reporter the reporter to return any error/warning when a problem is encountered - * @return the Expr containing the formatted and interpolated String or an error/warning report if the parameters are not correct - */ - def interpolate(parts0 : List[String], args : Seq[Expr[Any]], argsExpr: Expr[Seq[Any]], reporter : Reporter)(using qctx: QuoteContext) : Expr[String] = { - import qctx.tasty._ + def checked(parts0: List[String], args: List[Tree], reporter: Reporter)(using Context): String = { + /** Checks if the number of arguments are the same as the number of formatting strings * @@ -585,11 +584,11 @@ object StringContextMacro { * nothing otherwise */ def checkTypeWithArgs(argument : (Type, Int), conversionChar : Char, partIndex : Int, flags : List[(Char, Int)]) = { - val booleans = List(Type.of[Boolean], Type.of[Null]) - val dates = List(Type.of[Long], Type.of[java.util.Calendar], Type.of[java.util.Date]) - val floatingPoints = List(Type.of[Double], Type.of[Float], Type.of[java.math.BigDecimal]) - val integral = List(Type.of[Int], Type.of[Long], Type.of[Short], Type.of[Byte], Type.of[java.math.BigInteger]) - val character = List(Type.of[Char], Type.of[Byte], Type.of[Short], Type.of[Int]) + val booleans = List(defn.BooleanType, defn.NullType) + val dates = List(defn.LongType, requiredClass("java.util.Calendar").typeRef, requiredClass("java.util.Date").typeRef) + val floatingPoints = List(defn.DoubleType, defn.FloatType, requiredClass("java.math.BigDecimal").typeRef) + val integral = List(defn.IntType, defn.LongType, defn.ShortType, defn.ByteType, requiredClass("java.math.BigInteger").typeRef) + val character = List(defn.CharType, defn.ByteType, defn.ShortType, defn.IntType) val (argType, argIndex) = argument conversionChar match { @@ -597,9 +596,9 @@ object StringContextMacro { case 'd' | 'o' | 'x' | 'X' => { checkSubtype(argType, "Int", argIndex, integral : _*) if (conversionChar != 'd') { - val notAllowedFlagOnCondition = List(('+', !(argType <:< Type.of[java.math.BigInteger]), "only use '+' for BigInt conversions to o, x, X"), - (' ', !(argType <:< Type.of[java.math.BigInteger]), "only use ' ' for BigInt conversions to o, x, X"), - ('(', !(argType <:< Type.of[java.math.BigInteger]), "only use '(' for BigInt conversions to o, x, X"), + val notAllowedFlagOnCondition = List(('+', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use '+' for BigInt conversions to o, x, X"), + (' ', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use ' ' for BigInt conversions to o, x, X"), + ('(', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use '(' for BigInt conversions to o, x, X"), (',', true, "',' only allowed for d conversion of integral types")) checkFlags(partIndex, flags, notAllowedFlagOnCondition : _*) } @@ -608,7 +607,7 @@ object StringContextMacro { case 't' | 'T' => checkSubtype(argType, "Date", argIndex, dates : _*) case 'b' | 'B' => checkSubtype(argType, "Boolean", argIndex, booleans : _*) case 'h' | 'H' | 'S' | 's' => - if (!(argType <:< Type.of[java.util.Formattable])) + if !(argType <:< requiredClass("java.util.Formattable").typeRef) then for {flag <- flags ; if (flag._1 == '#')} reporter.argError("type mismatch;\n found : " + argType.widen.show.stripPrefix("scala.Predef.").stripPrefix("java.lang.").stripPrefix("scala.") + "\n required: java.util.Formattable", argIndex) case 'n' | '%' => @@ -647,7 +646,7 @@ object StringContextMacro { * @param maxArgumentIndex an Option containing the maximum argument index possible, None if no args are specified * @return a list with all the elements of the conversion per formatting string */ - def checkPart(part : String, start : Int, argument : Option[(Int, Expr[Any])], maxArgumentIndex : Option[Int]) : List[(Option[(Type, Int)], Char, List[(Char, Int)])] = { + def checkPart(part : String, start : Int, argument : Option[(Int, Tree)], maxArgumentIndex : Option[Int]) : List[(Option[(Type, Int)], Char, List[(Char, Int)])] = { reporter.resetReported() val hasFormattingSubstring = getFormattingSubstring(part, part.size, start) if (hasFormattingSubstring.nonEmpty) { @@ -658,7 +657,7 @@ object StringContextMacro { case Some(argIndex, arg) => { val (hasArgumentIndex, argumentIndex, flags, hasWidth, width, hasPrecision, precision, hasRelative, relativeIndex, conversion) = getFormatSpecifiers(part, argIndex, argIndex + 1, false, formattingStart) if (!reporter.hasReported()){ - val conversionWithType = checkFormatSpecifiers(argIndex + 1, hasArgumentIndex, argumentIndex, Some(argIndex + 1), start == 0, maxArgumentIndex, hasRelative, hasWidth, width, hasPrecision, precision, flags, conversion, Some(arg.unseal.tpe), part) + val conversionWithType = checkFormatSpecifiers(argIndex + 1, hasArgumentIndex, argumentIndex, Some(argIndex + 1), start == 0, maxArgumentIndex, hasRelative, hasWidth, width, hasPrecision, precision, flags, conversion, Some(arg.tpe), part) nextStart = conversion + 1 conversionWithType :: checkPart(part, nextStart, argument, maxArgumentIndex) } else checkPart(part, conversion + 1, argument, maxArgumentIndex) @@ -710,7 +709,6 @@ object StringContextMacro { } } - // macro expansion - '{(${Expr(parts.mkString)}).format(${argsExpr}: _*)} + parts.mkString } } diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index ff198f415baa..e0a565737fff 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -7,8 +7,9 @@ import dotty.tools.dotc.core.Decorators._ import dotty.tools.dotc.core.Constants.Constant import dotty.tools.dotc.core.Contexts._ import dotty.tools.dotc.core.StdNames._ +import dotty.tools.dotc.core.NameKinds._ import dotty.tools.dotc.core.Symbols._ -import dotty.tools.dotc.core.Types.MethodType +import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.transform.MegaPhase.MiniPhase /** @@ -116,6 +117,7 @@ class StringInterpolatorOpt extends MiniPhase { val sym = tree.symbol val isInterpolatedMethod = // Test names first to avoid loading scala.StringContext if not used (sym.name == nme.raw_ && sym.eq(defn.StringContext_raw)) || + (sym.name == nme.f && sym.eq(defn.StringContext_f)) || (sym.name == nme.s && sym.eq(defn.StringContext_s)) if (isInterpolatedMethod) tree match { @@ -132,6 +134,11 @@ class StringInterpolatorOpt extends MiniPhase { if (!str.const.stringValue.isEmpty) concat(str) } result + case Apply(intp, args :: Nil) if sym.eq(defn.StringContext_f) => + val partsStr = StringContextChecker.checkedParts(intp, args).mkString + resolveConstructor(defn.StringOps.typeRef, List(Literal(Constant(partsStr)))) + .select(nme.format) + .appliedTo(args) // Starting with Scala 2.13, s and raw are macros in the standard // library, so we need to expand them manually. // sc.s(args) --> standardInterpolator(processEscapes, args, sc.parts) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3099660b951c..9e9e8ba05ec4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3257,21 +3257,10 @@ class Typer extends Namer if ((inlined ne tree) && errorCount == ctx.reporter.errorCount) readaptSimplified(inlined) else inlined } - else if tree.symbol.name == nme.f && tree.symbol == defn.StringContext_f then - // To avoid forcing StringContext_f when compiling StingContex - // we test the name before accession symbol StringContext_f. - - // As scala.StringContext.f is defined in the standard library which - // we currently do not bootstrap we cannot implement the macro in the library. - // To overcome the current limitation we intercept the call and rewrite it into - // a call to dotty.internal.StringContext.f which we can implement using the new macros. - // As the macro is implemented in the bootstrapped library, it can only be used from the bootstrapped compiler. - val Apply(TypeApply(Select(sc, _), _), args) = tree - val newCall = ref(defn.InternalStringContextMacroModule_f).appliedTo(sc).appliedToArgs(args).withSpan(tree.span) - readaptSimplified(Inliner.inlineCall(newCall)) else if (tree.symbol.isScala2Macro && - // raw and s are eliminated by the StringInterpolatorOpt phase + // `raw`, `f` and `s` are eliminated by the StringInterpolatorOpt phase tree.symbol != defn.StringContext_raw && + tree.symbol != defn.StringContext_f && tree.symbol != defn.StringContext_s) if (ctx.settings.XignoreScala2Macros.value) { report.warning("Scala 2 macro cannot be used in Dotty, this call will crash at runtime. See https://dotty.epfl.ch/docs/reference/dropped-features/macros.html", tree.srcPos.startPos) diff --git a/library/src-non-bootstrapped/dotty/internal/StringContextMacro.scala b/library/src-non-bootstrapped/dotty/internal/StringContextMacro.scala deleted file mode 100644 index b3b7c443c332..000000000000 --- a/library/src-non-bootstrapped/dotty/internal/StringContextMacro.scala +++ /dev/null @@ -1,13 +0,0 @@ -// ALWAYS KEEP THIS FILE IN src-non-bootstrapped - -package dotty.internal - -import scala.quoted._ - -object StringContextMacro { - - /** Implementation of scala.StringContext.f used in Dotty */ - inline def f(inline sc: StringContext)(args: Any*): String = - scala.compiletime.error("Cannot expand f interpolator while bootstrapping the compiler") - -} diff --git a/project/Build.scala b/project/Build.scala index 5f2d13a1b10a..aec06d8b81d0 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1045,7 +1045,6 @@ object Build { )).get ++ (dir / "js/src/test/scala/org/scalajs/testsuite/javalib" ** (("*.scala": FileFilter) - -- "FormatterJSTest.scala" // compile error with the f"" interpolator -- "ObjectJSTest.scala" // non-native JS classes )).get diff --git a/tests/neg/f-interpolator-neg.scala b/tests/neg/f-interpolator-neg.scala new file mode 100644 index 000000000000..176d1fc5edf2 --- /dev/null +++ b/tests/neg/f-interpolator-neg.scala @@ -0,0 +1,83 @@ +object Test { + + def numberArgumentsTests(s : String, d : Int) = { + new StringContext().f() // error + new StringContext("", " is ", "%2d years old").f(s) // error + new StringContext("", " is ", "%2d years old").f(s, d, d) // error + new StringContext("", "").f() // error + } + + def interpolationMismatches(s : String, f : Double, b : Boolean) = { + f"$s%b" // error + f"$s%c" // error + f"$f%c" // error + f"$s%x" // error + f"$b%d" // error + f"$s%d" // error + f"$f%o" // error + f"$s%e" // error + f"$b%f" // error + f"$s%i" // error + } + + def flagMismatches(s : String, c : Char, d : Int, f : Double, t : java.util.Date) = { + f"$s%+ 0,(s" // error + f"$c%#+ 0,(c" // error + f"$d%#d" // error + f"$d%,x" // error + f"$d%+ (x" // error + f"$f%,(a" // error + f"$t%#+ 0,(tT" // error + f"%-#+ 0,(n" // error + f"%#+ 0,(%" // error + } + + def badPrecisions(c : Char, d : Int, f : Double, t : java.util.Date) = { + f"$c%.2c" // error + f"$d%.2d" // error + f"%.2%" // error + f"%.2n" // error + f"$f%.2a" // error + f"$t%.2tT" // error + } + + def badIndexes() = { + f"% - fooErrorsImpl(parts, args, argsExpr) - case ('{ new StringContext(${Varargs(parts)}: _*) }, Varargs(args)) => - fooErrorsImpl(parts, args, argsExpr) - } - } - - def fooErrorsImpl(parts0: Seq[Expr[String]], args: Seq[Expr[Any]], argsExpr: Expr[Seq[Any]])(using QuoteContext)= { - val errors = List.newBuilder[Expr[(Boolean, Int, Int, Int, String)]] - // true if error, false if warning - // 0 if part, 1 if arg, 2 if strCtx, 3 if args - // index in the list if arg or part, -1 otherwise - // offset, 0 if strCtx, args or arg - // message as given - val reporter = new dotty.internal.StringContextMacro.Reporter{ - private[this] var reported = false - private[this] var oldReported = false - def partError(message : String, index : Int, offset : Int) : Unit = { - reported = true - errors += '{ Tuple5(true, 0, ${Expr(index)}, ${Expr(offset)}, ${Expr(message)}) } - } - def partWarning(message : String, index : Int, offset : Int) : Unit = { - reported = true - errors += '{ Tuple5(false, 0, ${Expr(index)}, ${Expr(offset)}, ${Expr(message)}) } - } - - def argError(message : String, index : Int) : Unit = { - reported = true - errors += '{ Tuple5(true, 1, ${Expr(index)}, 0, ${Expr(message)}) } - } - - def strCtxError(message : String) : Unit = { - reported = true - errors += '{ Tuple5(true, 2, -1, 0, ${Expr(message)}) } - } - def argsError(message : String) : Unit = { - reported = true - errors += '{ Tuple5(true, 3, -1, 0, ${Expr(message)}) } - } - - def hasReported() : Boolean = { - reported - } - - def resetReported() : Unit = { - oldReported = reported - reported = false - } - - def restoreReported() : Unit = { - reported = oldReported - } - } - val parts = parts0.map { case Const(s) => s } - dotty.internal.StringContextMacro.interpolate(parts.toList, args.toList, argsExpr, reporter) // Discard result - Expr.ofList(errors.result()) - } -} \ No newline at end of file diff --git a/tests/run-macros/f-interpolator-neg/Tests_2.scala b/tests/run-macros/f-interpolator-neg/Tests_2.scala deleted file mode 100644 index cc5af3d1b1cc..000000000000 --- a/tests/run-macros/f-interpolator-neg/Tests_2.scala +++ /dev/null @@ -1,121 +0,0 @@ -object Test { - - def main(args: Array[String]): Unit = { - val s = "Scala" - val d = 8 - val b = false - val f = 3.14159 - val c = 'c' - val t = new java.util.Date - val x = new java.util.Formattable { - def formatTo(ff: java.util.Formatter, g: Int, w: Int, p: Int): Unit = ff format "xxx" - } - numberArgumentsTests(s, d) - interpolationMismatches(s, f, b) - flagMismatches(s, c, d, f, t) - badPrecisions(c, d, f, t) - badIndexes() - warnings(s) - badArgTypes(s) - misunderstoodConversions(t, s) - otherBrainFailures(d) - } - - def numberArgumentsTests(s : String, d : Int) = { - import TestFooErrors._ - assertEquals(new StringContext().foo(), List((true, 2, -1, 0, "there are no parts"))) - assertEquals(new StringContext("", " is ", "%2d years old").foo(s), List((true, 1, 0, 0, "too few arguments for interpolated string"))) - assertEquals(new StringContext("", " is ", "%2d years old").foo(s, d, d), List((true, 1, 2, 0, "too many arguments for interpolated string"))) - assertEquals(new StringContext("", "").foo(), List((true, 3, -1, 0, "too few arguments for interpolated string"))) - } - - def interpolationMismatches(s : String, f : Double, b : Boolean) = { - import TestFooErrors._ - assertEquals(foo"$s%b", List((true, 1, 0, 0, "type mismatch;\n found : String\n required: Boolean"))) - assertEquals(foo"$s%c", List((true, 1, 0, 0, "type mismatch;\n found : String\n required: Char"))) - assertEquals(foo"$f%c", List((true, 1, 0, 0, "type mismatch;\n found : Double\n required: Char"))) - assertEquals(foo"$s%x", List((true, 1, 0, 0, "type mismatch;\n found : String\n required: Int"))) - assertEquals(foo"$b%d", List((true, 1, 0, 0, "type mismatch;\n found : Boolean\n required: Int"))) - assertEquals(foo"$s%d", List((true, 1, 0, 0, "type mismatch;\n found : String\n required: Int"))) - assertEquals(foo"$f%o", List((true, 1, 0, 0, "type mismatch;\n found : Double\n required: Int"))) - assertEquals(foo"$s%e", List((true, 1, 0, 0, "type mismatch;\n found : String\n required: Double"))) - assertEquals(foo"$b%f", List((true, 1, 0, 0, "type mismatch;\n found : Boolean\n required: Double"))) - assertEquals(foo"$s%i", List((true, 0, 1, 1, "illegal conversion character 'i'"))) - } - - def flagMismatches(s : String, c : Char, d : Int, f : Double, t : java.util.Date) = { - import TestFooErrors._ - assertEquals(foo"$s%+ 0,(s", List((true, 0, 1, 1, "Illegal flag '+'"), (true, 0, 1, 2, "Illegal flag ' '"), - (true, 0, 1, 3, "Illegal flag '0'"), (true, 0, 1, 4, "Illegal flag ','"), (true, 0, 1, 5, "Illegal flag '('"))) - assertEquals(foo"$c%#+ 0,(c", List((true, 0, 1, 1, "Only '-' allowed for c conversion"))) - assertEquals(foo"$d%#d", List((true, 0, 1, 1, "# not allowed for d conversion"))) - assertEquals(foo"$d%,x", List((true, 0, 1, 1, "',' only allowed for d conversion of integral types"))) - assertEquals(foo"$d%+ (x", List((true, 0, 1, 1, "only use '+' for BigInt conversions to o, x, X"), (true, 0, 1, 2, "only use ' ' for BigInt conversions to o, x, X"), - (true, 0, 1, 3, "only use '(' for BigInt conversions to o, x, X"))) - assertEquals(foo"$f%,(a", List((true, 0, 1, 1, "',' not allowed for a, A"), (true, 0, 1, 2, "'(' not allowed for a, A"))) - assertEquals(foo"$t%#+ 0,(tT", List((true, 0, 1, 1, "Only '-' allowed for date/time conversions"))) - assertEquals(foo"%-#+ 0,(n", List((true, 0, 0, 1, "flags not allowed"))) - assertEquals(foo"%#+ 0,(%", List((true, 0, 0, 1, "Illegal flag '#'"), (true, 0, 0, 2, "Illegal flag '+'"), - (true, 0, 0, 3, "Illegal flag ' '"), (true, 0, 0, 4, "Illegal flag '0'"), (true, 0, 0, 5, "Illegal flag ','"), (true, 0, 0, 6, "Illegal flag '('"))) - } - - def badPrecisions(c : Char, d : Int, f : Double, t : java.util.Date) = { - import TestFooErrors._ - assertEquals(foo"$c%.2c", List((true, 0, 1, 1, "precision not allowed"))) - assertEquals(foo"$d%.2d", List((true, 0, 1, 1, "precision not allowed"))) - assertEquals(foo"%.2%", List((true, 0, 0, 1, "precision not allowed"))) - assertEquals(foo"%.2n", List((true, 0, 0, 1, "precision not allowed"))) - assertEquals(foo"$f%.2a", List((true, 0, 1, 1, "precision not allowed"))) - assertEquals(foo"$t%.2tT", List((true, 0, 1, 1, "precision not allowed"))) - } - - def badIndexes() = { - import TestFooErrors._ - assertEquals(foo"% Date: Thu, 1 Oct 2020 17:09:29 +0200 Subject: [PATCH 2/4] Optimize type related logic --- .../dotty/tools/dotc/core/Definitions.scala | 8 ++++- .../localopt/StringContextChecker.scala | 34 +++++++++---------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 6da9cf131c32..fad306a7759f 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -687,7 +687,13 @@ class Definitions { @tu lazy val SerializableType: TypeRef = JavaSerializableClass.typeRef def SerializableClass(using Context): ClassSymbol = SerializableType.symbol.asClass - @tu lazy val JavaEnumClass: ClassSymbol = { + @tu lazy val JavaBigIntegerClass: ClassSymbol = requiredClass("java.math.BigInteger") + @tu lazy val JavaBigDecimalClass: ClassSymbol = requiredClass("java.math.BigDecimal") + @tu lazy val JavaCalendarClass: ClassSymbol = requiredClass("java.util.Calendar") + @tu lazy val JavaDateClass: ClassSymbol = requiredClass("java.util.Date") + @tu lazy val JavaFormattableClass: ClassSymbol = requiredClass("java.util.Formattable") + + @tu lazy val JavaEnumClass: ClassSymbol = { val cls = requiredClass("java.lang.Enum") // jl.Enum has a single constructor protected(name: String, ordinal: Int). // We remove the arguments from the primary constructor, and enter diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala index b91665986d68..40aceeee271a 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala @@ -313,8 +313,8 @@ object StringContextChecker { * @return reports a type mismatch error if the actual type is not a subtype of any of the possibilities, * nothing otherwise */ - def checkSubtype(actualType : Type, expectedType : String, argIndex : Int, possibilities : Type*) = { - if (possibilities.find(actualType <:< _).isEmpty) + def checkSubtype(actualType: Type, expectedType: String, argIndex: Int, possibilities: List[Type]) = { + if !possibilities.exists(actualType <:< _) then reporter.argError("type mismatch;\n found : " + actualType.widen.show.stripPrefix("scala.Predef.").stripPrefix("java.lang.").stripPrefix("scala.") + "\n required: " + expectedType, argIndex) } @@ -584,31 +584,31 @@ object StringContextChecker { * nothing otherwise */ def checkTypeWithArgs(argument : (Type, Int), conversionChar : Char, partIndex : Int, flags : List[(Char, Int)]) = { - val booleans = List(defn.BooleanType, defn.NullType) - val dates = List(defn.LongType, requiredClass("java.util.Calendar").typeRef, requiredClass("java.util.Date").typeRef) - val floatingPoints = List(defn.DoubleType, defn.FloatType, requiredClass("java.math.BigDecimal").typeRef) - val integral = List(defn.IntType, defn.LongType, defn.ShortType, defn.ByteType, requiredClass("java.math.BigInteger").typeRef) - val character = List(defn.CharType, defn.ByteType, defn.ShortType, defn.IntType) + def booleans = List(defn.BooleanType, defn.NullType) + def dates = List(defn.LongType, defn.JavaCalendarClass.typeRef, defn.JavaDateClass.typeRef) + def floatingPoints = List(defn.DoubleType, defn.FloatType, defn.JavaBigDecimalClass.typeRef) + def integral = List(defn.IntType, defn.LongType, defn.ShortType, defn.ByteType, defn.JavaBigIntegerClass.typeRef) + def character = List(defn.CharType, defn.ByteType, defn.ShortType, defn.IntType) val (argType, argIndex) = argument conversionChar match { - case 'c' | 'C' => checkSubtype(argType, "Char", argIndex, character : _*) + case 'c' | 'C' => checkSubtype(argType, "Char", argIndex, character) case 'd' | 'o' | 'x' | 'X' => { - checkSubtype(argType, "Int", argIndex, integral : _*) + checkSubtype(argType, "Int", argIndex, integral) if (conversionChar != 'd') { - val notAllowedFlagOnCondition = List(('+', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use '+' for BigInt conversions to o, x, X"), - (' ', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use ' ' for BigInt conversions to o, x, X"), - ('(', !(argType <:< requiredClass("java.math.BigInteger").typeRef), "only use '(' for BigInt conversions to o, x, X"), + val notAllowedFlagOnCondition = List(('+', !(argType <:< defn.JavaBigIntegerClass.typeRef), "only use '+' for BigInt conversions to o, x, X"), + (' ', !(argType <:< defn.JavaBigIntegerClass.typeRef), "only use ' ' for BigInt conversions to o, x, X"), + ('(', !(argType <:< defn.JavaBigIntegerClass.typeRef), "only use '(' for BigInt conversions to o, x, X"), (',', true, "',' only allowed for d conversion of integral types")) checkFlags(partIndex, flags, notAllowedFlagOnCondition : _*) } } - case 'e' | 'E' |'f' | 'g' | 'G' | 'a' | 'A' => checkSubtype(argType, "Double", argIndex, floatingPoints : _*) - case 't' | 'T' => checkSubtype(argType, "Date", argIndex, dates : _*) - case 'b' | 'B' => checkSubtype(argType, "Boolean", argIndex, booleans : _*) + case 'e' | 'E' |'f' | 'g' | 'G' | 'a' | 'A' => checkSubtype(argType, "Double", argIndex, floatingPoints) + case 't' | 'T' => checkSubtype(argType, "Date", argIndex, dates) + case 'b' | 'B' => checkSubtype(argType, "Boolean", argIndex, booleans) case 'h' | 'H' | 'S' | 's' => - if !(argType <:< requiredClass("java.util.Formattable").typeRef) then - for {flag <- flags ; if (flag._1 == '#')} + if !(argType <:< defn.JavaFormattableClass.typeRef) then + for flag <- flags; if flag._1 == '#' do reporter.argError("type mismatch;\n found : " + argType.widen.show.stripPrefix("scala.Predef.").stripPrefix("java.lang.").stripPrefix("scala.") + "\n required: java.util.Formattable", argIndex) case 'n' | '%' => case illegal => From 6b0f1631813bd5f2fa2bfc4dc0e58239c96f3267 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 1 Oct 2020 17:14:02 +0200 Subject: [PATCH 3/4] Use lists directly --- .../localopt/StringContextChecker.scala | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala index 40aceeee271a..ec5ba13c008c 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala @@ -361,7 +361,7 @@ object StringContextChecker { * @param notAllowedFlagsOnCondition a list that maps which flags are allowed depending on the conversion Char * @return reports an error if the flag is not allowed, nothing otherwise */ - def checkFlags(partIndex : Int, flags : List[(Char, Int)], notAllowedFlagOnCondition : (Char, Boolean, String)*) = { + def checkFlags(partIndex : Int, flags : List[(Char, Int)], notAllowedFlagOnCondition: List[(Char, Boolean, String)]) = { for {flag <- flags ; (nonAllowedFlag, condition, message) <- notAllowedFlagOnCondition ; if (flag._1 == nonAllowedFlag && condition)} reporter.partError(message, partIndex, flag._2) } @@ -373,7 +373,7 @@ object StringContextChecker { * @param notAllowedFlagsOnCondition a list that maps which flags are allowed depending on the conversion Char * @return reports an error only once if at least one of the flags is not allowed, nothing otherwise */ - def checkUniqueFlags(partIndex : Int, flags : List[(Char, Int)], notAllowedFlagOnCondition : (Char, Boolean, String)*) = { + def checkUniqueFlags(partIndex : Int, flags : List[(Char, Int)], notAllowedFlagOnCondition : List[(Char, Boolean, String)]) = { reporter.resetReported() for {flag <- flags ; (nonAllowedFlag, condition, message) <- notAllowedFlagOnCondition ; if (flag._1 == nonAllowedFlag && condition)} { if (!reporter.hasReported()) @@ -394,7 +394,7 @@ object StringContextChecker { */ def checkCharacterConversion(partIndex : Int, flags : List[(Char, Int)], hasPrecision : Boolean, precisionIndex : Int) = { val notAllowedFlagOnCondition = for (flag <- List('#', '+', ' ', '0', ',', '(')) yield (flag, true, "Only '-' allowed for c conversion") - checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition) checkNotAllowedParameter(hasPrecision, partIndex, precisionIndex, "precision") } @@ -413,7 +413,7 @@ object StringContextChecker { */ def checkIntegralConversion(partIndex : Int, argType : Option[Type], conversionChar : Char, flags : List[(Char, Int)], hasPrecision : Boolean, precision : Int) = { if (conversionChar == 'd') - checkFlags(partIndex, flags, ('#', true, "# not allowed for d conversion")) + checkFlags(partIndex, flags, List(('#', true, "# not allowed for d conversion"))) checkNotAllowedParameter(hasPrecision, partIndex, precision, "precision") } @@ -471,7 +471,7 @@ object StringContextChecker { } val notAllowedFlagOnCondition = for (flag <- List('#', '+', ' ', '0', ',', '(')) yield (flag, true, "Only '-' allowed for date/time conversions") - checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition) checkNotAllowedParameter(hasPrecision, partIndex, precision, "precision") checkTime(part, partIndex, conversionIndex) } @@ -506,12 +506,12 @@ object StringContextChecker { checkNotAllowedParameter(hasPrecision, partIndex, precision, "precision") checkNotAllowedParameter(hasWidth, partIndex, width, "width") val notAllowedFlagOnCondition = for (flag <- List('-', '#', '+', ' ', '0', ',', '(')) yield (flag, true, "flags not allowed") - checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkUniqueFlags(partIndex, flags, notAllowedFlagOnCondition) } case '%' => { checkNotAllowedParameter(hasPrecision, partIndex, precision, "precision") val notAllowedFlagOnCondition = for (flag <- List('#', '+', ' ', '0', ',', '(')) yield (flag, true, "Illegal flag '" + flag + "'") - checkFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkFlags(partIndex, flags, notAllowedFlagOnCondition) } case _ => // OK } @@ -600,7 +600,7 @@ object StringContextChecker { (' ', !(argType <:< defn.JavaBigIntegerClass.typeRef), "only use ' ' for BigInt conversions to o, x, X"), ('(', !(argType <:< defn.JavaBigIntegerClass.typeRef), "only use '(' for BigInt conversions to o, x, X"), (',', true, "',' only allowed for d conversion of integral types")) - checkFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkFlags(partIndex, flags, notAllowedFlagOnCondition) } } case 'e' | 'E' |'f' | 'g' | 'G' | 'a' | 'A' => checkSubtype(argType, "Double", argIndex, floatingPoints) @@ -631,7 +631,7 @@ object StringContextChecker { (' ', true, "only use ' ' for BigInt conversions to o, x, X"), ('(', true, "only use '(' for BigInt conversions to o, x, X"), (',', true, "',' only allowed for d conversion of integral types")) - checkFlags(partIndex, flags, notAllowedFlagOnCondition : _*) + checkFlags(partIndex, flags, notAllowedFlagOnCondition) } case _ => //OK } From 73b14ae482aa3843141a4e1e26f76cf155383660 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 1 Oct 2020 19:31:57 +0200 Subject: [PATCH 4/4] Rename Reporter to avoid name clashes --- .../dotc/transform/localopt/StringContextChecker.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala index ec5ba13c008c..fbd09f43b853 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringContextChecker.scala @@ -17,7 +17,7 @@ object StringContextChecker { import tpd._ /** This trait defines a tool to report errors/warnings that do not depend on Position. */ - trait Reporter { + trait InterpolationReporter { /** Reports error/warning of size 1 linked with a part of the StringContext. * @@ -76,7 +76,7 @@ object StringContextChecker { return "" } - val reporter = new Reporter{ + val reporter = new InterpolationReporter{ private[this] var reported = false private[this] var oldReported = false def partError(message : String, index : Int, offset : Int) : Unit = { @@ -123,7 +123,7 @@ object StringContextChecker { checked(parts, args, reporter) } - def checked(parts0: List[String], args: List[Tree], reporter: Reporter)(using Context): String = { + def checked(parts0: List[String], args: List[Tree], reporter: InterpolationReporter)(using Context): String = { /** Checks if the number of arguments are the same as the number of formatting strings