Clojure

Macroexpansion discards &form metadata

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Vetted

Description

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 http://groups.google.com/group/clojure/browse_thread/thread/2690cb6ca0e8beb8 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?
  1. 0001-Add-test-for-macroexpansion-metadata-preservation.patch
    26/Oct/11 3:19 PM
    1 kB
    Alan Malloy
  2. 0002-Preserve-form-metadata-on-macroexpanded-forms.patch
    26/Oct/11 3:19 PM
    2 kB
    Alan Malloy
  3. 0003-Make-defmacro-preserve-form-metadata.patch
    28/Oct/11 1:12 AM
    4 kB
    Alan Malloy
  4. 0004-Another-stab-at-implementing-this.patch
    13/Nov/11 8:29 PM
    3 kB
    Alan Malloy
  5. 2013-10-11_CLJ-865_Fix-With-Tests.diff
    11/Oct/13 12:57 PM
    5 kB
    Timothy Baldridge
  6. clj865.patch
    03/Dec/13 2:20 AM
    11 kB
    Alan Malloy
  7. clj-865.patch
    11/Dec/13 10:26 AM
    10 kB
    Alan Malloy
  8. clj-865-updated-v2-patch.txt
    14/Aug/13 8:05 PM
    4 kB
    Andy Fingerhut
  9. updated.patch
    01/Jun/12 2:04 PM
    4 kB
    Alan Malloy

Activity

People

Vote (13)
Watch (9)

Dates

  • Created:
    Updated: