Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Does not handle metadata that is an evaling_lit #50

Closed
sogaiu opened this issue Mar 14, 2023 · 3 comments
Closed

Does not handle metadata that is an evaling_lit #50

sogaiu opened this issue Mar 14, 2023 · 3 comments

Comments

@sogaiu
Copy link
Owner

sogaiu commented Mar 14, 2023

Came across this code today:

(defn get-bucket
  [^Storage storage ^String bucket-name]
  (let [^#=(array-type Storage$BucketGetOption) no-options (into-array Storage$BucketGetOption [])]
    (.get storage bucket-name no-options)))

Specifically this bit was not so familiar:

^#=(array-type Storage$BucketGetOption) no-options

When this sort of thing is handed to clojure at the command line, one gets:

user=> ^#=(keyword "a") []
[]

To make things a bit more apparent:

user=> (binding [*print-meta* true] (prn (read-string "^#=(keyword \"a\") []")))
^{:a true} []

So I think that means that the #=(...) portion is supposed to be understood as what the grammar refers to as evaling_lit [1].

The upcoming grammar doesn't handle this properly:

$ echo "^#=(keyword \"a\") []" | tree-sitter.ahlinc parse -
(source [0, 0] - [1, 0]
  (vec_lit [0, 0] - [0, 19]
    meta: (meta_lit [0, 0] - [0, 16]
      value: (tagged_or_ctor_lit [0, 1] - [0, 16]
        tag: (sym_lit [0, 2] - [0, 3]
          name: (sym_name [0, 2] - [0, 3]))
        value: (list_lit [0, 3] - [0, 16]
          value: (sym_lit [0, 4] - [0, 11]
            name: (sym_name [0, 4] - [0, 11]))
          value: (str_lit [0, 12] - [0, 15]))))))

Here the piece in question is being recognized as tagged_or_ctor_lit rather than evaling_lit.


[1] I think use of this is "discouraged" and I don't come across it much -- project.clj is where I'd seen it the most.

In Clojure's source, LispReader.java has some relevant bits here and here.

@sogaiu
Copy link
Owner Author

sogaiu commented Mar 14, 2023

This looks like it's in the same neighborhood as #46.

Looking back at #35 (comment), there is this alternate fix idea:

meta_lit: $ =>
seq(field('marker', "^"),
repeat($._gap),
field('value', choice($.read_cond_lit,
$.map_lit,
$.str_lit,
$.kwd_lit,
$.sym_lit))),

...

This brings up the topic of whether that choice should be there restricting things...possibly it could be replaced with $._form.

@sogaiu
Copy link
Owner Author

sogaiu commented Mar 14, 2023

So I tried the following:

    meta_lit: $ =>
    seq(field('marker', "^"),
        repeat($._gap),
        field('value', $._form)),

    old_meta_lit: $ =>
    seq(field('marker', "#^"),
        repeat($._gap),
        field('value', $._form)),

With that, the command line invocation from before yielded:

$ echo "^#=(keyword \"a\") []" | tree-sitter.ahlinc parse -
(source [0, 0] - [1, 0]
  (vec_lit [0, 0] - [0, 19]
    meta: (meta_lit [0, 0] - [0, 16]
      value: (evaling_lit [0, 1] - [0, 16]
        value: (list_lit [0, 3] - [0, 16]
          value: (sym_lit [0, 4] - [0, 11]
            name: (sym_name [0, 4] - [0, 11]))
          value: (str_lit [0, 12] - [0, 15]))))))

...which is what I think we want.

The Clojars testing results didn't change so I guess that may be a good sign.

The potential fix has been pushed as a commit to the pre-0.0.12 branch.

Added a test at 1435704.

@sogaiu sogaiu added the candidate-on-dev The dev branch contains code to address label Mar 14, 2023
@sogaiu
Copy link
Owner Author

sogaiu commented May 8, 2023

Addressed in aaf31ad.

@sogaiu sogaiu closed this as completed May 8, 2023
@sogaiu sogaiu removed the candidate-on-dev The dev branch contains code to address label May 8, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant