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

Support conditional access for function calls with args #7

Closed
mattaylor opened this issue Jan 14, 2022 · 9 comments
Closed

Support conditional access for function calls with args #7

mattaylor opened this issue Jan 14, 2022 · 9 comments

Comments

@mattaylor
Copy link
Owner

mattaylor commented Jan 14, 2022

Current conditional access templates fail to compile when matching function chains with extra args
The following fail to compile...

let tab = { "one": "uno" }.toTable
check(tab.?getOrDefault("one") == "uno") 
check(tab.?getOrDefault("two") == "")
check(tab.?getOrDefault("two", "unknown") == "unknown")
@geekrelief
Copy link
Contributor

What's the error when it fails? I'll take a look in a bit.

@mattaylor
Copy link
Owner Author

mattaylor commented Jan 14, 2022

template/generic instantiation of `.?` from here
/Users/mat/work/elvis/tests.nim(99, 49) Error: in expression 'tab.getOrDefault("one")': identifier expected, but found 'getOrDefault("one")'

I don't know how to capture the argument one in a template....

I was hoping for something like..

template `.?`*[T,U,V](left: T, right: proc (x: T, y:V):U, arg: V):U =
  if ?left:
    let r = left.right(arg)
    if ?r: r else: default(typeof(left.right)) 
  else: default(typeof(left.right))

But when I enable these kind of templates then I get compile errors from your chained access tests ie..

/Users/mat/work/elvis/tests.nim(86, 35) Error: undeclared identifier: 'data'
candidates (edit distance, scope distance); see '--spellSuggest':
 (1, 6): 'Data' [type declared in /Users/mat/work/elvis/tests.nim(22, 3)]

Assuming this is possible the next question would be how to handle multiple arguments, and chaining

@geekrelief
Copy link
Contributor

I'm struggling to understand the error message from playing around with a small sample: https://play.nim-lang.org/#ix=3Md8 here an identifier is expected:

Error: in expression 'a`gensym4.bar(1)': identifier expected, but found 'bar(1)' It looks like bar(1) is being included into the identifier, but it's a call. I've tried asking for help on the forum, but I'm mulling this over as well.

@geekrelief
Copy link
Contributor

geekrelief commented Jan 15, 2022

I see what's going wrong. The AST produced is using the wrong identifier for the call. Instead of using the last identifier of the conditional accessor, the first argument of the call node has to be the DotExpr created from the prior identifiers.

In otherwords if we have: d?.foo(1) The current AST looks like:

DotExpr
  Ident "d"
  Call
    Ident "foo"
    IntLit 1

but it should be:

Call
  DotExpr
    Ident "d"
    Ident "foo"
  IntLit 1

So a macro is required for that transformation.

@mattaylor
Copy link
Owner Author

mattaylor commented Jan 15, 2022

Ive just pushed an update that provides an alternative workaround for this by introducing a 'default coalesce' operator ??
which returns the default of the right operand when it evaluates as falsey.
The following all work..

let opt0 = none(string)
let opt1 = some("one")
assert(??opt0.get.len == 0)
assert(??opt1.get.len == 3)
assert(??opt0.get("none").len == 4 

I would like to prefer to implement this as just a ? symbol that operates on a left operand, but im not sure if this is possible (or even a good idea) eg...

assert(opt0.get.len? == 0)
assert(opt1.get.len? == 3)
assert(opt0.get("none").len? == 4 

@geekrelief
Copy link
Contributor

Nim doesn't have postfix operators. My preference is for the prefix anyways. Nice work!

@mattaylor
Copy link
Owner Author

Not sure about the operator syntax '??' feels clunky and overloaded. What about :? (like the Elvis coleasce but reversed) eg..
assert(:?opt1.get.len == 3)
Alternatively...
assert(?!opt1.get.len == 3)
assert(?&opt1.get.len == 3)

@geekrelief
Copy link
Contributor

With the new ?? is there really a reason to keep ?. or .? around? If you're ok with removing those we could just use ?. as a prefix operator. Otherwise, I'm fine with ??. It's probably going to be the operator I use the most.

@mattaylor
Copy link
Owner Author

I renamed ?? to ?. which I think reads well. I have also updated tests and docs and removed the original conditional access template but I have kept the new conditional access syntax .? for now. As some may find it useful.

assert(?.opt0.get.len == 0)
assert(?.opt1.get.len == 3)
assert(?.opt0.get("none").len == 4 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants