Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

require and macro dependencies #1650

Closed
brandonwillard opened this issue Jun 29, 2018 · 7 comments
Closed

require and macro dependencies #1650

brandonwillard opened this issue Jun 29, 2018 · 7 comments

Comments

@brandonwillard
Copy link
Member

brandonwillard commented Jun 29, 2018

What are the expectations and conventions surrounding require-ed macros and their dependencies?

For example, consider the following:

In test-module.hy

(require [hy.contrib.walk [let]])

(defmacro test-module-macro [a]
  `(let [b 1] (+ b ~a)))

In some-file.hy

(require [test-module [*]])

(test-module-macro 1)

In its current state, running some-file.hy will produce

  => let([b, 1], b + 1)

  Traceback (most recent call last):
    File "/home/bwillard/projects/code/python/hy/hy/importer.py", line 199, in hy_eval
      return eval(ast_compile(expr, "<eval>", "eval"), namespace)
    File "<eval>", line 1, in <module>
  NameError: name 'let' is not defined

This result implies that (require [test-module [*]]) does not account for the required module's requirements (i.e. let in hy.contrib.walk).

How should dependencies like this be handled? More importantly, what would it take to automatically include macro dependencies?

FYI: This question started here, where @gilch provided a lot of important considerations and an approach.

@Kodiologist
Copy link
Member

Automatically determining which macros a piece of code depends upon is, I suspect, undecidable. But there's no reason in principle that we couldn't make require able to pull in macros that are themselves required by the named file. #1268 is related.

Generally, macro subroutines are better written as functions than macros. Unfortunately, that's not an option when you're using a preexisting macro like let. You could try using macroexpand.

@brandonwillard
Copy link
Member Author

The idea of require-ing a module's requires sounds helpful (in this situation), but perhaps not scalable — among other things.

My first response to this situation involved things like macroexpand, and @gilch provided some potential shortcomings in this comment.

Given that Hy seems partly inspired by Clojure, how does it handle these things? (Unfortunately, I'm not familiar enough with Clojure to readily find that answer in its source.)

@Kodiologist
Copy link
Member

Not scalable? Do you mean you expect there to be some kind of performance problem with allowing require to take macros that are required by the named file? Why so?

@refi64
Copy link
Contributor

refi64 commented Jun 29, 2018

I doubt it'd really be any slower than Python's imports already are TBH.

@brandonwillard
Copy link
Member Author

@Kodiologist, sure, but for no reason other than generic pessimism. Well, I would also include "complexity" (of code changes, comprehensibility, maintenance, etc.) into that scaling. Regardless, if there's negligible performance and/or complexity hits, then I'm all for it!

I'm walking through the require process in the debugger right now, so hopefully I'll have a better sense of what's going on (both in general and as it pertains to this issue).

@Kodiologist
Copy link
Member

I don't anticipate a performance issue, no, and the resulting Hy programs will obviously not be more complex. I don't think the implementation will be complex, either, but figuring out how to do it could take some work.

@gilch
Copy link
Member

gilch commented Jun 30, 2018

Given that Hy seems partly inspired by Clojure, how does it handle these things?

Clojure namespaces ("qualifies") symbols. So if you do a syntax quote, it automatically prepends the symbol's namespace to prevent conflicts. (Special forms are immune). If we had Clojure's system, then given your example,

(defmacro test-module-macro [a]
  `(let [b 1] (+ b ~a)))

If invoked like

(macroexpand-1 '(test-module-macro q))

Would expand to something like

(hy.contrib.walk/let [test-module/b 1] (+ test-module/b q))

Let's try it in Clojure. First change namespace.

Clojure 1.6.0
user=> (ns walk)
nil

Clojure already has let built in, so let's make a new one.

walk=> (defmacro LET [& body] `(let ~@body))
#'walk/LET
walk=> (ns test)
nil
test=> (refer 'walk :only '[LET])
nil
test=> (defmacro test-macro [a]
         `(LET [b 1] (+ b ~a)))
#'test/test-macro
test=> (macroexpand-1 '(test-macro q))
(walk/LET [test/b 1] (clojure.core/+ test/b q))

Of course, this doesn't actually work in Clojure.

test=> (test-macro q)
CompilerException java.lang.RuntimeException: Can't let qualified name: test/b, compiling:(NO_SOURCE_PATH:8:1)

That's because it was almost certainly a mistake. The b should have been a gensym to avoid accidental capture. That's why Clojure's let won't accept a qualified symbol as a target. This is easy to fix.

test=> (defmacro test-macro [a]
         `(LET [b# 1] (+ b# ~a)))
#'test/test-macro
test=> (macroexpand-1 '(test-macro q))
(walk/LET [b__36__auto__ 1] (clojure.core/+ b__36__auto__ q))
test=> (test-macro 41)
42

Hy uses defmacro/g! instead of the # suffix. Also + is a special form in Hy, so it wouldn't get qualified (But it would become hy.core.shadow/+ if not used in the function posiiton.)

If you actually want to capture (like for an anaphoric macro), you can do that too.

test=> (defmacro test-macro [a]
         `(LET [~'b 1] (+ ~'b ~a)))
#'test/test-macro
test=> (macroexpand-1 '(test-macro q))
(walk/LET [b 1] (clojure.core/+ b q))
test=> (test-macro 41)
42

Of course, all this is currently Hypothetical in Hy. Heh. But you can see how a proper namespacing implementation would be easier to use than my current recommendation,

(defmacro/g! test-module-macro [a]
  `(do
     (require [hy.contrib.walk [let :as ~g!let]])
     (~g!let [~g!b 1]
       (+ ~g!b ~a))))

brandonwillard added a commit to brandonwillard/hy that referenced this issue Sep 25, 2018
This commit adds just enough namespacing to resolve a macro first in the
Hy compilation namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace it was defined.  Namespacing is accomplished by adding
a `module` attribute to `HySymbol`, so that `HyExpression`s can be checked
for this definition namespace attribute and their car symbol resolved per the
above.

As well, a test is included that covers module-level macros (i.e. via `__macros`
and `__tags__`), AST generated for `require`, and the non-local macro namespace
resolution described above.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 5, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a comprehensive test has been added that covers
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 5, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 7, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 7, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 7, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 7, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 7, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 10, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

There's also a test--expected to fail--for hylang#1414.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 14, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 14, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 14, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 16, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 17, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
Kodiologist pushed a commit to brandonwillard/hy that referenced this issue Oct 23, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
Kodiologist pushed a commit to brandonwillard/hy that referenced this issue Oct 23, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 24, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Oct 24, 2018
This commit adds just enough namespacing to resolve a macro first in the macro's
defining module's namespace (i.e. the module assigned to the `HyASTCompiler`),
then in the namespace/module it's evaluated in.  Namespacing is accomplished by
adding a `module` attribute to `HySymbol`, so that `HyExpression`s can be
checked for this definition namespace attribute and their car symbol resolved
per the above.

As well, a couple tests have been added that cover
- the loading of module-level macros
  - e.g. that only macros defined in the `require`d module are added
- the AST generated for `require`
  - using macros loaded from modules imported via bytecode
- the non-local macro namespace resolution described above
  - a `require`d macro that uses a macro `require` exclusively in its
    module-level namespace
- and that (second-degree `require`d) macros can reference variables within
  their module-level namespaces.

Closes hylang#1268, closes hylang#1650, closes hylang#1416.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants