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

Marking PlutusTx.Builtins.Internal -O is overkill #4194

Closed
treeowl opened this issue Nov 5, 2021 · 3 comments
Closed

Marking PlutusTx.Builtins.Internal -O is overkill #4194

treeowl opened this issue Nov 5, 2021 · 3 comments
Labels
enhancement Low priority Doesn't require immediate attention status: triaged

Comments

@treeowl
Copy link

treeowl commented Nov 5, 2021

Currently,

-- This ensures that we don't put *anything* about these functions into the interface
-- file, otherwise GHC can be clever about the ones that are always error, even though
-- they're NOINLINE!
{-# OPTIONS_GHC -O0 #-}

Now I don't know what the problem is with GHC being clever like that (that should be documented!), but this is overkill regardless. As far as I can tell, the only thing that's always error is error :: BuiltinUnit -> a. Everything else in the module should ideally be analyzed properly. I think the right thing is to just tell GHC exactly what we mean.

error :: BuiltinUnit -> a
error (BuiltinUnit ()) = GHC.Exts.lazy (mustBeReplaced "error")

Note: in combination with #4193, we'll then want to use lazy in the BuiltinByteString functions so GHC sees those as strict in their BuiltinByteString arguments but not in the underlying ByteStrings.

@michaelpj
Copy link
Contributor

Yes, it's a blunt hammer, but it works. What we really want is ghc-proposals/ghc-proposals#415 (for essentially the same reason as the Clash folks want it).

@effectfully
Copy link
Contributor

Yes, it's a blunt hammer, but it works.

Given this, I'm marking this issue with "Low priority".

@effectfully effectfully added Low priority Doesn't require immediate attention status: triaged labels Feb 1, 2023
@effectfully
Copy link
Contributor

I think it's fine to have -O0 in there, so rather than spending time investigating this issue, I'm going to close it instead. Feel free to reopen if you have a strong feeling about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Low priority Doesn't require immediate attention status: triaged
Projects
None yet
Development

No branches or pull requests

3 participants