Simplify tag macros

[Kodiologist]

• Closes Exceptions in tag macros crash the compiler #1965.
• Closes Require fails to load tags with same name as macro in same module #1967.

I've reconstrued tag macros as macros whose names begin with "#", which you can call with special syntax. This simplifies a lot of internals and should remove various inconsistencies in the behavior of tag macros. To support this, defmacro and require allow macros to be named with string literals instead of symbols. deftag and #doc are unnecessary and have been removed.

I haven't updated the docs or NEWS yet. Tell me what you think.

[allison-casey]

oh heck the mad man went ahead and did it.

I'll take a look soon hopefully. this looks pretty promising though.
What's the reason we have to require them with "#atag" and not #atag? Parsing limitations?

[Kodiologist]

Right, the #foo in (require [#foo a]) parses as a tag-macro call, not a symbol.

[Kodiologist]

By the way, I first tried having a tag macro named #foo be called foo internally, as I'd originally proposed, but I realized that we want to use tag macros like #+ and #% without shadowing + and %.

[allison-casey]

tangentially affects #1970 need to test this

Right, the #foo in (require [#foo a]) parses as a tag-macro call, not a symbol.

makes sense and since it's only relevant in defmacro and require it doesn't seem so bad.

we want to use tag macros like #+ and #% without shadowing + and %.

agreed, i think I like this syntax better any ways

[scauligi]

Would it be worthwhile to add a deftag macro for backwards compatilibity / ease of use?

;; backwards compat
(defmacro deftag [name &rest forms]
  `(defmacro ~(+ "#" (str name)) ~@forms))

I'm also looking into if there's a way to hack up the parser so that we wouldn't need to string-quote tags for require.

[Kodiologist]

My philosophy is that backwards-compatibility features before the release of 1.0 are premature. I'm not sure that (deftag foo …) is better syntax than (defmacro "#foo" …) anyway, because the # isn't included.

I'm also looking into if there's a way to hack up the parser so that we wouldn't need to string-quote tags for require.

I wouldn't recommend it. It will complicate the lexer considerably for an edge case.

[scauligi]

Alternatively, we could single-quote a tag name a la '#%, so you can do e.g.:

(require [hy.extra.anaphoric ['#%]])
(defmacro '#x [x] x)

Here's a patch that would allow this for require, you'd have to do something similar for defmacro (and/or factor stuff out into model_patterns):

diff --git a/hy/compiler.py b/hy/compiler.py
index c7224c44..01ce10d5 100755
--- a/hy/compiler.py
+++ b/hy/compiler.py
@@ -1134,12 +1134,19 @@ class HyASTCompiler(object):
             expr, op=ops[root](), operand=operand.force_expr)
 
     def _importlike(*name_types):
-        name = some(lambda x: isinstance(x, name_types) and "." not in x)
+        _symn = some(lambda x: isinstance(x, HySymbol) and "." not in x)
+        _symt = some(lambda x: isinstance(x, HySymbol) and (x[0] == "#" or "." not in x))
+        _tname = _symn | pexpr(sym("quote") + _symt)
         return [many(
             SYM |
-            brackets(SYM, sym(":as"), name) |
+            brackets(SYM, sym(":as"), _symn) |
             brackets(SYM, brackets(many(
-                name + maybe(sym(":as") + name)))))]
+                _tname + maybe(sym(":as") + _tname)))))]
+
+    @staticmethod
+    def _unquote_required(kids):
+        for pair in kids:
+            yield map(lambda x: x[0] if isinstance(x, HyExpression) else x, pair)
 
     @special("import", _importlike(HySymbol))
     @special("require", _importlike(HySymbol, HyString))
@@ -1167,7 +1174,7 @@ class HyASTCompiler(object):
                         raise self._syntax_error(star,
                             "* in an import name list must be on its own")
                 else:
-                    assignments = [(k, v or k) for k, v in kids]
+                    assignments = [(k, v or k) for k, v in self._unquote_required(kids)]
 
             ast_module = ast_str(module, piecewise=True)
 
diff --git a/hy/lex/parser.py b/hy/lex/parser.py
index 6121125a..a2e74e47 100755
--- a/hy/lex/parser.py
+++ b/hy/lex/parser.py
@@ -17,7 +17,7 @@ from .lexer import lexer
 from .exceptions import LexException, PrematureEndOfInput
 
 
-pg = ParserGenerator([rule.name for rule in lexer.rules] + ['$end'])
+pg = ParserGenerator([rule.name for rule in lexer.rules] + ['$end'], [('nonassoc', ['HASHOTHER'])])
 
 
 def set_boundaries(fun):
@@ -112,6 +112,12 @@ def term_discard(state, p):
     return p[2]
 
 
+@pg.production("term : QUOTE HASHOTHER")
+@set_boundaries
+def term_quote_hash(state, p):
+    return HyExpression([HySymbol("quote"), HySymbol(p[1].value)])
+
+
 @pg.production("term : QUOTE term")
 @set_quote_boundaries
 def term_quote(state, p):

Note that using a straight quote will quote the tag name, but a quasiquote will quote a tag term. I think this can be considered a feature and not a bug.

[Kodiologist]

But then you wouldn't be able to use non-identifier characters in tag macros.

[scauligi]

I'm not sure I follow. How would one even use a tag macro with non-identifier characters?
The lexer already defines a tag name only as a "#" followed by identifier characters.

[Kodiologist]

So it does. Sorry. I was probably remembering the old days when tag macros had to be only 1 character long, so you could name them nearly anything, like (.

I'm still not entirely enthusiastic about using a quote form in place of a string literal, but I guess that's a matter of taste.

[allison-casey]

I'm still getting the __pycache__ error that scauligi brought up in #1970 just in a slightly different way, so it doesn't look like this will fix it outright. But it is a major simplification to the code base which is a big + plus will probably help us figure that one out too. With regard to quoting, @scauligi does the test suit run all the way through with your patch? String literal feels more explicit, but quoting feels more syntactically consistent to lisp so i'm not quite sure where i stand on that yet.

[scauligi]

I haven't run my patch through any of the tests; it's also not a complete patch, just a proof-of-concept since the require handler would also need to be updated and we would probably want to add a function to model_patterns.py.

I have a mild preference for the quoting since it feels more lisp-y to me (importing a specific symbol vs naming with a string) but I'm happy with either direction.

[allison-casey]

If it's going to be that big of a change to the core compiler code it may be worth going with string literals for now and making quoting it's own pr. That way we can focus on the removal of the concept of __tags__ for now any any unanticipated side effects that might have on it's own.

[Kodiologist]

Okay, the doc changes are in now.

[allison-casey]

I'm noticing that this also lets us define non identifier tag macros which leads to weird behavior. (deftag "#(" [a]) is valid but can't be called, (defamcro "#:" [a]) is actually valid and can be called now. Do we want to do some additional checking on names like this to ensure they're actually identifiers?

[Kodiologist]

You actually can call them, if you use mangled names. This is similar to the issue of how Python allows setattr(c, "$", 3) even though c.$ is a syntax error.

[scauligi]

There's a remaining reference to deftag in docs/style-guide.rst.

Are the changes in tests/native_tests/language.hy related to tag macros or just refactoring while you were in the file?

[scauligi]

Why did this test start failing?

[Kodiologist]

See the commit message.

[scauligi]

Hmm, I think I disagree with the (xfailed) assertion on line 447, but I'll make a separate issue for that.

[Kodiologist]

There's a remaining reference to deftag in docs/style-guide.rst.

Removed.

Are the changes in tests/native_tests/language.hy related to tag macros or just refactoring while you were in the file?

The ones in their own commit, at the beginning of this PR, are just refactoring while I was in the file.