Macroexpansion discards &form metadata


This patch changes the behavior of metadata when used in conjunction with macros. The metadata &form is now merged with the metadata of the macro call sexpr. This allows users to either type-hint the inner or the outer form in a macro call and have somewhat better results. In the past, the metadata from the macroexpand was used as-is. This disallowed code like the following, to work without reflection:

(.trim ^String (when true "hello "))

Patch: 2013-10-11_CLJ-865_Fix-With-Tests.diff
Screened by: Timothy Baldridge

--------- Implementation Details ----------
As discussed in there is a "surprise factor" when type-hinting an expression that represents a macro, such as with (.length ^String (doto (identity "x") prn)). Here the doto macro discards the metadata on &form, causing a reflective lookup. This has the effect that while expressions representing function calls can be type-hinted, expressions representing macros in general cannot. The doto macro could be rewritten to respect its &form metadata, but doing this for every macro in existence would be tedious and error-prone. Instead, I propose a change to the compiler, to cause macroexpansion to hang onto the metadata automatically.

The first patch attached adds a test for the behavior I propose: this test fails. After applying the second patch, the test passes.

There are a couple points that merit further consideration before accepting my patch:

  • I'm not sure I actually got the Java code formatted correctly. My editor is not well-configured to get the clojure/core style right automatically.

  • My solution is to take the &form metadata, drop :line/:file keys, and then merge with the returned metadata, with &form taking precedence. I'm not sure whether this is the right approach in all cases, even though it works for :tag metadata.

  • I achieved this with a change to the compiler, which makes it fairly heavy-weight. It should be possible to instead adjust defmacro if changes to the compiler are not desirable. However, I believe this would involve substantially more work and be harder to test (for example, multiple arities complicate things). It seems nicer to treat the macroexpansion as a black box and then make metadata tweaks to the result, rather than modifying their actual defmacro code.

  • If a macro expands to something that is not an IObj, such as an Integer, then my patch silently discards the caller's metadata. Would it be better to throw an exception?






Alan Malloy





Code and Test