<< Back to previous view

[TRDR-42] Allow wrapping constants so their location can be accessed Created: 18/Mar/17  Updated: 19/Mar/17  Resolved: 19/Mar/17

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Thomas Heller Assignee: Nicola Mometto
Resolution: Declined Votes: 0
Labels: None


Currently the reader will lose all source information (line, column, end-line, end-column, file) on values that don't support metadata.

This change allows (optionally) wrapping everything that doesn't support metadata in the Constant record.

This would allow the CLJS analyzer to show correct locations that it currently cannot do in some circumstances.

(+ 1 "2")

The location of "2" is unknown and thus the location of the error is that of the +.

The default behaviour is unchanged.

(require '[clojure.tools.reader :as rdr])
(require '[clojure.tools.reader.reader-types :as rdrt])

(let [rdr (-> (rdrt/string-reader "[1 \"2\" nil true false 1.01])")
      x (rdr/read {} rdr)]
  (binding [*print-meta* true]
    (prn x)))


^{:line 1, :column 1, :end-line 1, :end-column 28} [1 "2" nil true false 1.01]

With wrapping enabled you'd get:

(binding [rdr/*wrap-constants* true]
  (let [rdr (-> (rdrt/string-reader "[1 \"2\" nil true false 1.01])")
        x (rdr/read {} rdr)]
    (binding [*print-meta* true]
      (prn x))))


^{:line 1, :column 1, :end-line 1, :end-column 28} [#clojure.tools.reader.Constant{:loc-info {:line 1, :column 2, :end-line 1, :end-column 3}, :value 1} #clojure.tools.reader.Constant{:loc-info {:line 1, :column 4, :end-line 1, :end-column 7}, :value "2"} #clojure.tools.reader.Constant{:loc-info {:line 1, :column 8, :end-line 1, :end-column 11}, :value nil} #clojure.tools.reader.Constant{:loc-info {:line 1, :column 12, :end-line 1, :end-column 16}, :value true} #clojure.tools.reader.Constant{:loc-info {:line 1, :column 17, :end-line 1, :end-column 22}, :value false} #clojure.tools.reader.Constant{:loc-info {:line 1, :column 23, :end-line 1, :end-column 27}, :value 1.01}]

The loc-info property of the Constant record could be moved to the metadata of the record.

The CLJS analyzer could easily unwrap this and extract the correct location info to merge that into the AST.

Comment by Thomas Heller [ 18/Mar/17 5:07 AM ]

The implementation is currently available here [1].

I first wanted to confirm that this is something worth considering before cutting actual patches. This change can easily be ported to the CLJS/EDN readers as well.

[1] https://github.com/clojure/tools.reader/compare/master...thheller:constant-wrapper

Comment by Thomas Heller [ 18/Mar/17 11:55 AM ]

My initial enthusiasm was crushed by macroexpand. As soon as any macro does any kind of type detection on values it receives this whole approach breaks down. Unwrapping the form before handing it to macroexpand has the same effect as not having the information in the first place as analyze happens after expansion.

So while I still think this would be useful for tools, it is not viable for the analyzer.

Comment by Thomas Heller [ 19/Mar/17 2:35 AM ]

Even from a tools perspective this isn't all that useful as it makes for sense to wrap everything and not eval tagged literals.

Sorry for the noise.

Generated at Sat Mar 25 16:48:11 CDT 2017 using JIRA 4.4#649-r158309.