8000 Remove logic to check if a named arg could be an assignment (and enable `-Xsource:2.13` by default) by lrytz · Pull Request #6092 · scala/scala · GitHub
[go: up one dir, main page]
Skip to content

Remove logic to check if a named arg could be an assignment (and enable -Xsource:2.13 by default) #6092

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Dec 1, 2017
Merged

Remove logic to check if a named arg could be an assignment (and enable -Xsource:2.13 by default) #6092

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
960f0d8
Deprecate assignments in argument position
lrytz Sep 19, 2017
d5bf1d7
Make sure `@elidable` is only on concrete methods
lrytz Sep 22, 2017
ed914a8
Changes to allow compilation with -Xsource:2.13
lrytz Sep 22, 2017
a992c83
Change default Xsource to 2.13
lrytz Sep 21, 2017
870131b
Rename AssignOrNamedArg to NamedArg
lrytz Sep 21, 2017
926dbef
Remove logic to check if a named arg could be an assignment
lrytz Sep 21, 2017
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
Prev Previous commit
Remove logic to check if a named arg could be an assignment 
An `x = e` expression in parameter position is now a named argument,
syntactically, unconditionally.

Fixes scala/scala-dev#426
  • Loading branch information
@lrytz
lrytz committed Sep 22, 2017
commit 926dbef6d79d7d4118113047b92d12856b661feb
2 changes: 2 additions & 0 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ trait ScalaSettings extends AbsScalaSettings
def isScala212: Boolean = source.value >= version212
private[this] val version213 = ScalaVersion("2.13.0")
def isScala213: Boolean = source.value >= version213
private[this] val version214 = ScalaVersion("2.14.0")
def isScala214: Boolean = source.value >= version214

/**
* -X "Advanced" settings
Expand Down
32 changes: 4 additions & 28 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ trait ContextErrors {
case class NormalTypeError(underlyingTree: Tree, errMsg: String)
extends TreeTypeError

/**
* Marks a TypeError that was constructed from a CyclicReference (under silent).
* This is used for named arguments, where we need to know if an assignment expression
* failed with a cyclic reference or some other type error.
*/
class NormalTypeErrorFromCyclicReference(underlyingTree: Tree, errMsg: String)
extends NormalTypeError(underlyingTree, errMsg)

case class AccessTypeError(underlyingTree: Tree, errMsg: String)
extends TreeTypeError

Expand Down Expand Up @@ -1128,9 +1120,8 @@ trait ContextErrors {
// hence we (together with reportTypeError in TypeDiagnostics) make sure that this CyclicReference
// evades all the handlers on its way and successfully reaches `isCyclicOrErroneous` in Implicits
throw ex
case c @ CyclicReference(sym, info: TypeCompleter) =>
val error = new NormalTypeErrorFromCyclicReference(tree, typer.cyclicReferenceMessage(sym, info.tree) getOrElse ex.getMessage)
issueTypeError(error)
case CyclicReference(sym, info: TypeCompleter) =>
issueNormalTypeError(tree, typer.cyclicReferenceMessage(sym, info.tree) getOrElse ex.getMessage)
case _ =>
contextNamerErrorGen.issue(TypeErrorWithUnderlyingTree(tree, ex))
}
Expand Down Expand Up @@ -1322,24 +1313,9 @@ trait ContextErrors {
issueSymbolTypeError(sym, errMsg)
}

def AmbiguousReferenceInNamesDefaultError(arg: Tree, name: Name)(implicit context: Context) = {
if (!arg.isErroneous) { // check if name clash wasn't reported already
issueNormalTypeError(arg,
"reference to "+ name +" is ambiguous; it is both a method parameter "+
"and a variable in scope.")
setError(arg)
} else arg
}

def WarnAfterNonSilentRecursiveInference(param: Symbol, arg: Tree)(implicit context: Context) = {
val note = "failed to determine if '"+ param.name + " = ...' is a named argument or an assignment expression.\n"+
"an explicit type is required for the definition mentioned in the error message above."
context.warning(arg.pos, note)
}

def UnknownParameterNameNamesDefaultError(arg: Tree, name: Name, isVariableInScope: Boolean)(implicit context: Context) = {
def UnknownParameterNameNamesDefaultError(arg: Tree, name: Name, warnVariableInScope: Boolean)(implicit context: Context) = {
val suffix =
if (isVariableInScope)
if (warnVariableInScope)
s"\nNote that assignments in argument position are no longer allowed since Scala 2.13.\nTo express the assignment expression, wrap it in brackets, e.g., `{ $name = ... }`."
else ""
issueNormalTypeError(arg, s"unknown parameter name: $name$suffix")
Expand Down
93 changes: 2 additions & 91 deletions src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -486,82 +486,6 @@ trait NamesDefaults { self: Analyzer =>
} else NoSymbol
}

def isVariableInScope(context: Context, name: Name): Boolean = {
context.lookupSymbol(name, _.isVariable).isSuccess
}

/** A full type check is very expensive; let's make sure there's a name
* somewhere which could potentially be ambiguous before we go that route.
*/
private def isAmbiguousAssignment(typer: Typer, param: Symbol, arg: Tree) = {
import typer.context
(context isNameInScope param.name) && {
// for named arguments, check whether the assignment expression would
// typecheck. if it does, report an ambiguous error.
val paramtpe = param.tpe.cloneInfo(param)
// replace type parameters by wildcard. in the below example we need to
// typecheck (x = 1) with wildcard (not T) so that it succeeds.
// def f[T](x: T) = x
// var x = 0
// f(x = 1) << "x = 1" typechecks with expected type WildcardType
val udp = context.undetparams
context.savingUndeterminedTypeParams(reportAmbiguous = false) {
val subst = new SubstTypeMap(udp, udp map (_ => WildcardType)) {
override def apply(tp: Type): Type = super.apply(dropByName(tp))
}
// This throws an exception which is caught in `tryTypedApply` (as it
// uses `silent`) - unfortunately, tryTypedApply recovers from the
// exception if you use errorTree(arg, ...) and conforms is allowed as
// a view (see tryImplicit in Implicits) because it tries to produce a
// new qualifier (if the old one was P, the new one will be
// conforms.apply(P)), and if that works, it pretends nothing happened.
//
// To make sure tryTypedApply fails, we would like to pass EmptyTree
// instead of arg, but can't do that because eventually setType(ErrorType)
// is called, and EmptyTree can only be typed NoType. Thus we need to
// disable conforms as a view...
val errsBefore = reporter.ERROR.count
try typer.silent { tpr =>
val res = tpr.typed(arg.duplicate, subst(paramtpe))
// better warning for scala/bug#5044: if `silent` was not actually silent give a hint to the user
// [H]: the reason why `silent` is not silent is because the cyclic reference exception is
// thrown in a context completely different from `context` here. The exception happens while
// completing the type, and TypeCompleter is created/run with a non-silent Namer `context`
// and there is at the moment no way to connect the two unless we go through some global state.
if (errsBefore < reporter.ERROR.count)
WarnAfterNonSilentRecursiveInference(param, arg)(context)
res
} match {
case SilentResultValue(t) =>
!t.isErroneous // #4041
case SilentTypeError(e: NormalTypeErrorFromCyclicReference) =>
// If we end up here, the CyclicReference was reported in a silent context. This can
// happen for local definitions, when the completer for a definition is created during
// type checking in silent mode. ContextErrors.TypeSigError catches that cyclic reference
// and transforms it into a NormalTypeErrorFromCyclicReference.
// The cycle needs to be reported, because the program cannot be typed: we don't know
// if we have an assignment or a named arg.
context.issue(e)
// 'err = true' is required because we're in a silent context
WarnAfterNonSilentRecursiveInference(param, arg)(context)
false
case _ =>
// We got a type error, so it cannot be an assignment (it doesn't type check as one).
false
}
catch {
// `silent` only catches and returns TypeErrors which are not
// CyclicReferences. Fix for #3685
case cr @ CyclicReference(sym, _) =>
(sym.name == param.name) && sym.accessedOrSelf.isVariable && {
NameClashError(sym, arg)(context)
true
}
}
}
}
}

/** Removes name assignments from args. Additionally, returns an array mapping
* argument indices from call-site-order to definition-site-order.
*
Expand Down Expand Up @@ -597,28 +521,15 @@ trait NamesDefaults { self: Analyzer =>
def stripNamedArg(arg: NamedArg, argIndex: Int): Tree = {
val NamedArg(Ident(name), rhs) = arg
params indexWhere (p => matchesName(p, name, argIndex)) match {
case -1 if positionalAllowed && !settings.isScala213 =>
if (isVariableInScope(context0, name)) {
// only issue the deprecation warning if `name` is in scope, this avoids the warning when mis-spelling a parameter name.
context0.deprecationWarning(
arg.pos,
context0.owner,
s"assignments in argument position are deprecated in favor of named arguments. Wrap the assignment in brackets, e.g., `{ $name = ... }`.",
"2.12.4")
}
// prevent isNamed from being true when calling doTypedApply recursively,
// treat the arg as an assignment of type Unit
Assign(arg.lhs, rhs) setPos arg.pos
case -1 =>
UnknownParameterNameNamesDefaultError(arg, name, isVariableInScope(context0, name))
val warnVariableInScope = !settings.isScala214 && context0.lookupSymbol(name, _.isVariable).isSuccess
UnknownParameterNameNamesDefaultError(arg, name, warnVariableInScope)
case paramPos if argPos contains paramPos =>
val existingArgIndex = argPos.indexWhere(_ == paramPos)
val otherName = Some(args(paramPos)) collect {
case NamedArg(Ident(oName), _) if oName != name => oName
}
DoubleParamNamesDefaultError(arg, name, existingArgIndex+1, otherName)
case paramPos if !settings.isScala213 && isAmbiguousAssignment(typer, params(paramPos), arg) =>
AmbiguousReferenceInNamesDefaultError(arg, name)
case paramPos if paramPos != argIndex =>
positionalAllowed = false // named arg is not in original parameter order: require names after this
argPos(argIndex) = paramPos // fix up the arg position
Expand Down
0