Skip to content

Issue "positional after named argument" errors #18363

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 2 commits into from
Aug 21, 2023
Merged

Issue "positional after named argument" errors #18363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddc4cd0
Issue "positional after named argument" errors
odersky Aug 8, 2023
91e56de
Fix test
odersky Aug 8, 2023
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
35 changes: 26 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,21 @@ trait Applications extends Compatibility {
def infoStr = if methType.isErroneous then "" else i": $methType"
i"${err.refStr(methRef)}$infoStr"

/** Re-order arguments to correctly align named arguments */
/** Re-order arguments to correctly align named arguments
* Issue errors in the following situations:
*
* - "positional after named argument" if a positional argument follows a named
* argument and one of the following is true:
*
* - There is a formal argument before the argument position
* that has not yet been instantiated with a previous actual argument,
* (either named or positional), or
* - The formal parameter at the argument position is also mentioned
* in a subsequent named parameter.
* - "parameter already instantiated" if a two named arguments have the same name.
* - "does not have parameter" if a named parameter does not mention a formal
* parameter name.
*/
def reorder[T <: Untyped](args: List[Trees.Tree[T]]): List[Trees.Tree[T]] = {

/** @param pnames The list of parameter names that are missing arguments
Expand All @@ -517,40 +531,43 @@ trait Applications extends Compatibility {
* 2. For every `(name -> arg)` in `nameToArg`, `arg` is an element of `args`
*/
def handleNamed(pnames: List[Name], args: List[Trees.Tree[T]],
nameToArg: Map[Name, Trees.NamedArg[T]], toDrop: Set[Name]): List[Trees.Tree[T]] = pnames match {
nameToArg: Map[Name, Trees.NamedArg[T]], toDrop: Set[Name],
missingArgs: Boolean): List[Trees.Tree[T]] = pnames match {
case pname :: pnames1 if nameToArg contains pname =>
// there is a named argument for this parameter; pick it
nameToArg(pname) :: handleNamed(pnames1, args, nameToArg - pname, toDrop + pname)
nameToArg(pname) :: handleNamed(pnames1, args, nameToArg - pname, toDrop + pname, missingArgs)
case _ =>
def pnamesRest = if (pnames.isEmpty) pnames else pnames.tail
args match {
case (arg @ NamedArg(aname, _)) :: args1 =>
if (toDrop contains aname) // argument is already passed
handleNamed(pnames, args1, nameToArg, toDrop - aname)
handleNamed(pnames, args1, nameToArg, toDrop - aname, missingArgs)
else if ((nameToArg contains aname) && pnames.nonEmpty) // argument is missing, pass an empty tree
genericEmptyTree :: handleNamed(pnames.tail, args, nameToArg, toDrop)
genericEmptyTree :: handleNamed(pnames.tail, args, nameToArg, toDrop, missingArgs = true)
else { // name not (or no longer) available for named arg
def msg =
if (methodType.paramNames contains aname)
em"parameter $aname of $methString is already instantiated"
else
em"$methString does not have a parameter $aname"
fail(msg, arg.asInstanceOf[Arg])
arg :: handleNamed(pnamesRest, args1, nameToArg, toDrop)
arg :: handleNamed(pnamesRest, args1, nameToArg, toDrop, missingArgs)
}
case arg :: args1 =>
arg :: handleNamed(pnamesRest, args1, nameToArg, toDrop) // unnamed argument; pick it
if toDrop.nonEmpty || missingArgs then
report.error(i"positional after named argument", arg.srcPos)
arg :: handleNamed(pnamesRest, args1, nameToArg, toDrop, missingArgs) // unnamed argument; pick it
case Nil => // no more args, continue to pick up any preceding named args
if (pnames.isEmpty) Nil
else handleNamed(pnamesRest, args, nameToArg, toDrop)
else handleNamed(pnamesRest, args, nameToArg, toDrop, missingArgs)
}
}

def handlePositional(pnames: List[Name], args: List[Trees.Tree[T]]): List[Trees.Tree[T]] =
args match {
case (arg: NamedArg @unchecked) :: _ =>
val nameAssocs = for (case arg @ NamedArg(name, _) <- args) yield (name, arg)
handleNamed(pnames, args, nameAssocs.toMap, Set())
handleNamed(pnames, args, nameAssocs.toMap, toDrop = Set(), missingArgs = false)
case arg :: args1 =>
arg :: handlePositional(if (pnames.isEmpty) Nil else pnames.tail, args1)
case Nil => Nil
Expand Down
52 changes: 52 additions & 0 deletions tests/neg/i18122.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
-- Error: tests/neg/i18122.scala:10:16 ---------------------------------------------------------------------------------
10 | foo1(y = 1, 2, x = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:11:16 ---------------------------------------------------------------------------------
11 | foo2(y = 1, 2, x = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:12:16 ---------------------------------------------------------------------------------
12 | foo1(y = 1, 2, z = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:13:16 ---------------------------------------------------------------------------------
13 | foo2(y = 1, 2, z = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:14:16 ---------------------------------------------------------------------------------
14 | foo1(y = 1, 2) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:15:16 ---------------------------------------------------------------------------------
15 | foo2(y = 1, 2) // error: positional after named
| ^
| positional after named argument
-- [E171] Type Error: tests/neg/i18122.scala:17:8 ----------------------------------------------------------------------
17 | bar1() // error: missing arg
| ^^^^^^
| missing argument for parameter x of method bar1 in object Test: (x: Int, ys: Int*): Unit
-- [E171] Type Error: tests/neg/i18122.scala:23:8 ----------------------------------------------------------------------
23 | bar1(ys = 1) // error: missing arg
| ^^^^^^^^^^^^
| missing argument for parameter x of method bar1 in object Test: (x: Int, ys: Int*): Unit
-- Error: tests/neg/i18122.scala:43:16 ---------------------------------------------------------------------------------
43 | bar1(x = 1, 2, ys = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:44:18 ---------------------------------------------------------------------------------
44 | bar1(1, 2, ys = 3) // error: parameter ys is already instantiated
| ^^^^^^
| parameter ys of method bar1 in object Test: (x: Int, ys: Int*): Unit is already instantiated
-- Error: tests/neg/i18122.scala:45:16 ---------------------------------------------------------------------------------
45 | bar2(x = 1, 2, ys = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:46:17 ---------------------------------------------------------------------------------
46 | bar1(ys = 1, 2, x = 3) // error: positional after named
| ^
| positional after named argument
-- Error: tests/neg/i18122.scala:47:17 ---------------------------------------------------------------------------------
47 | bar2(ys = 1, 2, x = 3) // error: positional after named
| ^
| positional after named argument
49 changes: 49 additions & 0 deletions tests/neg/i18122.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
object Test {
def foo1(x: Int, y: Int, z: Int) = println((x, y, z))
def foo2(x: Int = 0, y: Int, z: Int) = println((x, y, z))
def bar1(x: Int, ys: Int*) = println((x, ys))
def bar2(x: Int = 0, ys: Int*) = println((x, ys))

def main(args: Array[String]) = {
foo1(1, y = 2, 3)
foo2(1, y = 2, 3)
foo1(y = 1, 2, x = 3) // error: positional after named
foo2(y = 1, 2, x = 3) // error: positional after named
foo1(y = 1, 2, z = 3) // error: positional after named
foo2(y = 1, 2, z = 3) // error: positional after named
foo1(y = 1, 2) // error: positional after named
foo2(y = 1, 2) // error: positional after named

bar1() // error: missing arg
bar2()
bar1(1)
bar2(1)
bar1(x = 1)
bar2(x = 1)
bar1(ys = 1) // error: missing arg
bar2(ys = 1)
bar1(1, 2)
bar2(1, 2)
bar1(1, ys = 2)
bar2(1, ys = 2)
bar1(x = 1, 2)
bar2(x = 1, 2)
bar1(x = 1, ys = 2)
bar2(x = 1, ys = 2)
bar1(ys = 1, x = 2)
bar2(ys = 1, x = 2)
bar1(1, 2, 3)
bar2(1, 2, 3)
bar1(1, ys = 2, 3)
bar2(1, ys = 2, 3)
bar1(x = 1, 2, 3)
bar2(x = 1, 2, 3)
bar1(x = 1, ys = 2, 3)
bar2(x = 1, ys = 2, 3)
bar1(x = 1, 2, ys = 3) // error: positional after named
bar1(1, 2, ys = 3) // error: parameter ys is already instantiated
bar2(x = 1, 2, ys = 3) // error: positional after named
bar1(ys = 1, 2, x = 3) // error: positional after named
bar2(ys = 1, 2, x = 3) // error: positional after named
}
Copy link
Contributor

Choose a reason for hiding this comment

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

bar2(x = 1, ys = 2, ys = 3)? probably parameter ys is already instantiated.

bar2(x = 1, 2, ys = 3) // error: positional after named on L45 is slightly confusing message.

I expect that if named args do not change ordering, then it's always OK.

I see the rule that's being enforced is that ys always names the start of the repeated param. Then the positional arg 2 is unknown detritus.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All true, but positional after named is technically correct, and it would be messy to change the logic to emit a different error message. Keeping logic clean in the compiler is an underestimated good. Lose it and you enter quickly a downward slide where in the end nobody understands the code anymore.

}
2 changes: 1 addition & 1 deletion tests/pos-with-compiler-cc/dotc/transform/PickleQuotes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class PickleQuotes extends MacroTransform {
contents += content
val holeType =
if isTerm then getTermHoleType(tree.tpe) else getTypeHoleType(tree.tpe)
val hole = cpy.Hole(tree)(content = EmptyTree, TypeTree(holeType))
val hole = cpy.Hole(tree)(content = EmptyTree, tpt = TypeTree(holeType))
if isTerm then Inlined(EmptyTree, Nil, hole).withSpan(tree.span) else hole
case tree: DefTree =>
val newAnnotations = tree.symbol.annotations.mapconserve { annot =>
Expand Down