Custom Parser and Reader Macros

[allison-casey]

closes #722

Open question if hy should support such a permissive reader macros. This PR has it so you can define a reader for any single char or any sym prefixed by a #.

In my opinion reader macros prefised by # are more idiomatic and restricting user defined macros to only ones prefixed by # would make Hy more predictable and tooling easier down the road. I'd be in favor of that kind of restriction, but i'll leave that as an open question for discussion.

There's also the behavior of reader macros on require. Right now, readers macros need to be required per module and then enabled per module. This is how Racket works and to me seems like the right thing to do for a macro that drastically change how code looks and is written. Again another question for discussion though.

@scauligi as this was your baby, do you have any thoughts?

[Kodiologist]

Ho nelly! It's happening! Thanks for all the work you two put into this.

I agree that we don't need single-character reader macros in addition to #-prefixed reader macros. However, I'm not sure # is the best choice of prefix because we already use that for tag macros. (This is not a coincidence. Tag macros were originally called "reader macros", although they've always had the semantics of regular macros, hence the rename.) Perhaps we should use a different character. Alternatively, we could get rid of tag macros as a concept, so to get the effect of what is currently a tag macro, the programmer would write a reader macro that immediately falls back on the standard reader to do the reading part.

There's also the behavior of reader macros on require. Right now, readers macros need to be required per module and then enabled per module.

I think that's redundant because the syntax for defining or requiring reader macros already makes it clear that they're reader macros. When you see (require mymodule :readers ["!"]) or (defreader "!" …), you know that ! is going to be used as a reader macro, so why have the extra step?

Unlike requiring regular macros, reader macros must be specified as strings, cannot be renamed with :as, and are not made available under their absolute paths to their source module

Is that for implementation reasons or theoretical reasons?

I don't blame you for wanting to rename parse_one_thing, which is probably only named that because of an annoying song I had stuck in my head at the time, but maybe "parse_one_form" makes more sense than "parse_one_node".

[allison-casey]

I wonder if we can make tag macros just a syntactic sugar for a reader macro that parses one thing and then passes that thing to the underlying macro function? Reader macros are a bit unwieldy by their nature and a sugared form could continue to be useful if you don't need to take advantage of the rest of the reader's functionality.

I think that's redundant because the syntax for defining or requiring reader macros already makes it clear that they're reader macros

That's valid. Let me test enabling readers at require to and see how it feels.

Is that for implementation reasons or theoretical reasons?

This is currently an implementation thing, but it think it could be done to make :as and absolute paths work. Though, those changes would be non breaking and i think can wait for a future PR so as not to make this take any longer than it has.

but maybe "parse_one_form" makes more sense than "parse_one_node".

Makes sense enough. Let me double check though that there isn't some other reader related stuff that uses node as the idiom

[Kodiologist]

I wonder if we can make tag macros just a syntactic sugar for a reader macro that parses one thing and then passes that thing to the underlying macro function?

Probably. One can imagine a macro-writing macro in Hyrule like (deftag my-tag-macro [arg] (f arg)) that defines my-tag-macro as a reader macro that calls the reader, binds the result to arg, and then evaluates (f arg) like a normal macro.

This is currently an implementation thing, but it think it could be done to make :as and absolute paths work. Though, those changes would be non breaking and i think can wait for a future PR so as not to make this take any longer than it has.

Okay.

[allison-casey]

deftag as a sugared reader macro works nicely. Just need to go through and replace old defmacro tag macros with reader macros and yoink that functionality out of defmacro. Otherwise, i've restricted user readers to # prefixed and enabled them by default in the module they're declared and on require.

[Kodiologist]

Neat.

[allison-casey]

• deftag is a sugar for simple read time macros
• defmacro no longer supports tag macro semantics
• defreader macros are always prefixed with a #
• reader macros are enabled immediately after they're defined and on require

did i miss anything?

[Kodiologist]

That sounds like everything we've discussed. I hope to take another look at the code soon.

[scauligi]

This is shaping up super well, once again many thanks for taking this on allie!

Here are some things I came across while taking this for a test run:

• What is the expected interface for deftag/defreader? Ie for (defreader THING ...) is THING supposed to be a (well-formed) symbol, a string, or any sequence of characters?

  • If we want to support the latter case (eg (defreader : ...) instead of (defreader ":" ...) then defreader itself will likely need to be a reader macro

• Along those lines, bad error message for using the wrong type for the reader name, eg (defreader : '5) produces TypeError: can only concatenate str (not "Keyword") to str (similar for other types, like Integer if you try to define (defreader 4 '5) instead of (defreader "4" '5))

• (defreader name [expr] ...) looks like it's specifying args, but this actually just a list as the first body expression. This could just be a #docmebro? or alternatively we can explicitly require an empty args list (eg (defreader name [] ...)) to make this explicit. I'm leaning towards the empty args list to make it consistent with all the other def... forms.

• requiring a reader macro ((require module :readers ["#xyz"])) is inconsistent with defining a reader macro, in that the leading "#" is required, which feels a bit strange

[Kodiologist]

It looks there is no equivalent of the old hy.read, and the new function of that name actually works like hy.read-str. Is that a mistake?

assert can_compile(r'f"hello {"n"} world"')

Are you sure this is correct? It seems counterintuitive to me, and it's not consistent with Python.

I think there's a mistake in test-tag-macros. You're testing (deftag [a b] '1) and (deftag [] '1) when you mean (deftag foo [a b] '1) and (deftag foo [] '1).

Some tests seem to have been lost, such as test_bad_tag_macros for https://github.com/hylang/hy/issues/1965 (or if you did make a new version of that test, it's missing the issue link).

[Kodiologist]

What is the expected interface for deftag/defreader? Ie for (defreader THING ...) is THING supposed to be a (well-formed) symbol, a string, or any sequence of characters?

Perhaps the intention was to be consistent with defmacro, which allows a symbol or a string. But the only reason I let defmacro accept a string literal as the macro name is to allow you to define tag macros with defmacro. You can no longer do that, so defmacro should (eventually, not necessarily in this PR) go back to requiring a symbol.

requiring a reader macro ((require module :readers ["#xyz"])) is inconsistent with defining a reader macro, in that the leading "#" is required, which feels a bit strange

I agree. We should either always forbid the # when mentioning (as opposed to calling) a reader macro or always require it. Arguably, forbidding it makes more sense, because it's redundant, and then we can use symbols instead of strings.

(defreader name [expr] ...) looks like it's specifying args, but this actually just a list as the first body expression. This could just be a #docmebro? or alternatively we can explicitly require an empty args list (eg (defreader name [] ...)) to make this explicit. I'm leaning towards the empty args list to make it consistent with all the other def... forms.

I would recommend against requiring [] because it doesn't mean anything. Putting in [] mistakenly is harmless because it's a no-op, and putting in [a] or something will crash with an obvious error, so nobody's going to be very inconvenienced by the lack of a requirement for a meaningless parameter list.

[scauligi]

assert can_compile(r'f"hello {"n"} world"')
Are you sure this is correct? It seems counterintuitive to me, and it's not consistent with Python.

My vote is to keep this; it's a lot easier to use format strings (and feels lispier) when curly braces work like unquote rather than being "just" a part of the string.

Putting in [] mistakenly is harmless because it's a no-op

Fair point!

[Kodiologist]

Oh right, I forgot to mention: deftag and #@ can be moved to Hyrule, since they just use the built-in reader-macro features now.

[allison-casey]

• user defined reader macros only allow symbols now
• require :readers uses bare symbols and not # prefixed strings
• added back the missing test_bad_tag_macros test

this does mean that #: slice tag macro is no longer possible (at least from the user perspective). there's nothing stopping us from adding it as a built in reader. Which i would be for anyways given how extensive slicing is in python and its libraries.

I'll open a followup PR with moving #@ to hyrule after this so tests will pass in hyrule

@scauligi can you speak to the read vs read-str point kodi brought up?

I think that's everything?

[Kodiologist]

If you're going ahead with f"hello {"n"} world", don't forget a NEWS line for it, since it's a breaking change.

this does mean that #: slice tag macro is no longer possible (at least from the user perspective).

No reason we couldn't just call it something else, right? We should have plenty of decent options without having to give it special accommodation in the reader. Perhaps #s, for "slice".

I'll open a followup PR with moving #@ to hyrule after this so tests will pass in hyrule

It looks like Hyrule is already broken by this PR. Not your fault; that's what usually happens with breaking changes to Hy. My recommendation is, remove deftag from and delete #@ in this PR, and then we can add them to Hyrule in the same PR that unbreaks it.

[scauligi]

It looks there is no equivalent of the old hy.read, and the new function of that name actually works like hy.read-str. Is that a mistake?

That was intentional; the API is different since the new reader can directly take a stream where (iirc) the old reader needed the entire input at once as a string; so there's no need to have a separate read and read-str anymore.

this does mean that #: slice tag macro is no longer possible

Hmm, I am leaning slightly towards adding #: as an additional baked-in reader; it should actually be very easy in the new reader. It's also one of the few non-user-definable constructs because of the : but also a very good mnemonic for what it does.

[Kodiologist]

the API is different since the new reader can directly take a stream

If that was intended, it doesn't seem to work:

=> (with [o (open "/tmp/test.hy")] (hy.read o))
Traceback (most recent call last):
  File "stdin-1219427163456d350c0c7b476cc55454c5eff66a", line 1, in <module>
    (with [o (open "/tmp/test.hy")] (hy.read o))
TypeError: initial_value must be str or None, not _io.TextIOWrapper

[scauligi]

Ah, looks like I mixed myself up; internally it uses a stream but the reader interface indeed still expects the whole string up front.

I expect changing the function names shouldn't affect users too much, but do you want to keep the original names?

[allison-casey]

think everything looks good. only thing i'm noticing now is that i didn't let :readers require all via (require some.module :readers *) which was a mistake on my part. I can try and add that real quick since we kinda need that for (require hyrule :macros * :readers *)

[Kodiologist]

I see, that's a good point.

[Kodiologist]

Actually, since this PR is already so big, and we probably have more such things we need to do, would you like to do it as a follow-up PR?

[allison-casey]

works for me

[allison-casey]

i'll rebase the tuple syntax pr and remove the whitespace stuff once we merge

[Kodiologist]

Very good. I shall rebase and push the merge button.