Skip to content
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

Redefine semantics of inline parameters #8060

Merged
merged 2 commits into from
Jan 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TypeErasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
// but potentially re-introduced by ResolveSuper, when we add
// forwarders to mixin methods.
// See doc comment for ElimByName for speculation how we could improve this.
else MethodType(Nil, Nil, eraseResult(sym.info.finalResultType))
else MethodType(Nil, Nil, eraseResult(sym.info.finalResultType.underlyingIfRepeated(isJava)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to check if we can move it earlier to simply the contract between phases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would also include val binding generated for repeated parameters.

case tp: PolyType =>
eraseResult(tp.resultType) match {
case rt: MethodType => rt
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3360,11 +3360,11 @@ object Types {
/** Produce method type from parameter symbols, with special mappings for repeated
* and inline parameters:
* - replace @repeated annotations on Seq or Array types by <repeated> types
* - add @inlineParam to inline call-by-value parameters
* - add @inlineParam to inline parameters
*/
def fromSymbols(params: List[Symbol], resultType: Type)(implicit ctx: Context): MethodType = {
def translateInline(tp: Type): Type = tp match {
case _: ExprType => tp
case ExprType(resType) => ExprType(AnnotatedType(resType, Annotation(defn.InlineParamAnnot)))
case _ => AnnotatedType(tp, Annotation(defn.InlineParamAnnot))
}
def paramInfo(param: Symbol) = {
Expand Down
29 changes: 29 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/trace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,18 @@ object trace extends TraceSyntax {
abstract class TraceSyntax {
val isForced: Boolean

// FIXME Use this signature after reference compiler is updated
// inline def onDebug[TD](inline question: String)(inline op: TD)(implicit ctx: Context): TD =
inline def onDebug[TD](question: => String)(op: => TD)(implicit ctx: Context): TD =
conditionally(ctx.settings.YdebugTrace.value, question, false)(op)

// FIXME Use this implementation after reference compiler is updated
// inline def conditionally[TC](inline cond: Boolean, inline question: String, inline show: Boolean)(op: => TC)(implicit ctx: Context): TC =
// inline if (isForced || Config.tracingEnabled) {
// if (cond) apply[TC](question, Printers.default, show)(op)
// else op
// }
// else op
inline def conditionally[TC](cond: Boolean, question: => String, show: Boolean)(op: => TC)(implicit ctx: Context): TC =
inline if (isForced || Config.tracingEnabled) {
def op1 = op
Expand All @@ -36,6 +45,13 @@ abstract class TraceSyntax {
}
else op

// FIXME Use this implementation after reference compiler is updated
// inline def apply[T](inline question: String, inline printer: Printers.Printer, inline showOp: Any => String)(op: => T)(implicit ctx: Context): T =
// inline if (isForced || Config.tracingEnabled) {
// if (!isForced && printer.eq(config.Printers.noPrinter)) op
// else doTrace[T](question, printer, showOp)(op)
// }
// else op
inline def apply[T](question: => String, printer: Printers.Printer, showOp: Any => String)(op: => T)(implicit ctx: Context): T =
inline if (isForced || Config.tracingEnabled) {
def op1 = op
Expand All @@ -44,6 +60,13 @@ abstract class TraceSyntax {
}
else op

// FIXME Use this implementation after reference compiler is updated
// inline def apply[T](inline question: String, inline printer: Printers.Printer, inline show: Boolean)(op: => T)(implicit ctx: Context): T =
// inline if (isForced || Config.tracingEnabled) {
// if (!isForced && printer.eq(config.Printers.noPrinter)) op
// else doTrace[T](question, printer, if (show) showShowable(_) else alwaysToString)(op)
// }
// else op
inline def apply[T](question: => String, printer: Printers.Printer, show: Boolean)(op: => T)(implicit ctx: Context): T =
inline if (isForced || Config.tracingEnabled) {
def op1 = op
Expand All @@ -52,12 +75,18 @@ abstract class TraceSyntax {
}
else op

// FIXME Use this signature after reference compiler is updated
// inline def apply[T](inline question: String, inline printer: Printers.Printer)(inline op: T)(implicit ctx: Context): T =
inline def apply[T](question: => String, printer: Printers.Printer)(op: => T)(implicit ctx: Context): T =
apply[T](question, printer, false)(op)

// FIXME Use this signature after reference compiler is updated
// inline def apply[T](inline question: String, inline show: Boolean)(inline op: T)(implicit ctx: Context): T =
inline def apply[T](question: => String, show: Boolean)(op: => T)(implicit ctx: Context): T =
apply[T](question, Printers.default, show)(op)

// FIXME Use this signature after reference compiler is updated
// inline def apply[T](inline question: String)(inline op: T)(implicit ctx: Context): T =
inline def apply[T](question: => String)(op: => T)(implicit ctx: Context): T =
apply[T](question, Printers.default, false)(op)

Expand Down
39 changes: 2 additions & 37 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -818,43 +818,8 @@ trait Checking {
tree.tpe.widenTermRefExpr match {
case tp: ConstantType if exprPurity(tree) >= purityLevel => // ok
case _ =>
tree match {
case Typed(expr, _) =>
checkInlineConformant(expr, isFinal, what)
case Inlined(_, Nil, expr) =>
checkInlineConformant(expr, isFinal, what)
case SeqLiteral(elems, _) =>
elems.foreach(elem => checkInlineConformant(elem, isFinal, what))
case Apply(fn, List(arg)) if defn.WrapArrayMethods().contains(fn.symbol) =>
checkInlineConformant(arg, isFinal, what)
case _ =>
def isCaseClassApply(sym: Symbol): Boolean =
sym.name == nme.apply && sym.is(Synthetic) && sym.owner.is(Module) && sym.owner.companionClass.is(Case)
def isCaseClassNew(sym: Symbol): Boolean =
sym.isPrimaryConstructor && sym.owner.is(Case) && sym.owner.isStatic
def isCaseObject(sym: Symbol): Boolean =
// TODO add alias to Nil in scala package
sym.is(Case) && sym.is(Module)
def isStaticEnumCase(sym: Symbol): Boolean =
sym.is(Enum) && sym.is(JavaStatic) && sym.is(Case)
val allow =
ctx.erasedTypes ||
ctx.inInlineMethod ||
(tree.symbol.isStatic && isCaseObject(tree.symbol) || isCaseClassApply(tree.symbol)) ||
isStaticEnumCase(tree.symbol) ||
isCaseClassNew(tree.symbol)

if (!allow) ctx.error(em"$what must be a known value", tree.sourcePos)
else {
def checkArgs(tree: Tree): Unit = tree match {
case Apply(fn, args) =>
args.foreach(arg => checkInlineConformant(arg, isFinal, what))
checkArgs(fn)
case _ =>
}
checkArgs(tree)
}
}
if (!ctx.erasedTypes && !ctx.inInlineMethod)
ctx.error(em"$what must be a known value", tree.sourcePos)
}
}

Expand Down
20 changes: 10 additions & 10 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ object Inliner {
}

val Apply(_, codeArg :: Nil) = tree
ConstFold(stripTyped(codeArg.underlyingArgument)).tpe.widenTermRefExpr match {
val underlyingCodeArg = stripTyped(codeArg.underlying)
ConstFold(underlyingCodeArg).tpe.widenTermRefExpr match {
case ConstantType(Constant(code: String)) =>
val source2 = SourceFile.virtual("tasty-reflect", code)
val ctx2 = ctx.fresh.setNewTyperState().setTyper(new Typer).setSource(source2)
Expand All @@ -223,7 +224,7 @@ object Inliner {
res ++= typerErrors.map(e => ErrorKind.Typer -> e)
res.toList
case t =>
assert(ctx.reporter.hasErrors) // at least: argument to inline parameter must be a known value
ctx.error("argument to compileError must be a statically known String", underlyingCodeArg.sourcePos)
Nil
}

Expand Down Expand Up @@ -333,9 +334,10 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
val argtpe = arg.tpe.dealiasKeepAnnots
val isByName = paramtp.dealias.isInstanceOf[ExprType]
var inlineFlags: FlagSet = InlineProxy
if (paramtp.hasAnnotation(defn.InlineParamAnnot)) inlineFlags |= Inline
if (paramtp.widenExpr.hasAnnotation(defn.InlineParamAnnot)) inlineFlags |= Inline
if (isByName) inlineFlags |= Method
val (bindingFlags, bindingType) =
if (isByName) (InlineByNameProxy.toTermFlags, ExprType(argtpe.widen))
if (isByName) (inlineFlags, ExprType(argtpe.widen))
else (inlineFlags, argtpe.widen)
val boundSym = newSym(name, bindingFlags, bindingType).asTerm
val binding = {
Expand Down Expand Up @@ -765,10 +767,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
def search(buf: mutable.ListBuffer[ValOrDefDef]) = buf.find(_.name == tree.name)
if (paramProxies.contains(tree.typeOpt))
search(bindingsBuf) match {
case Some(vdef: ValDef) if vdef.symbol.is(Inline) =>
Some(integrate(vdef.rhs, vdef.symbol))
case Some(ddef: DefDef) =>
Some(integrate(ddef.rhs, ddef.symbol))
case Some(bind: ValOrDefDef) if bind.symbol.is(Inline) =>
Some(integrate(bind.rhs, bind.symbol))
case _ => None
}
else None
Expand Down Expand Up @@ -1198,7 +1198,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
val bindingOfSym = newMutableSymbolMap[MemberDef]

def isInlineable(binding: MemberDef) = binding match {
case DefDef(_, Nil, Nil, _, _) => true
case ddef @ DefDef(_, Nil, Nil, _, _) => isPureExpr(ddef.rhs)
case vdef @ ValDef(_, _, _) => isPureExpr(vdef.rhs)
case _ => false
}
Expand Down Expand Up @@ -1236,7 +1236,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
case Some(x) => x > 1 || x == 1 && !boundSym.is(Method)
case none => true
}
} && !(boundSym.isAllOf(InlineMethod) && boundSym.isOneOf(GivenOrImplicit))
} && !boundSym.is(Inline)

val inlineBindings = new TreeMap {
override def transform(t: Tree)(implicit ctx: Context) = t match {
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2910,8 +2910,6 @@ class Typer extends Namer
tree
}
else if (tree.tpe.widenExpr <:< pt) {
if (pt.hasAnnotation(defn.InlineParamAnnot))
checkInlineConformant(tree, isFinal = false, "argument to inline parameter")
if (ctx.typeComparer.GADTused && pt.isValueType)
// Insert an explicit cast, so that -Ycheck in later phases succeeds.
// I suspect, but am not 100% sure that this might affect inferred types,
Expand Down
9 changes: 6 additions & 3 deletions compiler/src/dotty/tools/dotc/util/Stats.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import collection.mutable
override def default(key: String): Int = 0
}

// FIXME Use this signature after reference compiler is updated
// inline def record(inline fn: String, inline n: Int = 1): Unit =
inline def record(fn: => String, n: => Int = 1): Unit =
if (enabled) doRecord(fn, n)

Expand All @@ -28,16 +30,17 @@ import collection.mutable
hits(name) += n
}

// FIXME Use this signature after reference compiler is updated
// inline def trackTime[T](fn: String)(inline op: T): T =
inline def trackTime[T](fn: String)(op: => T): T =
if (enabled) doTrackTime(fn)(op) else op

def doTrackTime[T](fn: String)(op: => T): T = {
def op1 = op
if (monitored) {
val start = System.nanoTime
try op1 finally record(fn, ((System.nanoTime - start) / 1000).toInt)
try op finally record(fn, ((System.nanoTime - start) / 1000).toInt)
}
else op1
else op
}

final val GroupChar = '/'
Expand Down
Loading