Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

SI-7345 Prefer using a throwaway silent context over buffer flushing.

When we are using a throwaway silent context, we can just let it drift out of
scope and over the horizon, rather than ceremoniously flushing its buffers
on completion.

 - Applied to Scaladoc.
 - Applied to Infer#isApplicableSafe. Less manual error buffer management
   affords greater opportunity to cleanly express the logic.
 - Applied to `typerReportAnyContextErrors`.

The reasoning for the last case is as follows:

 - There were only two callers to `typerReportAnyContextErrors`, and
   they both passed in as `c` a child context of `context`.
 - That child context must share the error reporting mode and buffer
   with `context`.
 - Therefore, extracting an error from `c` and issuing it into `context`
   is a no-op. Because the error buffer is Set, it was harmless.

This part will probably textually conflict with the same change made in
SI-7319, but the end results are identical.
  • Loading branch information...
commit 510ebeca9e55b27bbe6e6c54bf0ea6ea294ee7be 1 parent ec5eaee
Jason Zaugg retronym authored
10 src/compiler/scala/tools/nsc/typechecker/Contexts.scala
View
@@ -261,11 +261,13 @@ trait Contexts { self: Analyzer =>
reportBuffer.clearAllErrors()
current
}
- /** Return and clear all warnings from the report buffer */
- def flushAndReturnWarningsBuffer(): immutable.Seq[(Position, String)] = {
- val current = reportBuffer.warnings
+
+ /** Issue and clear all warnings from the report buffer */
+ def flushAndIssueWarnings() {
+ reportBuffer.warnings foreach {
+ case (pos, msg) => unit.warning(pos, msg)
+ }
reportBuffer.clearAllWarnings()
- current
}
//
29 src/compiler/scala/tools/nsc/typechecker/Infer.scala
View
@@ -910,19 +910,28 @@ trait Infer extends Checkable {
}
/**
- * Todo: Try to make isApplicable always safe (i.e. not cause TypeErrors).
- * The chance of TypeErrors should be reduced through context errors
+ * Are arguments of the given types applicable to `ftpe`? Type argument inference
+ * is tried twice: firstly with the given expected type, and secondly with `WildcardType`.
*/
+ // Todo: Try to make isApplicable always safe (i.e. not cause TypeErrors).
+ // The chance of TypeErrors should be reduced through context errors
private[typechecker] def isApplicableSafe(undetparams: List[Symbol], ftpe: Type,
argtpes0: List[Type], pt: Type): Boolean = {
- val silentContext = context.makeSilent(reportAmbiguousErrors = false)
- val typer0 = newTyper(silentContext)
- val res1 = typer0.infer.isApplicable(undetparams, ftpe, argtpes0, pt)
- if (pt != WildcardType && silentContext.hasErrors) {
- silentContext.flushBuffer()
- val res2 = typer0.infer.isApplicable(undetparams, ftpe, argtpes0, WildcardType)
- if (silentContext.hasErrors) false else res2
- } else res1
+ final case class Result(error: Boolean, applicable: Boolean)
+ def isApplicableWithExpectedType(pt0: Type): Result = {
+ val silentContext = context.makeSilent(reportAmbiguousErrors = false)
+ val applicable = newTyper(silentContext).infer.isApplicable(undetparams, ftpe, argtpes0, pt0)
+ Result(silentContext.hasErrors, applicable)
+ }
+ val canSecondTry = pt != WildcardType
+ val firstTry = isApplicableWithExpectedType(pt)
+ if (!firstTry.error || !canSecondTry)
+ firstTry.applicable
+ else {
+ val secondTry = isApplicableWithExpectedType(WildcardType)
+ // TODO `!secondTry.error &&` was faithfully replicated as part of the refactoring, but mayberedundant.
+ !secondTry.error && secondTry.applicable
+ }
}
/** Is type `ftpe1` strictly more specific than type `ftpe2`
52 src/compiler/scala/tools/nsc/typechecker/Typers.scala
View
@@ -489,20 +489,9 @@ trait Typers extends Adaptations with Tags {
res
}
- @inline
- final def typerReportAnyContextErrors[T](c: Context)(f: Typer => T): T = {
- val res = f(newTyper(c))
- c.firstError match {
- case Some(err) =>
- context.issue(err)
- c.reportBuffer.warnings foreach {
- case (pos, msg) => context.warning(pos, msg)
- }
- // Seemingly we should perform `c.reportBuffer.clearAll()` here. But
- // `context` might share the ReportBuffer with `c`.
- case None =>
- }
- res
+ final def newImplTyper[T](impl: ImplDef, clazz: Symbol): Typer = {
+ val c = context.make(impl.impl, clazz, newScope)
+ newTyper(c)
}
@inline
@@ -701,11 +690,7 @@ trait Typers extends Adaptations with Tags {
SilentTypeError(err)
case None =>
// If we have a successful result, emit any warnings it created.
- if (context1.reportBuffer.hasWarnings) {
- context1.flushAndReturnWarningsBuffer() foreach {
- case (pos, msg) => unit.warning(pos, msg)
- }
- }
+ context1.flushAndIssueWarnings()
SilentResultValue(result)
}
} else {
@@ -1812,9 +1797,7 @@ trait Typers extends Adaptations with Tags {
assert(clazz != NoSymbol, cdef)
reenterTypeParams(cdef.tparams)
val tparams1 = cdef.tparams mapConserve (typedTypeDef)
- val impl1 = typerReportAnyContextErrors(context.make(cdef.impl, clazz, newScope)) {
- _.typedTemplate(cdef.impl, typedParentTypes(cdef.impl))
- }
+ val impl1 = newImplTyper(cdef, clazz).typedTemplate(cdef.impl, typedParentTypes(cdef.impl))
val impl2 = finishMethodSynthesis(impl1, clazz, context)
if (clazz.isTrait && clazz.info.parents.nonEmpty && clazz.info.firstParent.typeSymbol == AnyClass)
checkEphemeral(clazz, impl2.body)
@@ -1855,17 +1838,16 @@ trait Typers extends Adaptations with Tags {
|| !linkedClass.isSerializable
|| clazz.isSerializable
)
- val impl1 = typerReportAnyContextErrors(context.make(mdef.impl, clazz, newScope)) {
- _.typedTemplate(mdef.impl, {
- typedParentTypes(mdef.impl) ++ (
- if (noSerializable) Nil
- else {
- clazz.makeSerializable()
- List(TypeTree(SerializableClass.tpe) setPos clazz.pos.focus)
- }
- )
- })
- }
+ val impl1 = newImplTyper(mdef, clazz).typedTemplate(mdef.impl, {
+ typedParentTypes(mdef.impl) ++ (
+ if (noSerializable) Nil
+ else {
+ clazz.makeSerializable()
+ List(TypeTree(SerializableClass.tpe) setPos clazz.pos.focus)
+ }
+ )
+ })
+
val impl2 = finishMethodSynthesis(impl1, clazz, context)
// SI-5954. On second compile of a companion class contained in a package object we end up
@@ -4360,11 +4342,9 @@ trait Typers extends Adaptations with Tags {
case ex: CyclicReference =>
throw ex
case te: TypeError =>
- // @H some of typer erros can still leak,
+ // @H some of typer errors can still leak,
// for instance in continuations
None
- } finally {
- c.flushBuffer()
}
}
9 src/scaladoc/scala/tools/nsc/doc/model/ModelFactoryImplicitSupport.scala
View
@@ -226,12 +226,9 @@ trait ModelFactoryImplicitSupport {
// look for type variables in the type. If there are none, we can decide if the implicit is there or not
if (implType.isTrivial) {
try {
- context.flushBuffer() /* any errors here should not prevent future findings */
- // TODO: Not sure this is the right thing to do -- seems similar to what scalac should be doing
- val context2 = context.make(owner = sym.owner)
- val search = inferImplicit(EmptyTree, tpe, false, false, context2, false)
- context.flushBuffer() /* any errors here should not prevent future findings */
-
+ // TODO: Not sure if `owner = sym.owner` is the right thing to do -- seems similar to what scalac should be doing
+ val silentContext = context.make(owner = sym.owner).makeSilent(reportAmbiguousErrors = false)
+ val search = inferImplicit(EmptyTree, tpe, false, false, silentContext, false)
available = Some(search.tree != EmptyTree)
} catch {
case _: TypeError =>
Please sign in to comment.
Something went wrong with that request. Please try again.