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

Add flag to dump PIR ASTs for certifier #6797

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jaccokrijnen
Copy link
Contributor

Context

For my certifier in Coq, I need to get my hands on all intermediate PIR ASTs. I've added a plugin flag dump-cert-trace, that dumps all PIR ASTs to a single text file during a compilation.

Dumping to file

To prevent keeping all intermediate ASTs in memory, I'm dumping ASTs directly after a pass has run (rather than collecting a pure list of ASTs and dumping them at the end of the pipeline). To that end, I've added a parameter dumpCert :: Text -> m () to runPass (in the pass abstraction), which can be used to append text to the dump file. It needs to be of type Text -> m () rather than Text -> IO (), since the monad stack m does not necessarily have IO in it (the pass abstraction does not require it). The concrete monad stack in Compiler.hs does have IO, so it can be lifted into m.

As a consequence, this new argument also shows up in functions like PIR.compileProgram, PIR.compileToReadable, PIR.compileReadableToPlc. This feels very clumsy, but I couldn't find a nice way to do it. Here are two ideas:

  • The dump function could go in CompilationCtx as a Text -> IO (), but that requires a MonadIO constraint in Compiling type and the runPass function in order to actually run it. I'm not sure if that is desired.
  • Similar to how logging works (which uses traceM), use unsafePerformIO and add a "pure" function Monad m => Text -> m () in the CompilationCtx

Any other suggestions?

Pretty printing ASTs

The dumping format is textual, and easily parseable in the proof assistant. It's almost Show, but without record syntax and uses Text instead of String for performance. I've added a new typeclass SimpleShow in module Text.SimpleShow, which has some basic instances and the possibility for deriving using GHC generics. I've also written/derived instances for all PIR AST types.

I've added the SimpleShow constraints to the Compiling type/constraint synonym, this required me turning on the following extensions in some places:

  • QuantifiedConstraints (because of the constraint forall t. SimpleShow (uni t))
  • ImpredicativeTypes, to add the constraint forall t. SimpleShow (uni t) to the Compiling type synonym.

Pretty printing pass information

In addition to ASTs I'm also dumping information about which pass was run. Here I've added the PassId type, which is used as an additional field for every basic Pass constructor.

This adds a plugin flag dump-cert-trace, which if enabled dumps all PIR ASTs
to a .cert_trace file, which can be processed by the WIP coq certifier
(plutus-cert).
@Unisay Unisay requested a review from a team January 21, 2025 14:07
Copy link
Contributor

@effectfully effectfully left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like SimpleShow. I don't think we need field selectors in Show output, so let's just get rid of them. Which ones do we have?

If for some reason we do need field selectors, then let's use TextShow.Generic instead of rolling out our own inefficient way of doing the same. So we can always do deriving Show Blah via AsTextNoFields Blah for a custom AsTextNoFields newtype-wrapper whose Show instance uses TextShow.Generic.

instance (forall a. SimpleShow (uni a)) => SimpleShow (SomeTypeIn uni) where
simpleShow (SomeTypeIn x) = parens True (simpleShow x)

instance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please move these two instance to Text.SimpleShow instead? This module is intended to be as Plutus-unspecific as possible. It'll probably become its own library eventually like it happened with prettyprinter-configurable.

{-# LANGUAGE TupleSections #-}
{-# LANGUAGE TypeApplications #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE ImpredicativeTypes #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the pragma.

@@ -1,5 +1,6 @@
-- editorconfig-checker-disable-file
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE ImpredicativeTypes #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here. And everywhere where it's not needed.

simpleShow' U1 = ""

instance (SimpleShow' f, SimpleShow' g) => SimpleShow' (f :*: g) where
simpleShow' (x :*: y) = simpleShow' x <> " " <> simpleShow' y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet Text.pack . show is more efficient than this entire machinery, because <> is linear (for both Text and String) and the regular Show is difference-list-based to account for that, unlike your approach.

But why optimize Show at all? You're gonna dump all that stuff on the disk, IO is gonna be your bottleneck, not list creation.

And if you do want to optimize Show, we have configurable pretty-printing specifically to be able to pretty-print things differently, with Doc having O(1) <>.

So SimpleShow is a lot of additional complexity for what's likely negative benefit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants