Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Eliminated all forInteractive/forScaladoc uses.

This is the commit which brings it all together. The booleans
forInteractive and forScaladoc are now deprecated and are not
inspected for any purpose. All behavioral changes formerly
accomplished via tests of those flags are embodied in the globals
built specifically for those tasks.
  • Loading branch information...
commit 2352814d4be064d67794899cf5494d3324a131ec 1 parent e01c7ef
Paul Phillips paulp authored adriaanm committed
2  src/compiler/scala/tools/nsc/Global.scala
View
@@ -1696,8 +1696,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
}
})
}
- def forInteractive = false
- def forScaladoc = false
def createJavadoc = false
}
18 src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
View
@@ -683,7 +683,7 @@ trait ContextErrors {
// same reason as for MacroBodyTypecheckException
case object MacroExpansionException extends Exception with scala.util.control.ControlThrowable
- private def macroExpansionError(expandee: Tree, msg: String, pos: Position = NoPosition) = {
+ protected def macroExpansionError(expandee: Tree, msg: String, pos: Position = NoPosition) = {
def msgForLog = if (msg != null && (msg contains "exception during macro expansion")) msg.split(EOL).drop(1).headOption.getOrElse("?") else msg
macroLogLite("macro expansion has failed: %s".format(msgForLog))
if (msg != null) context.error(pos, msg) // issueTypeError(PosAndMsgTypeError(..)) won't work => swallows positions
@@ -772,15 +772,15 @@ trait ContextErrors {
))
}
- def MacroImplementationNotFoundError(expandee: Tree) = {
- val message =
- "macro implementation not found: " + expandee.symbol.name + " " +
- "(the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)" +
- (if (forScaladoc) ". When generating scaladocs for multiple projects at once, consider using -Ymacro-no-expand to disable macro expansions altogether."
- else "")
- macroExpansionError(expandee, message)
- }
+ def MacroImplementationNotFoundError(expandee: Tree) =
+ macroExpansionError(expandee, macroImplementationNotFoundMessage(expandee.symbol.name))
}
+
+ /** This file will be the death of me. */
Eugene Burmako Owner
xeno-by added a note

@paulp What does this mean?

Paul Phillips
paulp added a note

It means the error code is indescribably nightmarish, and holds us back in a huge way.

Eugene Burmako Owner
xeno-by added a note

Since this comment annotates a routine responsible for macro error handling, maybe you could elaborate on the problems in how the macro engine handles errors?

Paul Phillips
paulp added a note

The story must be somewhat visible in the diff. It was a lengthy process in which I had zero interest to get the one piece of the error code talking to the other piece. See the access shuffling and parameter sprouting. The error code is spread out over every form of mutation known to man, at least 5x the quantity of code which should be necessary, and it's so difficult to modify that I have completely given up - which is not exactly good news for a language like scala, which really needs clear errors. There is no rhyme nor reason to the organization. There is no way of knowing how any given error is supposed to be reported.

The code is an albatross around my neck. I would give anything to get rid of it. It is like a nightmare from which I cannot wake up.

It has nothing to do with the macro engine per se. If you don't like the comment being too close to a bit of macro code, you can remove it. My LGTM is yours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ protected def macroImplementationNotFoundMessage(name: Name): String = (
+ s"""|macro implementation not found: $name
+ |(the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)""".stripMargin
+ )
Jason Zaugg Owner

sm"""|concise and correct"""

Jason Zaugg Owner

Oh, this is old...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}
trait InferencerContextErrors {
45 src/compiler/scala/tools/nsc/typechecker/Namers.scala
View
@@ -50,19 +50,8 @@ trait Namers extends MethodSynthesis {
def newNamerFor(context: Context, tree: Tree): Namer = newNamer(context.makeNewScope(tree, tree.symbol))
abstract class Namer(val context: Context) extends MethodSynth with NamerContextErrors { thisNamer =>
-
- def saveDefaultGetter(meth: Symbol, default: Symbol) {
- if (forInteractive) {
- // save the default getters as attachments in the method symbol. if compiling the
- // same local block several times (which can happen in interactive mode) we might
- // otherwise not find the default symbol, because the second time it the method
- // symbol will be re-entered in the scope but the default parameter will not.
- meth.attachments.get[DefaultsOfLocalMethodAttachment] match {
- case Some(att) => att.defaultGetters += default
- case None => meth.updateAttachment(new DefaultsOfLocalMethodAttachment(default))
- }
- }
- }
+ // overridden by the presentation compiler
+ def saveDefaultGetter(meth: Symbol, default: Symbol) { }
import NamerErrorGen._
val typer = newTyper(context)
@@ -606,17 +595,6 @@ trait Namers extends MethodSynthesis {
}
}
- def enterIfNotThere(sym: Symbol) {
- val scope = context.scope
- @tailrec def search(e: ScopeEntry) {
- if ((e eq null) || (e.owner ne scope))
- scope enter sym
- else if (e.sym ne sym) // otherwise, aborts since we found sym
- search(e.tail)
- }
- search(scope lookupEntry sym.name)
- }
-
def enterValDef(tree: ValDef) {
if (noEnterGetterSetter(tree))
assignAndEnterFinishedSymbol(tree)
@@ -709,22 +687,9 @@ trait Namers extends MethodSynthesis {
validateCompanionDefs(tree)
}
- // this logic is needed in case typer was interrupted half
- // way through and then comes back to do the tree again. In
- // that case the definitions that were already attributed as
- // well as any default parameters of such methods need to be
- // re-entered in the current scope.
- protected def enterExistingSym(sym: Symbol): Context = {
- if (forInteractive && sym != null && sym.owner.isTerm) {
- enterIfNotThere(sym)
- if (sym.isLazy)
- sym.lazyAccessor andAlso enterIfNotThere
-
- for (defAtt <- sym.attachments.get[DefaultsOfLocalMethodAttachment])
- defAtt.defaultGetters foreach enterIfNotThere
- }
- this.context
- }
+ // Hooks which are overridden in the presentation compiler
+ def enterExistingSym(sym: Symbol): Context = this.context
+ def enterIfNotThere(sym: Symbol) { }
def enterSyntheticSym(tree: Tree): Symbol = {
enterSym(tree)
24 src/compiler/scala/tools/nsc/typechecker/Typers.scala
View
@@ -96,16 +96,16 @@ trait Typers extends Adaptations with Tags {
// - we may virtualize matches (if -Xexperimental and there's a suitable __match in scope)
// - we synthesize PartialFunction implementations for `x => x match {...}` and `match {...}` when the expected type is PartialFunction
// this is disabled by: interactive compilation (we run it for scaladoc due to SI-5933)
- protected def newPatternMatching = !forInteractive //&& !forScaladoc && (phase.id < currentRun.uncurryPhase.id)
+ protected def newPatternMatching = true // presently overridden in the presentation compiler
abstract class Typer(context0: Context) extends TyperDiagnostics with Adaptation with Tag with TyperContextErrors {
import context0.unit
import typeDebug.{ ptTree, ptBlock, ptLine }
import TyperErrorGen._
- /** (Will be) overridden to false in scaladoc and/or interactive. */
- def canAdaptConstantTypeToLiteral = !forScaladoc && !forInteractive
- def canTranslateEmptyListToNil = !forInteractive
+ /** Overridden to false in scaladoc and/or interactive. */
+ def canAdaptConstantTypeToLiteral = true
+ def canTranslateEmptyListToNil = true
def missingSelectErrorTree(tree: Tree, qual: Tree, name: Name): Tree = tree
def typedDocDef(docDef: DocDef, mode: Mode, pt: Type): Tree =
@@ -1041,7 +1041,7 @@ trait Typers extends Adaptations with Tags {
tree.tpe match {
case atp @ AnnotatedType(_, _, _) if canAdaptAnnotations(tree, this, mode, pt) => // (-1)
adaptAnnotations(tree, this, mode, pt)
- case ct @ ConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && !forScaladoc && !forInteractive => // (0)
+ case ct @ ConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && canAdaptConstantTypeToLiteral => // (0)
val sym = tree.symbol
if (sym != null && sym.isDeprecated) {
val msg = sym.toString + sym.locationString + " is deprecated: " + sym.deprecationMessage.getOrElse("")
@@ -2436,11 +2436,9 @@ trait Typers extends Adaptations with Tags {
if (pat1.tpe.paramSectionCount > 0)
pat1 setType pat1.tpe.finalResultType
- if (forInteractive) {
- for (bind @ Bind(name, _) <- cdef.pat)
- if (name.toTermName != nme.WILDCARD && bind.symbol != null && bind.symbol != NoSymbol)
- namer.enterIfNotThere(bind.symbol)
- }
+ for (bind @ Bind(name, _) <- cdef.pat)
+ if (name.toTermName != nme.WILDCARD && bind.symbol != null && bind.symbol != NoSymbol)
+ namer.enterIfNotThere(bind.symbol)
val guard1: Tree = if (cdef.guard == EmptyTree) EmptyTree
else typed(cdef.guard, BooleanClass.tpe)
@@ -4691,11 +4689,7 @@ trait Typers extends Adaptations with Tags {
if (!reallyExists(sym)) {
def handleMissing: Tree = {
- def errorTree = tree match {
- case _ if !forInteractive => tree
- case Select(_, _) => treeCopy.Select(tree, qual, name)
- case SelectFromTypeTree(_, _) => treeCopy.SelectFromTypeTree(tree, qual, name)
- }
+ def errorTree = missingSelectErrorTree(tree, qual, name)
def asTypeSelection = (
if (context.owner.enclosingTopLevelClass.isJavaDefined && name.isTypeName) {
atPos(tree.pos)(gen.convertToSelectFromType(qual, name)) match {
14 src/interactive/scala/tools/nsc/interactive/Global.scala
View
@@ -14,11 +14,20 @@ import scala.tools.nsc.util.MultiHashMap
import scala.reflect.internal.util.{ SourceFile, BatchSourceFile, Position, NoPosition }
import scala.tools.nsc.reporters._
import scala.tools.nsc.symtab._
+import scala.tools.nsc.doc.ScaladocAnalyzer
import scala.tools.nsc.typechecker.{ Analyzer, DivergentImplicit }
import symtab.Flags.{ACCESSOR, PARAMACCESSOR}
import scala.annotation.{ elidable, tailrec }
import scala.language.implicitConversions
+trait InteractiveScaladocAnalyzer extends InteractiveAnalyzer with ScaladocAnalyzer {
+ val global : Global
+ import global._
+ override def newTyper(context: Context) = new Typer(context) with InteractiveTyper with ScaladocTyper {
+ override def canAdaptConstantTypeToLiteral = false
+ }
+}
+
trait InteractiveAnalyzer extends Analyzer {
val global : Global
import global._
@@ -127,9 +136,10 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
if (verboseIDE) println("[%s][%s]".format(projectName, msg))
// don't keep the original owner in presentation compiler runs
- // (the map will grow indefinitely, and the only use case is the
- // backend).
+ // (the map will grow indefinitely, and the only use case is the backend)
override protected def saveOriginalOwner(sym: Symbol) { }
+ override protected def originalEnclosingMethod(sym: Symbol) =
+ abort("originalOwner is not kept in presentation compiler runs.")
override def forInteractive = true
11 src/interactive/scala/tools/nsc/interactive/tests/core/PresentationCompilerInstance.scala
View
@@ -13,11 +13,16 @@ private[tests] trait PresentationCompilerInstance extends TestSettings {
override def compiler = PresentationCompilerInstance.this.compiler
}
+ private class ScaladocEnabledGlobal extends Global(settings, compilerReporter) {
+ override lazy val analyzer = new {
+ val global: ScaladocEnabledGlobal.this.type = ScaladocEnabledGlobal.this
+ } with InteractiveScaladocAnalyzer
+ }
+
protected lazy val compiler: Global = {
prepareSettings(settings)
- new Global(settings, compilerReporter) {
- override def forScaladoc = withDocComments
- }
+ if (withDocComments) new ScaladocEnabledGlobal
+ else new Global(settings, compilerReporter)
}
/**
7 src/reflect/scala/reflect/internal/Required.scala
View
@@ -4,12 +4,9 @@ package internal
import settings.MutableSettings
trait Required { self: SymbolTable =>
-
def picklerPhase: Phase
-
def settings: MutableSettings
- def forInteractive: Boolean
-
- def forScaladoc: Boolean
+ @deprecated("Interactive is implemented with a custom Global; this flag is ignored", "2.11.0") def forInteractive = false
+ @deprecated("Scaladoc is implemented with a custom Global; this flag is ignored", "2.11.0") def forScaladoc = false
}
25 src/reflect/scala/reflect/internal/Symbols.scala
View
@@ -74,12 +74,15 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
// e.g. after flatten all classes are owned by package classes, there are lots and
// lots of these to be declared (or more realistically, discovered.)
protected def saveOriginalOwner(sym: Symbol) {
- // don't keep the original owner in presentation compiler runs
- // (the map will grow indefinitely, and the only use case is the
- // backend).
- if (!forInteractive) {
- if (originalOwner contains sym) ()
- else originalOwner(sym) = sym.rawowner
+ if (originalOwner contains sym) ()
+ else originalOwner(sym) = sym.rawowner
+ }
+ protected def originalEnclosingMethod(sym: Symbol): Symbol = {
+ if (sym.isMethod || sym == NoSymbol) sym
+ else {
+ val owner = originalOwner.getOrElse(sym, sym.rawowner)
+ if (sym.isLocalDummy) owner.enclClass.primaryConstructor
+ else originalEnclosingMethod(owner)
}
}
@@ -1920,15 +1923,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
* originalOwner map is not populated for memory considerations (the symbol
* may hang on to lazy types and in turn to whole (outdated) compilation units.
*/
- def originalEnclosingMethod: Symbol = {
- assert(!forInteractive, "originalOwner is not kept in presentation compiler runs.")
- if (isMethod) this
- else {
- val owner = originalOwner.getOrElse(this, rawowner)
- if (isLocalDummy) owner.enclClass.primaryConstructor
- else owner.originalEnclosingMethod
- }
- }
+ def originalEnclosingMethod: Symbol = Symbols.this.originalEnclosingMethod(this)
/** The method or class which logically encloses the current symbol.
* If the symbol is defined in the initialization part of a template
2  src/reflect/scala/reflect/runtime/JavaUniverse.scala
View
@@ -11,8 +11,6 @@ class JavaUniverse extends internal.SymbolTable with ReflectSetup with runtime.S
def inform(msg: String): Unit = log(msg)
def picklerPhase = internal.SomePhase
- def forInteractive = false
- def forScaladoc = false
lazy val settings = new Settings
private val isLogging = sys.props contains "scala.debug.reflect"
5 src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
View
@@ -24,6 +24,11 @@ trait ScaladocAnalyzer extends Analyzer {
override def canAdaptConstantTypeToLiteral = false
+ override protected def macroImplementationNotFoundMessage(name: Name): String = (
+ super.macroImplementationNotFoundMessage(name)
+ + "\nWhen generating scaladocs for multiple projects at once, consider using -Ymacro-no-expand to disable macro expansions altogether."
+ )
+
override def typedDocDef(docDef: DocDef, mode: Mode, pt: Type): Tree = {
val sym = docDef.symbol
3  test/files/neg/macro-basic-mamdmi.check
View
@@ -1,4 +1,5 @@
-Impls_Macros_Test_1.scala:36: error: macro implementation not found: foo (the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)
+Impls_Macros_Test_1.scala:36: error: macro implementation not found: foo
+(the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)
println(foo(2) + Macros.bar(2) * new Macros().quux(4))
^
one error found
3  test/files/neg/t5753.check
View
@@ -1,4 +1,5 @@
-Test_2.scala:9: error: macro implementation not found: foo (the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)
+Test_2.scala:9: error: macro implementation not found: foo
+(the most common reason for that is that you cannot use macro implementations in the same compilation run that defines them)
println(foo(42))
^
one error found
0  test/pending/presentation/doc.check → test/files/presentation/doc.check
View
File renamed without changes
16 test/pending/presentation/doc/doc.scala → test/files/presentation/doc/doc.scala
View
@@ -37,17 +37,23 @@ object Test extends InteractiveTest {
prepre + docComment(nTags) + prepost + post
}
-
-
override lazy val compiler = {
prepareSettings(settings)
- new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase {
+ new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase with doc.ScaladocGlobalTrait {
outer =>
+
val global: this.type = this
override lazy val analyzer = new {
val global: outer.type = outer
- } with doc.ScaladocAnalyzer
+ } with doc.ScaladocAnalyzer with InteractiveAnalyzer {
+ override def newTyper(context: Context): InteractiveTyper with ScaladocTyper =
+ new Typer(context) with InteractiveTyper with ScaladocTyper
+ }
+
+ override lazy val loaders = new scala.tools.nsc.symtab.SymbolLoaders {
+ val global: outer.type = outer
+ }
def chooseLink(links: List[LinkTo]): LinkTo = links.head
def internalLink(sym: Symbol, site: Symbol) = None
@@ -125,7 +131,7 @@ object Test extends InteractiveTest {
case s: Seq[_] => s exists (existsText(_, text))
case p: Product => p.productIterator exists (existsText(_, text))
}
- val (derived, base) = compiler.ask { () =>
+ val (derived, base) = compiler.ask { () =>
val derived = definitions.RootPackage.info.decl(newTermName("p")).info.decl(newTypeName("Derived"))
(derived, derived.ancestors(0))
}
0  test/pending/presentation/doc/src/Class.scala → test/files/presentation/doc/src/Class.scala
View
File renamed without changes
0  test/pending/presentation/doc/src/p/Base.scala → test/files/presentation/doc/src/p/Base.scala
View
File renamed without changes
0  test/pending/presentation/doc/src/p/Derived.scala → test/files/presentation/doc/src/p/Derived.scala
View
File renamed without changes
Paul Phillips

It means the error code is indescribably nightmarish, and holds us back in a huge way.

Eugene Burmako

Since this comment annotates a routine responsible for macro error handling, maybe you could elaborate on the problems in how the macro engine handles errors?

Paul Phillips

The story must be somewhat visible in the diff. It was a lengthy process in which I had zero interest to get the one piece of the error code talking to the other piece. See the access shuffling and parameter sprouting. The error code is spread out over every form of mutation known to man, at least 5x the quantity of code which should be necessary, and it's so difficult to modify that I have completely given up - which is not exactly good news for a language like scala, which really needs clear errors. There is no rhyme nor reason to the organization. There is no way of knowing how any given error is supposed to be reported.

The code is an albatross around my neck. I would give anything to get rid of it. It is like a nightmare from which I cannot wake up.

It has nothing to do with the macro engine per se. If you don't like the comment being too close to a bit of macro code, you can remove it. My LGTM is yours.

Please sign in to comment.
Something went wrong with that request. Please try again.