Clojure

BigInteger is print-dup'able but not readable

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.3
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

Per Rich, "We should never be printing something that can't be read."

user=> (binding [*print-dup* true] (print-str (java.math.BigInteger. "1")))
"1BIGINT"
user=> (read-string *1)
NumberFormatException Invalid number: 1BIGINT  clojure.lang.LispReader.readNumber (LispReader.java:253)

This can either be resolved by removing the print-dup for BigInteger, printing it using the N notation (thus being read in as a BigInt), or adding read support for the BIGINT notation.

  1. print-dup-big-integer.patch
    21/May/11 9:58 PM
    1 kB
    Alexander Taggart
  2. readable-BIGINT.patch
    27/May/11 1:33 PM
    3 kB
    Alexander Taggart
  3. readable-BIGINT-update-1.patch
    30/May/11 1:26 PM
    3 kB
    Alexander Taggart
  4. remove-biginteger-print.patch
    09/Jun/11 1:21 AM
    2 kB
    Alexander Taggart

Activity

Hide
Alexander Taggart added a comment -

Patch emits BigIntegers using the N notation.

Apply patch after CLJ-799.

Show
Alexander Taggart added a comment - Patch emits BigIntegers using the N notation. Apply patch after CLJ-799.
Hide
Christopher Redinger added a comment -
Show
Christopher Redinger added a comment - See discussion on clojure-dev mailing list: https://groups.google.com/d/topic/clojure-dev/VHvIGmS3HGw/discussion
Hide
Christopher Redinger added a comment -

Commentary on the Rich quote: We should read in the same thing that we wrote out. I think the preferred solution would be to read BigIntegers back in as BigIntegers.

Show
Christopher Redinger added a comment - Commentary on the Rich quote: We should read in the same thing that we wrote out. I think the preferred solution would be to read BigIntegers back in as BigIntegers.
Hide
Alexander Taggart added a comment -

readable-BIGINT.patch: makes the BIGINT notation readable to a java.lang.BigInteger.

Show
Alexander Taggart added a comment - readable-BIGINT.patch: makes the BIGINT notation readable to a java.lang.BigInteger.
Hide
Christopher Redinger added a comment -

Tests don't apply cleanly on latests master (due to us taking in CLJ-797 in alpha 8 without this one).

Would you mind refreshing this patch?

Show
Christopher Redinger added a comment - Tests don't apply cleanly on latests master (due to us taking in CLJ-797 in alpha 8 without this one). Would you mind refreshing this patch?
Hide
Alexander Taggart added a comment -

readable-BIGINT-update-1.patch: applies to master

Show
Alexander Taggart added a comment - readable-BIGINT-update-1.patch: applies to master
Hide
Stuart Sierra added a comment -

Patch readable-BIGINT-update-1.patch applies on master as of commit 66a88de9408e93cf2b0d73382e662624a54c6c86.

However, I find this notation confusing. "BIGINT" looks like BigInt but actually means BigInteger.

If Clojure uses BigInt everywhere instead of BigInteger, I recommend removing this literal syntax.

Show
Stuart Sierra added a comment - Patch readable-BIGINT-update-1.patch applies on master as of commit 66a88de9408e93cf2b0d73382e662624a54c6c86. However, I find this notation confusing. "BIGINT" looks like BigInt but actually means BigInteger. If Clojure uses BigInt everywhere instead of BigInteger, I recommend removing this literal syntax.
Hide
Rich Hickey added a comment -

Let's please remove print-dup for BigIntegers. None of these other ideas have been approved. I don't want new literal syntax, nor print-dup taking one thing and reading as another. I especially don't want to have to discover what the plan is by reading the patch, The plan should be approved up front and be clear from the description.

Show
Rich Hickey added a comment - Let's please remove print-dup for BigIntegers. None of these other ideas have been approved. I don't want new literal syntax, nor print-dup taking one thing and reading as another. I especially don't want to have to discover what the plan is by reading the patch, The plan should be approved up front and be clear from the description.
Hide
Alexander Taggart added a comment - - edited

remove-biginteger-print.patch: removes both print-dup and print-method methods for BigInteger. Now defaults to Number's methods.

And I strongly agree that patches should come after "the plan", but in the face of silence from the planners, some of us might feel inclined to make ready with some potential solutions while we have some time to spare.

Show
Alexander Taggart added a comment - - edited remove-biginteger-print.patch: removes both print-dup and print-method methods for BigInteger. Now defaults to Number's methods. And I strongly agree that patches should come after "the plan", but in the face of silence from the planners, some of us might feel inclined to make ready with some potential solutions while we have some time to spare.
Hide
Christopher Redinger added a comment -

Rich: I am interpreting the "no new literal syntax" part of your comment to mean that you don't want the "BIGINT" which is in print-method, therefore print-method is out, as well as print-dup. If that is the correct interpretation, this patch looks right.

Show
Christopher Redinger added a comment - Rich: I am interpreting the "no new literal syntax" part of your comment to mean that you don't want the "BIGINT" which is in print-method, therefore print-method is out, as well as print-dup. If that is the correct interpretation, this patch looks right.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: