From a3d03ab8667e09e53c74d3fbb0cd7b8ae00fc8c9 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Thu, 9 May 2013 12:07:03 +0200 Subject: fixes a crash in ReflectionUtils.systemProperties Due to SI-7465 we were erroneously assuming that System.getProperties only contains string key and string values. This led to a CCE when there was something else. --- .../scala/reflect/runtime/ReflectionUtils.scala | 6 +++-- test/files/run/macro-system-properties.check | 26 ++++++++++++++++++++++ test/files/run/macro-system-properties.scala | 16 +++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 test/files/run/macro-system-properties.check create mode 100644 test/files/run/macro-system-properties.scala diff --git a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala index 7b093e0e80..2685085326 100644 --- a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala +++ b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala @@ -28,9 +28,11 @@ private[scala] object ReflectionUtils { case ex if pf isDefinedAt unwrapThrowable(ex) => pf(unwrapThrowable(ex)) } - private def systemProperties: Iterator[(String, String)] = { + private def systemProperties: Map[String, String] = { import scala.collection.JavaConverters._ - System.getProperties.asScala.iterator + // cannot use System.getProperties.asScala because of SI-7465 + val javaProperties: java.util.Dictionary[Object, Object] = System.getProperties + javaProperties.asScala.collect{ case (k: String, v: String) => (k, v) }.toMap } private def inferBootClasspath: String = ( diff --git a/test/files/run/macro-system-properties.check b/test/files/run/macro-system-properties.check new file mode 100644 index 0000000000..dce976df02 --- /dev/null +++ b/test/files/run/macro-system-properties.check @@ -0,0 +1,26 @@ +Type in expressions to have them evaluated. +Type :help for more information. + +scala> + +scala> import language.experimental._, reflect.macros.Context +import language.experimental._ +import reflect.macros.Context + +scala> object GrabContext { + def lastContext = Option(System.getProperties.get("lastContext").asInstanceOf[reflect.macros.runtime.Context]) + // System.properties lets you stash true globals (unlike statics which are classloader scoped) + def impl(c: Context)() = { System.getProperties.put("lastContext", c); c.literalUnit } + def grab() = macro impl + } +defined module GrabContext + +scala> object Test { class C(implicit a: Any) { GrabContext.grab } } +defined module Test + +scala> object Test { class C(implicit a: Any) { GrabContext.grab } } +defined module Test + +scala> + +scala> diff --git a/test/files/run/macro-system-properties.scala b/test/files/run/macro-system-properties.scala new file mode 100644 index 0000000000..e182defc81 --- /dev/null +++ b/test/files/run/macro-system-properties.scala @@ -0,0 +1,16 @@ +import scala.tools.nsc._ +import scala.tools.partest.ReplTest + +object Test extends ReplTest { + def code = """ + import language.experimental._, reflect.macros.Context + object GrabContext { + def lastContext = Option(System.getProperties.get("lastContext").asInstanceOf[reflect.macros.runtime.Context]) + // System.properties lets you stash true globals (unlike statics which are classloader scoped) + def impl(c: Context)() = { System.getProperties.put("lastContext", c); c.literalUnit } + def grab() = macro impl + } + object Test { class C(implicit a: Any) { GrabContext.grab } } + object Test { class C(implicit a: Any) { GrabContext.grab } } + """ +} -- cgit v1.2.3 From 35c0145313659b0431481617c53fb30b2c0ec109 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Thu, 9 May 2013 12:15:44 +0200 Subject: removes the traces of always on debug diagnostics pun intended --- .../scala/reflect/macros/util/Traces.scala | 2 -- .../scala/tools/nsc/typechecker/Macros.scala | 40 ++++++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/compiler/scala/reflect/macros/util/Traces.scala b/src/compiler/scala/reflect/macros/util/Traces.scala index d16916b753..2dffc68745 100644 --- a/src/compiler/scala/reflect/macros/util/Traces.scala +++ b/src/compiler/scala/reflect/macros/util/Traces.scala @@ -6,8 +6,6 @@ trait Traces { val macroDebugLite = globalSettings.YmacrodebugLite.value val macroDebugVerbose = globalSettings.YmacrodebugVerbose.value - val macroTraceLite = scala.tools.nsc.util.trace when (macroDebugLite || macroDebugVerbose) - val macroTraceVerbose = scala.tools.nsc.util.trace when macroDebugVerbose @inline final def macroLogLite(msg: => Any) { if (macroDebugLite || macroDebugVerbose) println(msg) } @inline final def macroLogVerbose(msg: => Any) { if (macroDebugVerbose) println(msg) } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Macros.scala b/src/compiler/scala/tools/nsc/typechecker/Macros.scala index 0ba30ffa73..9e8c136ab5 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Macros.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Macros.scala @@ -341,11 +341,12 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { } import SigGenerator._ - macroTraceVerbose("generating macroImplSigs for: ")(macroDef) - macroTraceVerbose("tparams are: ")(tparams) - macroTraceVerbose("vparamss are: ")(vparamss) - macroTraceVerbose("retTpe is: ")(retTpe) - macroTraceVerbose("macroImplSig is: ")((paramss, implRetTpe)) + macroLogVerbose(s"generating macroImplSigs for: $macroDef") + macroLogVerbose(s"tparams are: $tparams") + macroLogVerbose(s"vparamss are: $vparamss") + macroLogVerbose(s"retTpe is: $retTpe") + macroLogVerbose(s"macroImplSig is: $paramss, $implRetTpe") + (paramss, implRetTpe) } /** Verifies that the body of a macro def typechecks to a reference to a static public non-overloaded method, @@ -505,7 +506,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { type MacroRuntime = MacroArgs => Any private val macroRuntimesCache = perRunCaches.newWeakMap[Symbol, MacroRuntime] private def macroRuntime(macroDef: Symbol): MacroRuntime = { - macroTraceVerbose("looking for macro implementation: ")(macroDef) + macroLogVerbose(s"looking for macro implementation: $macroDef") if (fastTrack contains macroDef) { macroLogVerbose("macro expansion is serviced by a fast track") fastTrack(macroDef) @@ -522,18 +523,18 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { // upd. my latest experiments show that everything's okay // it seems that in 2.10.1 we can easily switch to Scala reflection try { - macroTraceVerbose("loading implementation class: ")(className) - macroTraceVerbose("classloader is: ")(ReflectionUtils.show(macroClassloader)) + macroLogVerbose(s"loading implementation class: $className") + macroLogVerbose(s"classloader is: ${ReflectionUtils.show(macroClassloader)}") val implObj = ReflectionUtils.staticSingletonInstance(macroClassloader, className) // relies on the fact that macro impls cannot be overloaded // so every methName can resolve to at maximum one method val implMeths = implObj.getClass.getDeclaredMethods.find(_.getName == methName) val implMeth = implMeths getOrElse { throw new NoSuchMethodException(s"$className.$methName") } - macroLogVerbose("successfully loaded macro impl as (%s, %s)".format(implObj, implMeth)) + macroLogVerbose(s"successfully loaded macro impl as ($implObj, $implMeth)") args => implMeth.invoke(implObj, ((args.c +: args.others) map (_.asInstanceOf[AnyRef])): _*) } catch { case ex: Exception => - macroTraceVerbose(s"macro runtime failed to load: ")(ex.toString) + macroLogVerbose(s"macro runtime failed to load: ${ex.toString}") macroDef setFlag IS_ERROR null } @@ -577,8 +578,8 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { if (argcDoesntMatch && !nullaryArgsEmptyParams) { typer.TyperErrorGen.MacroPartialApplicationError(expandee) } val argss: List[List[Any]] = exprArgs.toList - macroTraceVerbose("context: ")(context) - macroTraceVerbose("argss: ")(argss) + macroLogVerbose(s"context: $context") + macroLogVerbose(s"argss: $argss") val preparedArgss: List[List[Any]] = if (fastTrack contains macroDef) { @@ -602,7 +603,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { // whereas V won't be resolved by asSeenFrom and need to be loaded directly from `expandee` which needs to contain a TypeApply node // also, macro implementation reference may contain a regular type as a type argument, then we pass it verbatim val binding = loadMacroImplBinding(macroDef) - macroTraceVerbose("binding: ")(binding) + macroLogVerbose(s"binding: $binding") val tags = binding.signature filter (_ != -1) map (paramPos => { val targ = binding.targs(paramPos).tpe.typeSymbol val tpe = if (targ.isTypeParameterOrSkolem) { @@ -620,7 +621,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { targ.tpe context.WeakTypeTag(tpe) }) - macroTraceVerbose("tags: ")(tags) + macroLogVerbose(s"tags: $tags") // transforms argss taking into account varargness of paramss // note that typetag context bounds are only declared on macroImpls @@ -637,7 +638,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { } else as }) } - macroTraceVerbose("preparedArgss: ")(preparedArgss) + macroLogVerbose(s"preparedArgss: $preparedArgss") MacroArgs(context, preparedArgss.flatten) } @@ -695,7 +696,8 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { val numErrors = reporter.ERROR.count def hasNewErrors = reporter.ERROR.count > numErrors val result = typer.context.withImplicitsEnabled(typer.typed(tree, EXPRmode, pt)) - macroTraceVerbose(s"""${if (hasNewErrors) "failed to typecheck" else "successfully typechecked"} against $phase $pt:\n$result\n""")(result) + macroLogVerbose(s"""${if (hasNewErrors) "failed to typecheck" else "successfully typechecked"} against $phase $pt:\n$result""") + result } var expectedTpe = expandee.tpe @@ -724,7 +726,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { withInfoLevel(nodePrinters.InfoLevel.Quiet) { if (expandee.symbol.isErroneous || (expandee exists (_.isErroneous))) { val reason = if (expandee.symbol.isErroneous) "not found or incompatible macro implementation" else "erroneous arguments" - macroTraceVerbose("cancelled macro expansion because of %s: ".format(reason))(expandee) + macroLogVerbose(s"cancelled macro expansion because of $reason: $expandee") return Cancel(typer.infer.setError(expandee)) } @@ -794,7 +796,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { private def macroExpandWithoutRuntime(typer: Typer, expandee: Tree): MacroExpansionResult = { import typer.TyperErrorGen._ val fallbackSym = expandee.symbol.nextOverriddenSymbol orElse MacroImplementationNotFoundError(expandee) - macroTraceLite("falling back to: ")(fallbackSym) + macroLogLite(s"falling back to: $fallbackSym") def mkFallbackTree(tree: Tree): Tree = { tree match { @@ -846,7 +848,7 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { undetparams --= undetNoMore map (_.id) if (undetparams.isEmpty) { hasPendingMacroExpansions = true - macroTraceVerbose("macro expansion is pending: ")(expandee) + macroLogVerbose(s"macro expansion is pending: $expandee") } case _ => // do nothing -- cgit v1.2.3 From 75a3b88c697d485a321429a7526c823f1b4bfc8e Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Thu, 9 May 2013 14:04:14 +0200 Subject: replaces inferBootClasspath with a simple lookup at sun.boot.class.path It's not like we're achieving any generality by iterating through all keys in System.getProperties and looking for ones which resemble "boot.class.path", so let's be simpler. --- src/reflect/scala/reflect/runtime/ReflectionUtils.scala | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala index 2685085326..3ee53eb7a1 100644 --- a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala +++ b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala @@ -28,17 +28,6 @@ private[scala] object ReflectionUtils { case ex if pf isDefinedAt unwrapThrowable(ex) => pf(unwrapThrowable(ex)) } - private def systemProperties: Map[String, String] = { - import scala.collection.JavaConverters._ - // cannot use System.getProperties.asScala because of SI-7465 - val javaProperties: java.util.Dictionary[Object, Object] = System.getProperties - javaProperties.asScala.collect{ case (k: String, v: String) => (k, v) }.toMap - } - - private def inferBootClasspath: String = ( - systemProperties find (_._1 endsWith ".boot.class.path") map (_._2) getOrElse "" - ) - def show(cl: ClassLoader): String = { import scala.language.reflectiveCalls @@ -53,7 +42,7 @@ private[scala] object ReflectionUtils { case cl if cl != null && isAbstractFileClassLoader(cl.getClass) => cl.asInstanceOf[{val root: scala.reflect.io.AbstractFile}].root.canonicalPath case null => - inferBootClasspath + scala.util.Properties.propOrEmpty("sun.boot.class.path") case _ => "" } -- cgit v1.2.3 From 5751ddd9eee0573dcf889983b423397fb6670415 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 10 May 2013 21:51:29 +0200 Subject: pull request feedback --- src/compiler/scala/tools/nsc/typechecker/Macros.scala | 12 +++++++----- src/reflect/scala/reflect/runtime/ReflectionUtils.scala | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Macros.scala b/src/compiler/scala/tools/nsc/typechecker/Macros.scala index 9e8c136ab5..5955081345 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Macros.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Macros.scala @@ -341,11 +341,13 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { } import SigGenerator._ - macroLogVerbose(s"generating macroImplSigs for: $macroDef") - macroLogVerbose(s"tparams are: $tparams") - macroLogVerbose(s"vparamss are: $vparamss") - macroLogVerbose(s"retTpe is: $retTpe") - macroLogVerbose(s"macroImplSig is: $paramss, $implRetTpe") + macroLogVerbose(sm""" + |generating macroImplSigs for: $macroDef + |tparams are: $tparams + |vparamss are: $vparamss + |retTpe is: $retTpe + |macroImplSig is: $paramss, $implRetTpe + """.trim) (paramss, implRetTpe) } diff --git a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala index 3ee53eb7a1..33ad6d2430 100644 --- a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala +++ b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala @@ -42,7 +42,8 @@ private[scala] object ReflectionUtils { case cl if cl != null && isAbstractFileClassLoader(cl.getClass) => cl.asInstanceOf[{val root: scala.reflect.io.AbstractFile}].root.canonicalPath case null => - scala.util.Properties.propOrEmpty("sun.boot.class.path") + val loadBootCp = (flavor: String) => scala.util.Properties.propOrNone(flavor + ".boot.class.path") + loadBootCp("sun") orElse loadBootCp("java") getOrElse "" case _ => "" } -- cgit v1.2.3