Skip to content
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.

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

sem: varargs now have and conform to a specification #1207

Open
wants to merge 20 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ which means derived object should correctly pass a slice/ptr to the procedure.
When `varargs` procedure is called, temporary array with values is constructed, before
being passed to the called procedure. If derived object is passed, correct `.Sup` must
be used to achieve desired behavior.

'''
knownIssue: "varargs cannot pass polymorphic derived types"

"""

var invalidAssings: seq[string]
var invalidAssigns: seq[string]

block regular_types:
## Issue is present for both regular and generic code
Expand All @@ -36,19 +34,19 @@ block regular_types:
test(Base())

except ObjectAssignmentDefect:
invalidAssings.add "Base()"
invalidAssigns.add "Base()"

try:
test(Base(), Derived1())

except ObjectAssignmentDefect:
invalidAssings.add "Base(), Derived1()"
invalidAssigns.add "Base(), Derived1()"

try:
test(Derived1(), Derived2())

except ObjectAssignmentDefect:
invalidAssings.add "Derived1(), Derived2()"
invalidAssigns.add "Derived1(), Derived2()"


block:
Expand Down Expand Up @@ -84,8 +82,8 @@ block:
) == "243", "Passing subtypes via varargs must work the same way as mutliple arguments"

except ObjectAssignmentdefect:
invalidAssings.add "Derived1, Derived, Base"
invalidAssigns.add "Derived1, Derived, Base"


doAssert invalidAssings.len == 0, "Failed object assignment for " & $invalidAssings.len &
" cases - " & $invalidAssings
doAssert invalidAssigns.len == 0, "Failed object assignment for " & $invalidAssigns.len &
" cases - " & $invalidAssigns
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ Passing `var` subtype to the varargs results in C codegen error.
When fixed should be added to the `subtype_match/subtype_varargs` spec section.
'''
knownIssue: "cannot pass varargs of var subtype due to codegen error"

"""

type
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
discard """
description: '''
A `varargs` exact match should precede one with an explicit conversion
'''
knownIssue: "a match without conversion is better than one with"
"""

# xxx: revisit if the implementation is too difficult

type
C = object

proc toC(i: int): C = C()

proc impl(args: varargs[C, toC]): string = "C"
proc impl(args: varargs[int]): string = "int"

## Should pass because `varargs[C, toC]` has a lower match precedence than
## `varargs[int]`.
doAssert impl(1, 2, 3, 4) == "int"
22 changes: 0 additions & 22 deletions tests/lang/s02_core/t03_overload_core_ambiguous_varargs.nim

This file was deleted.

11 changes: 5 additions & 6 deletions tests/lang_callable/converter/tconverter_with_varargs.nim
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@

# bug #888
# bug https://github.com/nim-lang/Nim/issues/888

type
PyRef = object
PPyRef* = ref PyRef

converter to_py*(i: int) : PPyRef = nil
converter to_py*(i: int): PPyRef = nil

when false:
proc to_tuple*(vals: openarray[PPyRef]): PPyRef =
discard
proc to_tuple*(vals: openarray[PPyRef]): PPyRef =
discard

proc abc(args: varargs[PPyRef]) =
#let args_tup = to_tuple(args)
let args_tup = to_tuple(args)
discard

abc(1, 2)
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
discard """
description: "Disallow setting a generic param to varargs"
errormsg: "it is illegal to set a type parameter to varargs"
line: 12
knownIssue: "compiler doesn't guard against this yet"
"""


block cannot_set_varargs_as_type_parameter:
proc foo[T](a: T) =
discard

foo[varargs[int]](1, 2, 3)
138 changes: 138 additions & 0 deletions tests/lang_callable/varargs/tvarargs.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
discard """
description: "Tests for varargs, in particular during overload resolution."
disabled: true
"""

#[
TODO:
(This is a list of all the properties/scenarios to test wrt varargs, if marked
as done, then the test is written, but open questions might remain.)
- [x] non-ast varargs:
- [x] non-ast trailing varargs:
- [x] are implicitly converted to a `bracketExpr`
- [x] allow for a conversion call
- [x] non-ast non-trailing varargs
- [x] are converted to an `argList`??? (still has open question, see below)
- [x] allows for a conversion call
- [x] match no more args than remaining non-defaulted/vararg formals
- [x] subtype relationship matching
- [x] range
- [x] inheritance object (see: tvararg_polymorphic, t01_varargs_does_not_support_polymorphic, t07_varargs_var_subtype, t08_varargs_regular_subtype)
- [x] inheritance ref object (see: tvararg_polymorphic)
- [x] with converters (see: tconverter_with_varargs)
- [x] error: widening
- [x] overloads
- [x] arity has precedence:
- [x] match no-param candidate over varargs
- [x] match one-param candidate over varargs
- [x] match non-varargs generic over empty varargs
- [x] match non-varargs generic over varargs (open question, see below)
- [x] match non-generic varargs candidate over generic varargs candidate
- [x] match openArray param candidate over varargs
- [x] match array param candidate over varargs
- [x] match seq param candidate over varargs
- [x] match non-conversion candidate over with conversion (see: t03_overload_core_ambiguous_varargs)
- [x] conversion loses to subtype
- [x] conversion loses to widening
- [x] conversion loses to inheritance object
- [x] conversion loses to inheritance ref object
- [x] conversion wins over converter
- [x] error: varargs with/without conversions are ambiguous
- [x] generics:
- [x] infer the type parameter
- [x] generic parameter cannot be set to varargs (see open question below)
- [x] ast varargs:
- [x] untyped varargs:
- [x] override named parameter checking/act as "rest" param (see: tvarargsexpr)
- [x] check untyped ast:
- [x] bracket expr/array params are not flattened (see: tvarargsexpr)
- [x] constructs an nnkArgList (see: tvarargsexpr)
- [x] error:
- [x] ast varargs forces comptime context
- [x] must only be trailing/positional
- [x] only one such param per routine
- [x] will consume following AST if "repositioned" via named param
- [x] disallows conversion parameter (open question, see below)
- [x] ensure varargs[typed] param always constructs a bracket wrapper
- [x] scenario: typed varargs param after some_type param
- [x] typed allows for a conversion call (still has open question, see below)
- [x] limit greediness based on trailing param count
- [x] errors:
- [x] ast varargs forces comptime context
- [x] only one such param per routine
- [x] may only be followed by non-vararg/non-defaulted params
- [x] call arguments given for `varargs[typed]` params cannot be
repositioned via named argument syntax at the callsite
- [ ] varargs pragma
- [x] errors: forces runtime context
- [ ] ensure automatic string to cstring conversion
- [x] defensive/regression tests:
- [x] sigmatch container re-use logic doesn't clobber existing bracket
exprs (see: tvararg_exact_match)

FAQ:
- [x] when passing a valid call with non-ast varargs to a typed macro
parameter, should they always become bracket expressions, or should we
only do that with trailing varargs? At least then we'd treat them
consistently as a form of `openArray` parameters.
- [x] Should the handling be different between meta/non-meta routines?
Perhaps macros would rather have non-trailing as an `argList`,
instead of `bracket`, as each arg in the vararg is more like
a specialized `typed[T]` at that point.
- future direction: meta-routines should always wrap in an `arglist` and
non-meta-routines should always use `bracket`

- [x] should we support widening as the tests do at present?
- the implementation is likely going to be very complex, so the plan is
to get everything else working first, and then revisit

- [x] echo "requires", `varargs[typed, $]`, but this doesn't quite make sense:
- [x] shouldn't type, in this case `typed`, represent the output of the
conversion routine, `$`? an affirmative would imply:
- no conversion functions for untyped varargs
- we're treating `typed` as an "all", a type which is all types,
as opposed to `any`, which is any one type, neat!
- disallow conversions on `varargs[untyped]`
- future direction: rework `echo` to not require `varargs[typed, $]`
- future direction: introduce `arrayargs[T]`, or something, that does the
always wrapping in `bracket` that `varargs[typed]`
does, then `arrayargs[string, $]` can mean, take a
variable number of arguments, ensure they're all
`string` typed, via a routine `$`, and assemble an
array for an `openArray[string]` param. at that point
`var/arrayargs[T, conv]`, `T` is the final type
- future direction: disallow conversions on `varargs[typed]`

- [x] should we be able to set a generic parameter to a varargs?
personally, I think this is a bad idea for presently hard to fathom
reasons, one issue I can see is that we cannot infer `varargs` and they
seem to be a combination type + argument passing convention (syntax and
semantics). I believe it would move us in the wrong direction, treating
generics as templates, instead of types. Where passing in `varargs`
would break things like the arity of a generic routine vs its instance
- future direction: we should disallow this

- [x] should `varargs[untyped]` respect parameter passing and boundaries,
instead of acting as a "rest of the passed argument AST"?
- yes, it should continue doing this, as programmers would no longer be
able to create APIs such as those found in `htmlgen`, for HTML element
construction.

Open Questions:
- [ ] optimization: conversion calls shouldn't happen if the type is correct?
- [ ] implies: conversion calls are side-effect free, also should they be
anyways?

- [ ] should varargs arity precedence change based on how many matched, 0 vs 1
vs many?
- [x] where given a parameter `a: varargs[int]`, 0, 1, and many are lower
than a routine with `a: int`, but 1 is higher than a generic
routine with `a: T`, with `T` inferred as `int`.
- yes, this should impact precedence, implementation will clarify
if there are major issues
- [ ] similar question wrt to varargs vs openarray across generic and
non-generic overloads


N.B.: if an item is checked it's in the `tvarargs` test, otherwise there is a `(see: ...)` note
]#
12 changes: 12 additions & 0 deletions tests/lang_callable/varargs/tvarargs_generic_properties.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
discard """
description: "Tests for generic varargs properties"
"""


block generics_properties:
block generic_varargs_infer_the_type_T:
proc foo[T](v: varargs[T]): int =
doAssert typeof(v[0]) is T
v.len

doAssert foo(1, 2, 3) == 3
94 changes: 94 additions & 0 deletions tests/lang_callable/varargs/tvarargs_non_ast_basic.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
discard """
description: "Tests for non-ast varargs, those that are not `un/typed`."
"""

import std/macros

block single_varargs:
proc foo(v: varargs[int]): int = v.len

doAssert foo() == 0
doAssert foo(9) == 1
doAssert foo(1, 2, 3) == 3

block a_var_arg_position_can_be_satisfied_by_an_openarray:
doAssert foo([1, 2, 3, 4]) == 4, "varargs as array"
doAssert foo(@[1, 2, 3]) == 3, "varargs as sequence"

macro checkStructure(call: typed, argArray: openArray[int]): bool =
## check the array conversion performed on trailing varargs
newTree(nnkCall, ident"==", call[1], argArray)

doAssert checkStructure(foo(), []), "empty array for no varargs"
doAssert checkStructure(foo(9), [9]), "single item array for one varargs"
doAssert checkStructure(foo(1, 2), [1, 2]), "all items in an array for varargs"


block trailing_varargs:
block with_leading_param:
proc foo(leading: string, v: varargs[int]): int =
v.len

doAssert foo("foo") == 0, "no varargs provided"
doAssert foo("foo", 1) == 1, "one varargs provided"
doAssert foo("foo", 1, 2) == 2, "two varargs provided"
doAssert foo("foo", [1, 2, 3]) == 3, "3 provided as an array"
doAssert foo("foo", @[1, 2]) == 2, "2 provided as a sequnce"

block with_a_leading_defaulted_param:
proc foo(leading = 3, v: varargs[int]): int =
doAssert v.len == 0
leading

doAssert foo() == 3


block non_trailing_varargs:
# xxx: this should probably be at a lower precendence
proc foo(v: varargs[int], trailing: string): int =
v.len

block knownIssue:
# doAssert foo("foo") == 0, "leading match with zero candidates"
discard "knownIssue: test in `tvarargs_non_ast_types_basic_known_issue_3"

doAssert foo(1, "foo") == 1, "one leading varargs provided"
doAssert foo(1, 2, "foo") == 2, "two leading varargs provided"
doAssert foo([1, 2, 3], "foo") == 3, "3 leading provided as array"
doAssert foo(@[1, 2], "foo") == 2, "2 leading provided as sequence"


block limit_greediness_of_varargs_parameter_consumption:
## following non-defaulted/vararg params means: varargs must stop matching in
## order to leave enough for following params
block match_args_with_enough_for_each_trailing_param:
func foo(_: varargs[string], wutboutme: string) =
doAssert wutboutme == "we didn't forget"

foo("test", "best", "this", "one too", "we didn't forget")

block defaulted_dont_count:
discard "knownIssue: tvarargs_non_ast_types_basic_known_issue_1"

block varargs_dont_count:
discard "knownIssue: tvarargs_non_ast_types_basic_known_issue_1"


block multiple_varargs:
block same_types:
proc foo(v: varargs[int], w: varargs[int]): (int, int) =
(v.len, w.len)

discard "knownIssue: test in `tvarargs_non_ast_types_basic_known_issue_2"

block first_vararg_will_match_all:
discard "knownIssue: test in `tvarargs_non_ast_types_basic_known_issue_2"

block differing_types:
proc foo(v: varargs[int], w: varargs[bool]): (int, int) =
(v.len, w.len)

discard "knownIssue: tests in `tvarargs_non_ast_types_basic_known_issues_4"

block wraps_in_bracket:
discard "knownIssue: tests in `tvarargs_non_ast_types_basic_known_issues_4"
Loading
Loading