Fix #15502 by not casting to Int during TH conversion
ClosedPublic

Authored by RyanGlScott on Aug 22 2018, 1:01 PM.

Details

Summary

When turning an IntegerL to an IntegralLit during TH
conversion, we were stupidly casting an Integer to an Int in
order to determine how it should be pretty-printed. Unsurprisingly,
this causes problems when the Integer doesn't lie within the bounds
of an Int, as demonstrated in Trac #15502.

The fix is simple: don't cast to an Int.

Test Plan

make test TEST=T15502

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
RyanGlScott created this revision.Aug 22 2018, 1:01 PM
RyanGlScott added inline comments.Aug 22 2018, 1:02 PM
compiler/basicTypes/BasicTypes.hs
1468–1475

Note that there's a similar phenomenon happening in mkFractionalLit, where we cast a Real thing to a Double before show-ing it. I'm not sure if this should be classified as a bug or not, though.

simonpj accepted this revision.Aug 22 2018, 3:11 PM
simonpj added a subscriber: simonpj.

Great!

compiler/basicTypes/BasicTypes.hs
1468–1475

I'd add a comment pointing this out, and citing the ticket.

But why not at least define r_rational = toRational r and use r_rational in both the fl_value field and the fl_text field? That would at least keep the two in sync.

Maybe it'd be funny to print these (usually) floats as rationals though?

This revision is now accepted and ready to land.Aug 22 2018, 3:11 PM
RyanGlScott marked 2 inline comments as done.
  • Explain why mkFractionalLit is the way it is
compiler/basicTypes/BasicTypes.hs
1468–1475

Maybe it'd be funny to print these (usually) floats as rationals though?

My thoughts exactly. We're doing all this for the sake of displaying things neatly for users' consumption (in -ddump-splices and elsewhere), so while converting to Rational would technically preserve the most accuracy in all cases, it also has the distinct downside that it makes the -ddump-splices output harder to read. In light of this, I think I'll just leave this code the way it is unless someone complains.

I've added a comment to this effect in mkFractionalLit.

> I think I'll just leave this code the way it is unless someone complains.

OK

compiler/basicTypes/BasicTypes.hs
1473

add c.f. mkIntegralLit.

RyanGlScott marked 2 inline comments as done.
  • Reference mkIntegralLit
This revision was automatically updated to reflect the committed changes.