
#15696: Derived Ord instance for enumerations with more than 8 elements seems to be incorrect -------------------------------------+------------------------------------- Reporter: mrkkrp | Owner: osa1 Type: bug | Status: patch Priority: highest | Milestone: 8.6.2 Component: Compiler | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #14677, #15155 | Differential Rev(s): Phab:D5196, Wiki Page: | Phab:D5201 -------------------------------------+------------------------------------- Comment (by simonpj): It's tricky, I agree; and I agree that's the result that `exprOkForSpeculation` should return False in these cases. But as I say in item (6) of comment:60, `app_ok` already has a special case for `SeqOp` for this very reason, and I think we should just extend that to `DataToTagOp`. In fact I got as far as writing the Note to accompany the change to `app_ok`: {{{ Note [PrimOps that evaluate their arguments] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Most primops to not evaluate their arguments, even lifted arguments. But two do: DataToTagOp and SeqOp (rembember the latter is monadic; it is not the primop corresponding to 'seq'). Now, is (dataToTag# x) ok-for-speculation? You might say "yes, if x is ok-for-speculation", because then it'll obey the rules for ok-for-speculation. But it's very fragile. Consider: \z. case x of y { DEFAULT -> let v = dataToTag# y in ... } This looks OK: we ask if 'y' is ok-for-spec, and say yes because it is evaluated. But if we do the binder-swap operation (which happens in FloatOut) we have \z. case x of y { DEFAULT -> let v = dataToTag# x in ... } and now it is /not/ ok-for-spec. This becomes even clearer if we float it to give let v = dataToTag# x in \z. case x of y { DEFAULT -> ... } Conclusion: always return False for ok-to-spec on SeqOp and DataToTagOp. }}} Does that makes sense? The can-fail thing is a hack, and we don't do it for `SeqOp`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15696#comment:69 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler