Comments (4)
We could potentially consider another node for this, but that's what the attribute_write
flag on call nodes is supposed to indicate. a[0] = 1
will have the attribute_write
flag and a.[]=(0, 1)
will not. Similarly, foo.bar = 1
will have the attribute_write
flag. You can see how we use it in the CRuby compiler (https://github.com/ruby/ruby/blob/371790165f65d195a1dd6bd615dc6fd42eed8f94/prism_compile.c#L3477).
Would that flag work for you, or do you think we should try a new node?
from prism.
@kddnewton Thanks for the reply (and thank you for your all-time effort!)
Would that flag work for you,
Yes, technically, I can compile it correctly by looking at the flags.
My personal opinion about this case: a[idx] = object
is that it feels a bit strange that the BIG structure of the AST (if I can say so) doesn't express that this is an assignment expression.
a[] = obj
and a.[]=(obj)
are fundamentally different in terms of grammar.
For me, a tricky part is knowing whether I need to look at the flags carefully all the time.
For example, I usually don't need to worry about whether the flags for an integer literal (PM_INTEGER_NODE) ββare decimal or hexadecimal. On the other hand, I must not overlook the PM_REGULAR_EXPRESSION_NODE's flag.
But these two examples appear both natural because there is no fundamental grammatical difference while the flags express only the details in the same grammar.
Well, I don't have a strong thought. It might be a good idea to hear many people's opinions on this. How about making it an agenda for the Ruby core team?
from prism.
Making the return value the same as the RHS is specifically what attribute_write
is for, and since it affects multiple nodes I think it makes a lot of sense as a flag.
IMO we should keep this as-is.
Regarding whether to care or not about flags I would think for a compiler one should consider all flags on a node, most are necessary or useful for compilation, a few can be ignored. Maybe we could document flags which are expected to have no effect on compilation, OTOH it seems few of them and relatively clear already.
from prism.
@hasumikin β I can definitely see your point. We do have quite a few different node types, so I can see why you would want another one here.
My reasoning for putting it into one node: one of the biggest complaints with the existing CRuby AST was that there were too many different call node types. (QCall, VCall, FCall, Command, CommandCall, Call, etc.) It made it very difficult for consumers of the AST on the static analysis side to know that they had covered all call sites. While we have already deviated from that a bit (because of control-flow calls which could represent many method calls and target calls which have different field sets) we've mostly tried to isolate all of the method calls into one node to make it easier on those consumers.
The other piece of this is that while making this into a new node will solve your problem for a[0] = 1
, it will not solve your problem for a.b = 1
(you will still have to check the flag to get this right). So in this case adding a new node will cause everyone to have to handle it differently, but not fully solve the original problem of having to check the flags.
For these reasons, I'd like to keep it in the same node for now. But thank you very much for the thoughtful issue!
from prism.
Related Issues (20)
- Lex difference for self assignment HOT 2
- Prism rejects valid (?) syntax that CRuby accepts: `break while false` HOT 2
- Lex difference for heredoc with zero indentation and line continuation
- Heredoc flagged with `Layout/HeredocIndentation` creates infinite loop in rubocop HOT 2
- Warning/error messages mismatching HOT 6
- Installation in devcontainer fails on Apple Silicon HOT 2
- Handle modifications for interpolated string into regexp literal
- Investigate SyntaxError#path changed errors
- ConstantReadNode in ClassNode#constant_path or ModuleNode#constant_path is not just a constant read HOT 1
- Better scope prism's C API
- Gitpull
- Running zeitwerk unit tests raise `NameError: uninitialized constant` HOT 1
- Prism accepts double wildcard with no required fields in pattern match HOT 2
- `Prism::Translation::RubyParser` - Match operator HOT 3
- `Prism::Translation::RubyParser` - Interpolated adjacent strings
- CRuby test failures
- `eval` handles non-standard shebang differently
- `Parser::Translator` is accepting certain regexp flags where `parser` would raise HOT 1
- Allow generating ctags via prism
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from prism.