Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't use bigint as count for repeat #55

Open
jfhamlin opened this issue Sep 11, 2023 · 3 comments
Open

Can't use bigint as count for repeat #55

jfhamlin opened this issue Sep 11, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@jfhamlin
Copy link
Contributor

jfhamlin commented Sep 11, 2023

Actual

user=> (repeat 2N 1)
argument 0: cannot coerce *lang.BigInt to int64

Expected

user=> (repeat 2N 1)
(1 1)
@jfhamlin jfhamlin added the bug Something isn't working label Sep 11, 2023
@mateodif
Copy link
Contributor

mateodif commented Nov 13, 2023

I've been looking into this issue.

Here's what I got so far:
The coercion of values happens in coerceGoValue, in apply.go.

  • There are a couple special cases (for Slice and Func) and a default case that should handle the rest.
  • We don't seem to handle pointers, which is the case for this particular issue (*lang.BigInt). This can be easily fixed by checking for reflect.Ptr in the default case.
  • BigInt, BigDecimal, etc are wrappers of their native types i.e. big.Int, big.Decimal respectively.
    • This causes the Convert function of reflect to not be able to know how to convert the type to, for example, int64.
    • These wrapper types have internal methods for converting to types such as Int64(), String(), etc.

A way of going around this issue would be to use something like MethodByName and check for methods with the same name as the targetType. This is a bit ugly, and we have the issue that MethodByName is case sensitive, although that could be easily fixed by getting the native type as a string and doing .Title() to it or with a helper function.

I have a working solution based on these points, but I recognize that it's an ugly hack. Let me know your thoughts.

@jfhamlin
Copy link
Contributor Author

Thanks for taking a look! I'm afk right now so I may be mistaken on some of this, but I think the ideal solve here is a dedicated Go implementation of Repeat for BigInt that supports the arbitrarily large count that it implies. When in doubt, refer to clojure's source for this: both core.clj and the core Java classes.

@jfhamlin
Copy link
Contributor Author

Looks like my memory on this was dodgy re Clojure's support for Repeat, and you're probably on the right track here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants