Resolve boxed math warnings

[valerauko]

• added a bunch of type hints to resolve boxed math warnings
• enabled warnings in the dev profile to motivate people to fix things

[KingMob]

The non-clj-commons namespaces are deprecated. I'd prefer not to update them for anything other than security fixes.

[KingMob]

Some minor changes, but looks fine otherwise

[KingMob]

Hmmm. hash returns an int, not a long. But the compiler uses the hint to force a cast. Not sure which is faster, though.

Before:

    public int hashCode() {
        return RT.intCast(Numbers.xor(((IFn)const__4.getRawRoot()).invoke(this.wrapper), ((IFn)const__4.getRawRoot()).invoke(this.type)));
    }

After:

    public int hashCode() {
        return RT.intCast(RT.uncheckedLongCast((Number)((IFn)const__4.getRawRoot()).invoke(this.wrapper)) ^ RT.uncheckedLongCast((Number)((IFn)const__4.getRawRoot()).invoke(this.type)));
    }

Truthfully, the best thing is to skip hints here and use explicit conversions to match the types involved. Try:

(hashCode [_]
    (int
      (bit-xor
        (long (hash wrapper))
        (long (hash type)))))

[valerauko]

Does Clojure even allow ^int type hints? It gives me an "Only long and double primitives are supported" error.

I'll change to explicit cast.

[KingMob]

^int is a valid type hint, but not for function parameters or return values. It's fine in local contexts, like a let.

Hell, it used to be (iirc) that unboxed parameters weren't allowed at all, but it seems like they're allowed for fn arities up to 4 now.

[KingMob]

(You shouldn't need to hint hashCode btw, that's defined by Object.)

[KingMob]

Seems like the hint should be higher up, but it's not working. I forget if this is a limitation of deftype or not.

Probably preferable to convert at the top of the let, like:

(let [cost (double cost)
      m' (assoc-in ...

[KingMob]

Or a limitation of defprotocol, I mean, which defines IConversionGraph. (Which should be named something else, since it's not an interface.)

[valerauko]

Yeah I tried to add the hint both on defprotocol and deftype but it errored...

[KingMob]

Not surprised. I get the compromises necessary to interop with Java, but the whole deftype/protocol/record system is ugly and sticks out.

[KingMob]

You know, we already use clj-commons.primitive-math elsewhere, we should probably do the same here for consistency. Sorry, can you change it?

[valerauko]

@KingMob late, but I updated to using primitive-math. Is this what you were referring to?

Are the boxed math warning around line 130 in graph.clj also unfixable due to #68 ?

[KingMob]

Hmmm. I wonder why we downcast the index to an int. Since chunk-size is long, the index will be immediately widened back to a long for the addition.

[KingMob]

Looks good, but we've been using p as the alias for primitive-math elsewhere, instead of m. Can you make it consistent?

[valerauko]

we've been using p as the alias for primitive-math elsewhere, instead of m

I used m because p was already taken. Should I change the existing alias to something else?

[KingMob]

I think p is more consistent with existing primitive-math examples, and would prefer proto for protocols.

[valerauko]

Eastwood fails with the reflection on the (.type dst) which can't be fixed according to the comment

[KingMob]

I think this is fine for now. Thanks!

Something is still wrong about that def-conversion/AOT problem, but we're not going to fix it here. It seemed related to CLJ-1650/1741, but the timing is wrong; that bug was fixed in Clojure 1.8. I wonder if it's related to Potemkin and deftype+?

[valerauko]

Thanks! I guess the warnings in manifold are next!