Macro name shadowing/collisions and removal

[brandonwillard]

A user defined require macro will block the compilation of all require statements and there's little recourse for undoing this. As far as I can tell, a user would have to remove the macro entry in hy.macros._hy_macros (or __macros__ under #1682) and, since the macro dictionary is a Hy internal, that isn't a very reasonable recommendation.

Nearly the same situation occurs with plain functions:

hy 0.15.0+39.gd2319dc using CPython(default) 3.6.6 on Linux
=> (defn test [] (print "test function"))
=> (defmacro test [] (print "test macro"))
<function <lambda> at 0x7fc95f8c0ea0>
=> (test)
test macro

Since we can't simply (del test) (the macro or the plain function) or redefine the function in order to regain it, this situation can easily lead to confusion and frustration.
(I imagine this might have an issue of its own already, but I didn't see one in my first pass.)
One can reset the Hy/Python session, but, when that's the easiest/best option, something needs to be fixed!

This general discord between macros, plain functions and Hy special forms like require is not a trivial issue, and, while we could make it easier to work around the aforementioned situations (e.g. have the compiler apply del to macros and tags), we should simply rethink and/or formalize the interaction between these three.

For instance, I believe I've mentioned/asked this before, but if the functions backing macros used their macro's names and were defined directly in their namespaces, then this wouldn't be an issue.

Anyway, thoughts?

[Kodiologist]

One thing to keep in mind is that in at least one way, we're currently taking advantage of this: hy.core.shadow works by defining functions with the same names as special operators, so wherever you can't use a special form (e.g., in (reduce + foo), the symbol + can't be a special operator), the shadow function gets used instead.

It wouldn't be too unreasonable to forbid users from giving a macro the same name as a special operator. That could be part of a solution, perhaps.

[gilch]

In Clojure, if I recall, a special form in the function position always takes precedence over a macro with the same name. To use a macro with the same name as a special form, you'd have to use its fully qualified name; the unqualified name is assumed to mean the special form.

[brandonwillard]

Out of curiosity, I made some minor edits that expose the macro functions at module level.
With those changes, one can do the following:

hy 0.15.0+43.g31a630d using CPython(default) 3.6.6 on Linux
=> (defmacro test [] (print "macro"))
=> (test)
macro
=> test
<function _hy_anon_var_1 at 0x7f0be5b2cea0>
=> (defn test [] (print "function"))
=> (test)
function
=> test
<function test at 0x7f0be5b2c950>
=> (del test)
=> test
Traceback (most recent call last):
  File "/home/bwillard/projects/code/python/hy/hy/importer.py", line 173, in hy_eval
    return eval(ast_compile(expr, "<eval>", "eval"), globals, locals)
  File "<eval>", line 1, in <module>
NameError: name 'test' is not defined
=> (test)
Traceback (most recent call last):
  File "/home/bwillard/projects/code/python/hy/hy/importer.py", line 173, in hy_eval
    return eval(ast_compile(expr, "<eval>", "eval"), globals, locals)
  File "<eval>", line 1, in <module>
NameError: name 'test' is not defined
=> 

The changes are in an experimental macros-in-module-namespaces branch that starts from #1682.

Some interesting things have already come up, and I'll describe them here when I get a minute.

[peaceamongworlds]

I think that @brandonwillard's approach is the correct way forward, but it doesn't fully solve the problem.

When running a script, macros still have to get expanded out before any of the code is actually evaluated. Hence they will still shadow any functions or objects which are being called. This seems to work in a repl because the effective unit of compilation is a form, not an entire module. Hence the compiler can check the runtime environment to see if the macro has been overwritten.

It seems to me that the way to actually solve this issue is to change hy's unit of compilation to a top-level form everywhere so that the compiler can interact with the runtime environment. I know that this would be a big change to how the compiler works, but the only downside in terms of backwards compatibility that I can see is that hy2py would have to execute an entire file in order to compile it. It wouldn't actually change the way that normal hy scripts are run, since they get executed anyway.

[scauligi]

Since we can't simply (del test)

This might not be an issue anymore with how macros are handled in the current Hy:

;; test.hy

(defn test [] (print "fn"))
(defmacro test [] (print "macro-c") '(print "macro-e"))

(test)

(eval-when-compile
  (del (get __macros__ "test")))

(test)

$ hy test.hy
macro-c
macro-e
fn

$ hy test.hy
macro-e
fn

[peaceamongworlds]

I know that macros can technically be deleted, but you can still end up in very confusing situation if you don't realise that a Python object is being shadowed by a macro. In the previous example, (type test), (help test), (test.__call__) would all work as though test is a function, not a macro. In cases where there are accidental name clashes, it can be really difficult to debug what's going on.

I don't think that (del (get __macros__ "test")) should be official advice in dealing with this. This method also won't work for builtin-in macros from #1979.

[peaceamongworlds]

If, for example, hy had a Macro object, that existed at runtime, which had the same name as the macro, this would solve these potential clashes, and it would actually allow the macro to be easily inspected at runtime. The compiler could then just check if the calling symbol is of type Macro when macroexpanding. However this does require the compiler to be able to interact with the runtime environment.

[scauligi]

Ah ok, I see what you're saying now.
If we take @brandonwillard 's original suggestion to have the underlying macro function (perhaps wrapped into a Macro type?) have the same name as the macro, would that cause any problems? We could have require import the corresponding macro objects as well. The compiler already does macro expansion by the compile-time contents of __macros__, so I don't think there should be any problems leaving that as-is.

[peaceamongworlds]

I think that it's a step in the right direction, but it doesn't solve the issue if all macros get expanded out before any code is evaluated, because then you have no way of knowing whether a macro will be shadowed by another python object.

I don't see any way of solving this without compiling form by form, but that would have other issues like breaking hy2py.

Maybe the best thing to do would just be to have a compile time check to see if a symbol that is already assigned to a macro is being redefined and just raise a warning to the user.

[allison-casey]

was this fixed in #2104? A user defined require macro can now be overwritten by re-requring the original result macro. i.e.

python-venv-3.10.0b1 ❯ hy --spy
import hy
hy 1.0a1+159.gb47a1a74.dirty using CPython(default) 3.10.0b1 on Linux
=> (require [hy.contrib.walk [let]])
hy.macros.require('hy.contrib.walk', None, assignments=[['let', 'let']], prefix='')
None
=> (defmacro require [] 1)
hy.macros.macro('require')(lambda hyx_XampersandXcompiler: 1)
None
=> (require)
1
1
=> (hy.macros.require "hy.core.result_macros" None :assignments [["require" "require"]])
hy.macros.require('hy.core.result_macros', None, assignments=[['require', 'require']])
True
=> (require [hy.contrib.walk [let]])
hy.macros.require('hy.contrib.walk', None, assignments=[['let', 'let']], prefix='')
None

granted the interface could be better, but the issue of user defined macros shadowing special forms seems to be fixed?

[Kodiologist]

Yeah, it's not obvious to the user how to do it, but it can be done now.

[allison-casey]

so is this a docme issue now? or does there need to be code changes to improve the interface in this situation. It would be nice to be able to do
(hy.macros.require "hy.core.result_macros" None :assignments ["require"]) in cases where you don't need to require as a different name

[Kodiologist]

If we want to advertise this to users, we should probably provide a nicer interface, yeah.

[allison-casey]

probably can just make the target_module and assignments default args so when doing it manually it could be written as

(hy.macros.require "hy.core.result_macros" :assignments ["require"]) with minimal/no changes to the require result macro

[allison-casey]

#2242 made Hy's unit of compilation a top level form and #1979 made it so builtin macros aren't auto injected into every module's __macros__ object. So feasibly we could just emit a warning when redefining a macro like clojure does

Clojure 1.10.2
user=> (defmacro require [])
WARNING: require already refers to: #'clojure.core/require in namespace: user, being replaced by: #'user/require
#'user/require

and then maybe add a delmacro core macro to delete a macro from the current module without affecting core macros. So that the following works

=> (require hyrule *)
=> (defmacro require [])
<string>:1: UserWarning: require already refers to: `require` in module: `builtins`, being replaced by: `some-module.require`
=> (delmacro require)
=> (require hyrule *)