Support dot-like method calls #19384

markspanbroek · 2022-01-13T12:40:17Z

Add support for calls using a custom operator that starts with a dot, such as a.?b(c)

This implements a small part of RFC341, proposal 2.

Araq · 2022-01-13T13:53:10Z

We need to enable -d:nimPreviewDotLikeOps for all important packages to see what it breaks before we add more tweaks to it.

planetis-m · 2022-01-13T18:19:38Z

What if it expanded to (Call (Prefix (Ident ".?") (Ident 'a")) (Ident "b") (Ident "c")) instead of: (Infix (Ident ".?()") (Ident "a") (Ident "b") (Ident "c")) ?Is it not possible?

Or even:
(Call (DotExpr (Prefix (Ident "?") (Ident 'a")) (Ident "b") (Ident "c")))

Vindaar · 2022-01-13T18:37:42Z

What if it expanded to (Call (Prefix (Ident ".?") (Ident 'a")) (Ident "b") (Ident "c")) instead of: (Infix (Ident ".?()") (Ident "a") (Ident "b") (Ident "c")) ?Is it not possible?

Or even: (Call (DotExpr (Prefix (Ident "?") (Ident 'a")) (Ident "b") (Ident "c")))

Yeah, I agree. So far (unless I'm missing something) an infix node always has exactly two children. That's nice to deal with in macros. Turning infix into something that might have N children would be a bit annoying and break more macro logic than necessary.

planetis-m · 2022-01-13T19:02:09Z

Ok this is more inline with reality. (Call (AccQuoted (Ident ".?()")) (Ident "a") (Ident "b") (Ident "c") )

markspanbroek · 2022-01-14T08:48:13Z

So far (unless I'm missing something) an infix node always has exactly two children

I've been told that this isn't strictly true, but it certainly is unexpected. Let's go through the alternatives that have been mentioned:

(Call (Prefix (Ident ".?") (Ident 'a")) (Ident "b") (Ident "c")) To me it looks a bit weird to have Prefix in there, when it is really an infix operator.
(Call (DotExpr (Prefix (Ident "?") (Ident 'a")) (Ident "b") (Ident "c"))) Again Prefix looks a bit weird, and separating out the dot from the rest of the operator is not in line with how dot-like operators are handled now.
(Call (AccQuoted (Ident ".?()")) (Ident "a") (Ident "b") (Ident "c") ) I like this one, but I would leave out the AccQuoted, because there are no quotes in a.?b(c)

Vindaar · 2022-01-14T13:09:16Z

So far (unless I'm missing something) an infix node always has exactly two children

I've been told that this isn't strictly true, but it certainly is unexpected. Let's go through the alternatives that have been mentioned:

Oh, wow. I had no idea. Seems to me like that example should ideally also be some different AST, but I suppose it's sensible.

(Call (Prefix (Ident ".?") (Ident 'a")) (Ident "b") (Ident "c")) To me it looks a bit weird to have Prefix in there, when it is really an infix operator.

(Call (DotExpr (Prefix (Ident "?") (Ident 'a")) (Ident "b") (Ident "c"))) Again Prefix looks a bit weird, and separating out the dot from the rest of the operator is not in line with how dot-like operators are handled now.

(Call (AccQuoted (Ident ".?()")) (Ident "a") (Ident "b") (Ident "c") ) I like this one, but I would leave out the AccQuoted, because there are no quotes in a.?b(c)

Personally, I'd vote for 3 (with a preference excluding nnkAccQuoted, for the reason you state). It is a call of .? after all. One can write

`.?`(a, b, c)

after all (or I'd expect to be able to).

I dislike 1, because it's not really a "prefix". If anything ? is a postfix, but... And something like +. doesn't change into some prefix / postfix AST either.

I dislike 2, because it's not really a dot expression either. It may look like it, but that makes it seem like .? has something to do with ., which isn't really the case.

markspanbroek · 2022-01-17T09:55:08Z

Updated to produce a Call AST instead of an Infix AST.

Menduist · 2022-04-25T06:09:04Z

We need to enable -d:nimPreviewDotLikeOps for all important packages to see what it breaks before we add more tweaks to it.

AFAICT this has been enabled in #19598

Araq · 2022-04-25T20:23:33Z

Correct.

markspanbroek · 2022-06-14T14:57:10Z

Can this be merged?

Araq · 2022-06-15T07:15:21Z

Looks like a hack to me.

Araq · 2022-06-15T07:16:40Z

compiler/parser.nim

@@ -480,6 +483,16 @@ proc dotLikeExpr(p: var Parser, a: PNode): PNode =
  result.add(opNode)
  result.add(a)
  result.add(parseSymbol(p, smAfterDot))
+  if p.tok.tokType == tkParLe and p.tok.strongSpaceA <= 0:
+    var call = newNodeI(nkCall, info)
+    let operator = opNode.ident.s & "()"


Looks rather hacky. What is it that we're trying to achieve here?

It's parsing a.?b(c) as `.?()`(a, b, c), so that it can be matched by a template or macro.

Araq · 2022-06-15T07:24:42Z

So what is a.?b(c) ? Is it `.?()`(a, b, c)? If so, that sounds more dangerous than useful to me. Readability is harmed by too many rewrite rules.

markspanbroek · 2022-06-15T07:42:31Z

Yes, this PR parses a.?b(c) as `.?()`(a, b, c).

Without this PR, a.?b(c) parses as (Call (Infix ".?" "a" "b") "c"). As far as I know, this cannot be matched by a template or macro. Which is problematic for what I'm trying to achieve.

What would you recommend as a good AST for a.?b(c) that allows it to be matched by a template or macro?

Araq · 2022-06-15T18:52:09Z

But a.f(b, c) is turned into f(a, b, c) so this new rewrite rule is not natural.

I don't know what you need to accomplish but +?[a, b, c] where +? is a macro can deal with a variable number of arguments. There are plenty of other solutions too, of course.

markspanbroek · 2022-06-16T12:48:33Z

But a.f(b, c) is turned into f(a, b, c) so this new rewrite rule is not natural.

Thanks, I see your point. I would love to have something closer to f(a,b,c). I tried a bunch of AST shapes, but the only two shapes that I could make work are:

`.?`(a, f(b,c)) <-- this is how it was parsed before dot-like syntax was introduced
`.?()`(a, f, b, c) <-- this is what the code in this PR currently does

I could rewrite the code in this PR to return the first form, which has the added benefit that it's backward compatible with any code that was written before dot-like syntax was introduced. But it also doesn't feel very natural to me.

I don't know what you need to accomplish

I wrote a library called questionable that is heavily inspired by how Swift handles optionals. One example of its use is:

import std/sequtils[
import pkg/questionable

let a: ?seq[int] = @[41, 42].some
let b: ?seq[int] = seq[int].none

let x = a.?distribute(2).?len() # equals 2.some
let y = b.?distribute(2).?len() # equals int.none

As you can see, the .? operator allows you to chain function calls on an optional. But it broke when the new dot-like syntax was introduced. When I enable it, I can no longer handle a.?f(b,c) in a template or macro.

I could switch over the library to e.g. the ?. operator, which is not impacted by the dot-like syntax change, but that has one major disadvantage. The operator precedence of ?. is significantly different from .. This means that users of the library need to add extra parenthesis in an expression when they use ?. instead of ., which is rather surprising. It seems that the dot-like syntax was added to Nim for exactly this use case, but ironically it broke it instead. With this PR I want to fix that so that we can have the new dot-like syntax parsing for a.?f(b,c) expressions.

Araq · 2022-06-17T04:46:15Z

Maybe we should undo -d:nimPreviewDotLikeOps instead if it doesn't deliver what it promised.

markspanbroek · 2022-06-17T11:53:16Z

That would solve my problem, yes.
I do think that having good support for dot-like operators including a.?f(b,c) would greatly simplify the job of writing templates and macros for dot-like operators. But as it currently stands, -d:nimPreviewDotLikeOps is more of a hindrance than a help for this use case.

ringabout · 2022-06-22T11:14:16Z

I hope the -d:nimPreviewDotLikeOps option can stay(not deprecated or removed). It is useful for "enabling dynamic fields without affecting the built-in . operator, e.g. for std/jsffi, std/json, pkg/nimpy". I use it in my frontend framework to get the attributes of the Reactive objects.

It works well for my usage.

template `.?`*[T: ref](x: Reactive[T], y: untyped{ident}): untyped =
  cast[T](x.value).y


type
  Card = ref object
    id: int
  Counter = ref object
    num: int
    card: Card

y := Counter(card: Card(id: 16))
watch:
  console.log "card: ", y.?card.id

y.?card.id += 1

Without -d:nimPreviewDotLikeOps, it breaks

Error: undeclared field: 'id'

Menduist · 2022-06-22T11:43:09Z

Please don't add flags that change the way programs compile, it's a nightmare for every library developer..

ringabout · 2022-06-22T11:46:07Z

.?(a, f(b,c)) <-- this is how it was parsed before dot-like syntax was introduced

I could rewrite the code in this PR to return the first form, which has the added benefit that it's backward compatible with any code that was written before dot-like syntax was introduced. But it also doesn't feel very natural to me.

I agree. We can parse it like before.

arnetheduck · 2022-06-22T12:14:05Z

Please don't add flags that change the way programs compile, it's a nightmare for every library developer..

the idea with the preview flags is that library developers can enable them to see if the feature being enabled is both useful and doesn't break other useful stuff - when a feature has "proven it's worth" it can be enabled more broadly.

It's important that potentially breaking features gain wider reach and distribution than sitting unnoticed in devel and this is one mechanism - likewise, if a feature gets added but nobody but its original author is interested in turning it on after 6 months because the utility is low, it should likely be reverted as a feature.

there should probably be a -d:preview flag as well which enables all preview features.

Araq · 2022-06-22T12:17:05Z

A new feature was added behind a preview flag, we enabled it on devel in order to detect problems, we detected problems, now we can undo or refine the feature. The process works.

Menduist · 2022-06-22T12:20:29Z

Should have been more precise: I don't have anything against preview flags, as mentioned they are useful.
However, if we close this PR and merge #19919, it's no longer a preview flag, it becomes a feature flag that will dangle around for who knows how long, and make compatibility harder

So please, fix it or revert it, don't keep it as a alternate flag forever

ringabout · 2022-06-22T12:23:30Z

So please, fix it or revert it, don't keep it as a alternate flag forever

#19919 has already clarified that - nimPreviewDotLikeOps is going to be removed or deprecated in changelog.

markspanbroek · 2022-06-22T12:45:37Z

y.?card.id += 1

Without -d:nimPreviewDotLikeOps, it breaks

You can still handle these expressions without -d:nimPreviewDotLikeOps, but it is less convenient. For instance, I handle expressions of the shape a.?b.c in the questionable library here.

Support dotlike calls a.?b(c)

1ab7623

ringabout mentioned this pull request Jan 16, 2022

A macro with -d:nimPreviewDotLikeOps enabled gets different arguments #19392

Open

AST for dot-like calls is a Call instead of an Infix

9740fd7

Merge branch 'devel' into dotlike-call

4740744

Araq reviewed Jun 15, 2022

View reviewed changes

ringabout mentioned this pull request Jun 22, 2022

Revert "enable nimPreviewDotLikeOps" #19919

Merged

ringabout added a commit that referenced this pull request Jun 22, 2022

Support dot-like method calls; alternative to #19384 #19919

e6c96b6

ringabout mentioned this pull request Jun 22, 2022

Support dot-like method calls; alternative to #19384 #19919; fixes #19392 #19921

Closed

Menduist mentioned this pull request Aug 3, 2022

Questionable doesn't work on Nim devel codex-storage/questionable#17

Closed

ringabout closed this in #19919 Aug 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support dot-like method calls #19384

Support dot-like method calls #19384

markspanbroek commented Jan 13, 2022

Araq commented Jan 13, 2022

planetis-m commented Jan 13, 2022 •

edited

Loading

Vindaar commented Jan 13, 2022

planetis-m commented Jan 13, 2022 •

edited

Loading

markspanbroek commented Jan 14, 2022

Vindaar commented Jan 14, 2022

markspanbroek commented Jan 17, 2022

Menduist commented Apr 25, 2022

Araq commented Apr 25, 2022

markspanbroek commented Jun 14, 2022

Araq commented Jun 15, 2022

Araq Jun 15, 2022

markspanbroek Jun 15, 2022

Araq commented Jun 15, 2022

markspanbroek commented Jun 15, 2022

Araq commented Jun 15, 2022

markspanbroek commented Jun 16, 2022

Araq commented Jun 17, 2022

markspanbroek commented Jun 17, 2022

ringabout commented Jun 22, 2022 •

edited

Loading

Menduist commented Jun 22, 2022

ringabout commented Jun 22, 2022

arnetheduck commented Jun 22, 2022

Araq commented Jun 22, 2022

Menduist commented Jun 22, 2022

ringabout commented Jun 22, 2022 •

edited

Loading

markspanbroek commented Jun 22, 2022

Support dot-like method calls #19384

Support dot-like method calls #19384

Conversation

markspanbroek commented Jan 13, 2022

Araq commented Jan 13, 2022

planetis-m commented Jan 13, 2022 • edited Loading

Vindaar commented Jan 13, 2022

planetis-m commented Jan 13, 2022 • edited Loading

markspanbroek commented Jan 14, 2022

Vindaar commented Jan 14, 2022

markspanbroek commented Jan 17, 2022

Menduist commented Apr 25, 2022

Araq commented Apr 25, 2022

markspanbroek commented Jun 14, 2022

Araq commented Jun 15, 2022

Araq Jun 15, 2022

Choose a reason for hiding this comment

markspanbroek Jun 15, 2022

Choose a reason for hiding this comment

Araq commented Jun 15, 2022

markspanbroek commented Jun 15, 2022

Araq commented Jun 15, 2022

markspanbroek commented Jun 16, 2022

Araq commented Jun 17, 2022

markspanbroek commented Jun 17, 2022

ringabout commented Jun 22, 2022 • edited Loading

Menduist commented Jun 22, 2022

ringabout commented Jun 22, 2022

arnetheduck commented Jun 22, 2022

Araq commented Jun 22, 2022

Menduist commented Jun 22, 2022

ringabout commented Jun 22, 2022 • edited Loading

markspanbroek commented Jun 22, 2022

planetis-m commented Jan 13, 2022 •

edited

Loading

planetis-m commented Jan 13, 2022 •

edited

Loading

ringabout commented Jun 22, 2022 •

edited

Loading

ringabout commented Jun 22, 2022 •

edited

Loading