diff --git a/fastparse/src/fastparse/internal/MacroImpls.scala b/fastparse/src/fastparse/internal/MacroImpls.scala index b09debef..25a98a6a 100644 --- a/fastparse/src/fastparse/internal/MacroImpls.scala +++ b/fastparse/src/fastparse/internal/MacroImpls.scala @@ -15,11 +15,41 @@ import scala.reflect.macros.blackbox.Context * String/Char values are known at compile time. */ object MacroImpls { + + /** Checks a tree for subexpressions that contain references to parsers passed as parameters, + * and makes sure these parameters are by-name. + * We exclude from the check: + * - implicit parameters, because they are used pervasively to pass context around, as in: `def foo[_: P] = ...` + * - parsers whose type argument is an existential/wildcard, because they can be used safely (their value is not used) + */ + private[fastparse] def checkByNames[T](c: Context)(p: c.Expr[T]): Unit = { + import c.universe._ + val ParsingRunSym = typeOf[ParsingRun[_]].typeSymbol + + // Note: it is not enough to just check the top-level tree, as sometimes it may be wrapped in EagerOps or others. + + new Traverser{ + override def traverse(tree: Tree): Unit = { + Option(tree.tpe).map(_.baseType(ParsingRunSym)).collect{ + case TypeRef(_, ParsingRunSym, ty :: Nil) => // type ParsingRun[T] has one type parameter + if (ty.typeSymbol.isType && !ty.typeSymbol.asType.isExistential) { + val sym = tree.symbol + if (sym != null && sym.isParameter && sym.isTerm && !sym.asTerm.isByNameParam && !sym.asTerm.isImplicit) + c.error(tree.pos, s"Parameters passed to parsers should be by-name: $tree") + } + } + super.traverse(tree) + } + }.traverse(p.tree) + + } + def filterMacro[T: c.WeakTypeTag](c: Context) (f: c.Expr[T => Boolean]) (ctx: c.Expr[ParsingRun[_]]) = { import c.universe._ val lhs = c.prefix.asInstanceOf[c.Expr[EagerOps[T]]] + checkByNames(c)(lhs) reify{ ctx.splice match{ case ctx1 => lhs.splice @@ -141,6 +171,7 @@ object MacroImpls { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[T]]] + checkByNames(c)(lhs0) reify { lhs0.splice.parse0 match{ case lhs => if (!lhs.isSuccess) lhs.asInstanceOf[ParsingRun[V]] @@ -160,6 +191,7 @@ object MacroImpls { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[T]]] + checkByNames(c)(lhs0) reify { val lhs = lhs0.splice.parse0 if (!lhs.isSuccess) lhs.asInstanceOf[ParsingRun[V]] @@ -174,6 +206,7 @@ object MacroImpls { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[T]]] + checkByNames(c)(lhs0) reify { val lhs = lhs0.splice.parse0 whitespace.splice match{ case ws => @@ -198,6 +231,8 @@ object MacroImpls { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[T]]] + checkByNames(c)(lhs0) + checkByNames(c)(other) reify { ctx.splice match { case ctx5 => val oldCut = ctx5.cut @@ -244,6 +279,7 @@ object MacroImpls { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[_]]] + checkByNames(c)(lhs0) reify { ctx.splice match{ case ctx6 => @@ -427,6 +463,9 @@ object MacroImpls { val lhs = c.prefix.asInstanceOf[Expr[EagerOps[T]]] val cut1 = c.Expr[Boolean](if(cut) q"true" else q"ctx1.cut") + checkByNames(c)(lhs) + checkByNames(c)(other) + val rhs = q""" if (!ctx1.isSuccess && ctx1.cut) ctx1 else { @@ -493,6 +532,7 @@ object MacroImpls { def cutMacro[T: c.WeakTypeTag](c: Context)(ctx: c.Expr[ParsingRun[_]]): c.Expr[ParsingRun[T]] = { import c.universe._ val lhs = c.prefix.asInstanceOf[c.Expr[EagerOps[_]]] + checkByNames(c)(lhs) reify{ ctx.splice match { case ctx0 => val startIndex = ctx0.index @@ -584,6 +624,7 @@ object MacroImpls { ctx: c.Expr[ParsingRun[Any]]): c.Expr[ParsingRun[V]] = { import c.universe._ val lhs0 = c.prefix.asInstanceOf[c.Expr[EagerOps[_]]] + checkByNames(c)(lhs0) reify{ ctx.splice match{ case ctx1 => optioner.splice match { case optioner1 => diff --git a/fastparse/src/fastparse/internal/RepImpls.scala b/fastparse/src/fastparse/internal/RepImpls.scala index ea6088eb..81c76e64 100644 --- a/fastparse/src/fastparse/internal/RepImpls.scala +++ b/fastparse/src/fastparse/internal/RepImpls.scala @@ -14,15 +14,20 @@ import language.experimental.macros * due to https://github.com/scala/bug/issues/5920). * * Even the normal method overloads are manually-specialized to some extent - * for various sorts of inputs as a best-effort attempt ot minimize branching + * for various sorts of inputs as a best-effort attempt to minimize branching * in the hot paths. */ object MacroRepImpls{ + import MacroImpls.checkByNames + def repXMacro0[T: c.WeakTypeTag, V: c.WeakTypeTag](c: Context) (whitespace: Option[c.Tree], min: Option[c.Tree]) (repeater: c.Tree, ctx: c.Tree): c.Tree = { import c.universe._ + + checkByNames(c)(c.prefix) + val repeater1 = TermName(c.freshName("repeater")) val ctx1 = TermName(c.freshName("repeater")) val acc = TermName(c.freshName("acc")) diff --git a/fastparse/test/src/fastparse/ParsingTests.scala b/fastparse/test/src/fastparse/ParsingTests.scala index e1d813f2..fc999028 100644 --- a/fastparse/test/src/fastparse/ParsingTests.scala +++ b/fastparse/test/src/fastparse/ParsingTests.scala @@ -219,6 +219,31 @@ object ParsingTests extends TestSuite{ checkWhitespaceFlatMap() checkNonWhitespaceFlatMap() } + 'parameterized{ + def digit[_: P] = CharIn("0-9") + def letter[_: P] = CharIn("A-Z") + + assert { + val msg = compileError("def twice[T, _: P](p: P[T]): P[(T,T)] = p ~ p").msg + msg == "Parameters passed to parsers should be by-name: p" + } + def twice[T, _: P](p: => P[T]): P[(T,T)] = p ~ p + + def p[_: P] = P( twice(digit) ~ twice(letter) ) + val Parsed.Success(_, _) = parse("12AB", p(_)) // Note: this would fail if 'p' was not passed by name + + assert { + val msg = compileError("def concats[T, _: P](l: P[T], r: => P[T]): P[(T,(T,T))] = l ~ (l ~ r)").msg + msg == "Parameters passed to parsers should be by-name: l" + } + def concats[T, _: P](l: => P[T], r: => P[T]): P[(T,(T,T))] = l ~ (l ~ r) + + def p2[_: P] = P( concats(digit,letter) ~ concats(letter,digit) ) + val Parsed.Success(_, _) = parse("12ABC3", p2(_)) + + def foo[T, _: P](p: P[_]): P[_] = p ~ digit // compiles because we used a wildcard in P[_] + + } } def checkWhitespaceFlatMap() = {