Clojure

data-reader returning nil causes exception

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.4, Release 1.5, Release 1.6, Release 1.7, Release 1.8
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Environment:
    clojure 1.5 beta2, Mac OS X 10.8.2, java version "1.6.0_37"
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

user=> (binding [*data-readers* {'f/ignore (constantly nil)}] 
         (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f  clojure.lang.Util.runtimeException (Util.java:219)

The original reader code did not distinguish between the absence of a data-reader and a returned value of nil from the appropriate data-reader. It therefore got confused and tried to find a dispatch macro, sending it further down the incorrect code path, ultimately yielding a misleading error message.

The original documentation did not distinguish nil as an illegal value. Clearly this bug was an oversight in the original data-reader code, not an intentional feature.

The patch uses a sentinel value to distinguish the missing data-reader case from the nil returned value case.

Patch: clj-1139-2.patch

Activity

Hide
Steve Miner added a comment - - edited

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch allows a data-reader to return nil instead of throwing. Does sanity check that possible tag or record isJavaIdentifierStart(). Gives better error message for special characters that might actually be dispatch macros (rather than assuming it's a tagged literal).

Show
Steve Miner added a comment - - edited clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch allows a data-reader to return nil instead of throwing. Does sanity check that possible tag or record isJavaIdentifierStart(). Gives better error message for special characters that might actually be dispatch macros (rather than assuming it's a tagged literal).
Steve Miner made changes -
Field Original Value New Value
Attachment clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch [ 11773 ]
Steve Miner made changes -
Labels reader patch reader
Steve Miner made changes -
Patch Code and Test [ 10002 ]
Hide
Steve Miner added a comment -

clj-1138-data-reader-return-nil-for-no-op.patch allows a data-reader returning nil to be treated as a no-op by the reader (like #_). nil is not normally a useful value (actually it causes an exception in Clojure 1.4 through 1.5 beta2) for a data-reader to return. With this patch, one could get something like a conditional feature reader using data-readers.

Show
Steve Miner added a comment - clj-1138-data-reader-return-nil-for-no-op.patch allows a data-reader returning nil to be treated as a no-op by the reader (like #_). nil is not normally a useful value (actually it causes an exception in Clojure 1.4 through 1.5 beta2) for a data-reader to return. With this patch, one could get something like a conditional feature reader using data-readers.
Steve Miner made changes -
Attachment clj-1138-data-reader-return-nil-for-no-op.patch [ 11774 ]
Hide
Steve Miner added a comment -

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch is the first patch to consider. It merely allows nil as a value from a data-reader and returns nil as the final value. I think it does what was originally intended for dispatch macros, and gives a better error message in many cases (mostly typos).

The second patch, clj-1138-data-reader-return-nil-for-no-op.patch, depends on the other being applied first. It takes an extra step to treat a nil value returned from a data-reader as a no-op for the reader (like #_).

Show
Steve Miner added a comment - clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch is the first patch to consider. It merely allows nil as a value from a data-reader and returns nil as the final value. I think it does what was originally intended for dispatch macros, and gives a better error message in many cases (mostly typos). The second patch, clj-1138-data-reader-return-nil-for-no-op.patch, depends on the other being applied first. It takes an extra step to treat a nil value returned from a data-reader as a no-op for the reader (like #_).
Hide
Steve Miner added a comment -

It turns out that you can work around the original problem by having your data-reader return '(quote nil) instead of plain nil. That expression conveniently evaluates to nil so you can get a nil if necessary. This also works after applying the patches so there's still a way to return nil if you really want it.

(binding [*data-readers* {'x/nil (constantly '(quote nil))}] (read-string "#x/nil 42"))
;=> (quote nil)

Show
Steve Miner added a comment - It turns out that you can work around the original problem by having your data-reader return '(quote nil) instead of plain nil. That expression conveniently evaluates to nil so you can get a nil if necessary. This also works after applying the patches so there's still a way to return nil if you really want it. (binding [*data-readers* {'x/nil (constantly '(quote nil))}] (read-string "#x/nil 42")) ;=> (quote nil)
Hide
Andy Fingerhut added a comment -

Patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch dated Dec 22 2012 still applies cleanly to latest master if you use the following command:

% git am --keep-cr -s --ignore-whitespace < clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch

Without the --ignore-whitespace option, the patch fails only because some whitespace was changed in Clojure master recently.

Show
Andy Fingerhut added a comment - Patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch dated Dec 22 2012 still applies cleanly to latest master if you use the following command: % git am --keep-cr -s --ignore-whitespace < clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch Without the --ignore-whitespace option, the patch fails only because some whitespace was changed in Clojure master recently.
Hide
Andy Fingerhut added a comment -

OK, now with latest master (1.5.0-RC15 at this time), patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch no longer applies cleanly, not even using --ignore-whitespace in the 'git am' command given above. Steve, if you could see what needs to be updated, that would be great. Using the patch command as suggested in the "Updating stale patches" section of http://dev.clojure.org/display/design/JIRA+workflow wasn't enough, so it should probably be carefully examined by hand to see what needs updating.

Show
Andy Fingerhut added a comment - OK, now with latest master (1.5.0-RC15 at this time), patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch no longer applies cleanly, not even using --ignore-whitespace in the 'git am' command given above. Steve, if you could see what needs to be updated, that would be great. Using the patch command as suggested in the "Updating stale patches" section of http://dev.clojure.org/display/design/JIRA+workflow wasn't enough, so it should probably be carefully examined by hand to see what needs updating.
Steve Miner made changes -
Attachment clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch [ 11773 ]
Steve Miner made changes -
Attachment clj-1138-data-reader-return-nil-for-no-op.patch [ 11774 ]
Hide
Steve Miner added a comment -

I removed my patches. Things have changes recently with the LispReader and new EdnReader.

Show
Steve Miner added a comment - I removed my patches. Things have changes recently with the LispReader and new EdnReader.
Steve Miner made changes -
Patch Code and Test [ 10002 ]
Steve Miner made changes -
Labels patch reader reader
Nicola Mometto made changes -
Affects Version/s Release 1.7 [ 10250 ]
Affects Version/s Release 1.6 [ 10157 ]
Affects Version/s Release 1.8 [ 10254 ]
Patch Code and Test [ 10002 ]
Nicola Mometto made changes -
Alex Miller made changes -
Approval Triaged [ 10120 ]
Hide
Alex Miller added a comment -

Fixed whitespace warning and updated patch so it applies, no semantic changes, attribution retained in clj-1139-2.patch.

Show
Alex Miller added a comment - Fixed whitespace warning and updated patch so it applies, no semantic changes, attribution retained in clj-1139-2.patch.
Alex Miller made changes -
Description If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

    (binding [*data-readers* {'f/ignore (constantly nil)}] (read-string "#f/ignore 42 10"))

RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

{code}
user=> (binding [*data-readers* {'f/ignore (constantly nil)}] (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
{code}

*Patch:* clj-1139-2.patch
Attachment clj-1139-2.patch [ 16490 ]
Alex Miller made changes -
Description If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

{code}
user=> (binding [*data-readers* {'f/ignore (constantly nil)}] (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
{code}

*Patch:* clj-1139-2.patch
If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

{code}
user=> (binding [*data-readers* {'f/ignore (constantly nil)}]
         (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
{code}

*Patch:* clj-1139-2.patch
Hide
Alex Miller added a comment -

Ticket needs better description of problem and approach taken in the patch.

Show
Alex Miller added a comment - Ticket needs better description of problem and approach taken in the patch.
Hide
Steve Miner added a comment -

If the problem isn't clear, I would ask why would a nil return value be treated specially for a data-reader? And if it is considered illegal by design, does this error message enlighten the user?

I could not find any documented restriction at the time the bug was filed and I still can't find any today. So it seems like a simple bug to me. The data-reader should be allowed to return nil, and the Clojure reader should process the nil as usual. My work-around was to return (quote nil) which gave the intended behavior without triggering the bug.

Show
Steve Miner added a comment - If the problem isn't clear, I would ask why would a nil return value be treated specially for a data-reader? And if it is considered illegal by design, does this error message enlighten the user? I could not find any documented restriction at the time the bug was filed and I still can't find any today. So it seems like a simple bug to me. The data-reader should be allowed to return nil, and the Clojure reader should process the nil as usual. My work-around was to return (quote nil) which gave the intended behavior without triggering the bug.
Hide
Alex Miller added a comment -

Would appreciate more updates to the description. My question would be whether invoking a data reader function should ever return nil. Is there a good use case to need this? It seems you are reading the description of a non-nil tagged value with the reader and thus getting back nil is confusing. That's not possibly round-trippable and thus seems asymmetric.

Show
Alex Miller added a comment - Would appreciate more updates to the description. My question would be whether invoking a data reader function should ever return nil. Is there a good use case to need this? It seems you are reading the description of a non-nil tagged value with the reader and thus getting back nil is confusing. That's not possibly round-trippable and thus seems asymmetric.
Hide
Steve Miner added a comment -

Nulla poena sine lege or basically unless you say it's illegal, I should be able to do it. My trivial example is #C NULL which seems like an obvious nil to me.

Looking back on this issue, I can see that most people think of tagged literals as a way of encoding foreign values in Clojure literals. If you only care about an extensible data notation, who needs another way of writing nil? That's a fair question.

I wanted to use data-readers as somewhat circumscribed reader macros (as used in Common Lisp). I discovered this bug while I was doing something platform specific (long before reader conditionals were implemented). In my situation, it was convenient to return nil on "other" platforms.

Many usages of data-readers are not bijective. For example, #infix (3 + 4) interpreted as constant 7 is likewise not round-trippable. Unless you're Dan Friedman or Wil Byrd, round-tripping is a tough requirement.

I will try to update my description with a bit more context, but I don't want to distract anyone from the obvious bug (and bad error message) with my unorthodox usage.

Show
Steve Miner added a comment - Nulla poena sine lege or basically unless you say it's illegal, I should be able to do it. My trivial example is #C NULL which seems like an obvious nil to me. Looking back on this issue, I can see that most people think of tagged literals as a way of encoding foreign values in Clojure literals. If you only care about an extensible data notation, who needs another way of writing nil? That's a fair question. I wanted to use data-readers as somewhat circumscribed reader macros (as used in Common Lisp). I discovered this bug while I was doing something platform specific (long before reader conditionals were implemented). In my situation, it was convenient to return nil on "other" platforms. Many usages of data-readers are not bijective. For example, #infix (3 + 4) interpreted as constant 7 is likewise not round-trippable. Unless you're Dan Friedman or Wil Byrd, round-tripping is a tough requirement. I will try to update my description with a bit more context, but I don't want to distract anyone from the obvious bug (and bad error message) with my unorthodox usage.
Hide
Steve Miner added a comment -

By the way, this bug is CLJ-1138, but the proposed patch says "1139" which might confuse some busy reviewers.

Show
Steve Miner added a comment - By the way, this bug is CLJ-1138, but the proposed patch says "1139" which might confuse some busy reviewers.
Hide
Steve Miner added a comment -

I tested the patch and it worked well for me with the current master. I would suggest adding another test to confirm that the edn/read-string works correctly as well. Here's what I used. This also tests that overriding the default readers works. Please feel free to take the test if you want it.

(deftest clj-1138-uuid-override
  (is (nil? (binding [*data-readers* {'uuid (constantly nil)}]
              (read-string "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))
  (is (nil? (edn/read-string {:readers {'uuid (constantly nil)}}
                             "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))
Show
Steve Miner added a comment - I tested the patch and it worked well for me with the current master. I would suggest adding another test to confirm that the edn/read-string works correctly as well. Here's what I used. This also tests that overriding the default readers works. Please feel free to take the test if you want it.
(deftest clj-1138-uuid-override
  (is (nil? (binding [*data-readers* {'uuid (constantly nil)}]
              (read-string "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))
  (is (nil? (edn/read-string {:readers {'uuid (constantly nil)}}
                             "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))
Steve Miner made changes -
Description If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

{code}
user=> (binding [*data-readers* {'f/ignore (constantly nil)}]
         (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
{code}

*Patch:* clj-1139-2.patch
If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

{code}
user=> (binding [*data-readers* {'f/ignore (constantly nil)}]
         (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)
{code}

The original reader code did not distinguish between the absence of a data-reader and a returned value of nil from the appropriate data-reader. It therefore got confused and tried to find a dispatch macro, sending it further down the incorrect code path, ultimately yielding a misleading error message.

The original documentation did not distinguish nil as an illegal value. Clearly this bug was an oversight in the original data-reader code, not an intentional feature.

The patch uses a sentinel value to distinguish the missing data-reader case from the nil returned value case.

*Patch:* clj-1139-2.patch

People

Vote (2)
Watch (2)

Dates

  • Created:
    Updated: