dmd
dmd copied to clipboard
Fix Issue 15749 - Implement with expressions
I've definitely missed some things but this is enough to be reviewed (i.e. non-draft).
I haven't decided what to do with the union
usage yet. Hypothetically one could use the same symbol table thing the WithStatements use and avoid any such thing entirely, but this seemed very ugly i.e. dmd already conflates a bunch of things behind it's back as is so making Scope
granular on the level of expressions is a bit ridiculous in my mind.
Thanks for your pull request, @maxhaton!
Bugzilla references
Auto-close | Bugzilla | Severity | Description |
---|---|---|---|
✓ | 15749 | enhancement | allow `with` on an expression |
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + dmd#13776"
I assume I've broken one or part of the visitors
I assume I've broken one or part of the visitors
The vtable layout has changed, so yes.
ccing @pbackus @CyberShadow since they have proposed library implementations for bind.
with
is already controversial because of the hiding issue, I think anything that extends it would need a larger discussion (i.e. a DIP).
The main difference between this and bind
(or a with
statement wrapped in a lambda) is that a with
expression does not introduce a new scope.
I've wanted this for a long time, but I think this should go through a DIP proposal first.
@iK4tsu Why isn't the library solution a good alternative?
I didn't know about the library proposals. The bind
implementation looks really good actually. One thing it does not provide is access to members through the type.
struct Foo { ... }
...
with(Foo) ...
However, this is really nitpicking and I don't have right now a concrete example where it could be crucially useful.
The other thing is that it kinda completes the with
feature we have right now. It just feels weird and unintuitive that we can't use with
with expressions, and it gives an incomplete feature kind of vibe to me. This is not really a strong point to support the change, just an opinion. Someone new to the language finds about with
, but then sees that it is really restrictive and its use cases are too few to even bother using it, then he learns that the useful stuff that should be done with with
is actually a library implementation. This is not a rant btw. I just think that the library implementation would look a bit "out of place" when we have with
and with
would look even more useless.
Having said that, I would totally use the library solution if that's the decision you're heading for :smile:
@RazvanN7 as mentioned above you can implement a terrible approximation to this as a library, yes, but it doesn't actually generalize with
to expressions so the comparison is pointless.
I agree with extending with
to an expression. A library solution would be a workaround that is slower and can lead to more unclean code. If we have with
, why not just use it?
@maxhaton you also forgot to add the new Expression node to astbase.d
.
@ljmf00 removing the union requires making my visitor much more complicated as I now have to walk everything myself versus just hooking where the identifiers appear
@ljmf00 removing the union requires making my visitor much more complicated as I now have to walk everything myself versus just hooking where the identifiers appear
I understand that, but I don't see the beneficial usage of a union in public fields on such a high-level interface, either in terms of performance or memory usage. AFAIU, I would say it is just harder to implement but way safer if someone decides to use this field in the future. I don't have any suggestions other than what you suggested above, but if anyone comes up with an idea of making it safer, would be cool.
@ljmf00 removing the union requires making my visitor much more complicated as I now have to walk everything myself versus just hooking where the identifiers appear
Does it really though? Outside of unary and binary nodes, what else is there to have a visitor for?
@ljmf00 removing the union requires making my visitor much more complicated as I now have to walk everything myself versus just hooking where the identifiers appear
Does it really though? Outside of unary and binary nodes, what else is there to have a visitor for?
Just because something inherits from a UnaExp doesn't mean it only has one member, e.g. to walk things properly you need to handle all the annoying stuff.
I can make the union safe I just haven't worked out the best way of doing it yet. A property would probably do the job.
Does it really though? Outside of unary and binary nodes, what else is there to have a visitor for?
Just because something inherits from a UnaExp doesn't mean it only has one member, e.g. to walk things properly you need to handle all the annoying stuff.
There are examples of rewrite visitors in the front-end so you're not going at it alone. Secondly, WithStatements manage just fine without this union.
Does it really though? Outside of unary and binary nodes, what else is there to have a visitor for?
Just because something inherits from a UnaExp doesn't mean it only has one member, e.g. to walk things properly you need to handle all the annoying stuff.
There are examples of rewrite visitors in the front-end so you're not going at it alone. Secondly, WithStatements manage just fine without this union.
Take a look at how with statements are implemented... It uses a completely different system: the symbol table is effectively augmented to search the with-ed symbol first. This does not scale to expression very well if at all because it relies on the Scope structure which hypothetically could be bludgeoned into being granular at the level of an expression but would be extremely ugly and probably would break nested expressions etc.
So, is there any future for this?