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

A somewhat radical proposal for Show #1675

Closed
hdgarrood opened this Issue Nov 27, 2015 · 31 comments

Comments

Projects
None yet
10 participants
@hdgarrood
Contributor

hdgarrood commented Nov 27, 2015

(Vaguely related to purescript/purescript-prelude#46.)

I think @jonsterling is on to something here (I recommend reading at least the first few replies as well): https://twitter.com/jonsterling/status/670091766549344257

I've felt vaguely uneasy about the Show type class for a long time and I think this hits the nail on the head. I think our current Show type class is unsuitable for debugging / use in the REPL (which ought to be its main aim) because:

  • Some types have no Show instance, for example, (->). This doesn't seem so bad, because what use would showing a function be anyway? But it can be irritating when you want to print larger structures which contain types like (->). In particular, at the moment, you can't use Generic; you have to write out Show instances which skip over the "unshowable" things yourself.
  • When you have a function that turns almost anything into a String, it's just too tempting to abuse it for serialization (as we have discovered in the Haskell community).
  • The more types that do have Show instances, the more dangerous show becomes; given that it works on almost anything, and that the return type is just String (rather than something that mentions the input type like pure, for example) it's very easy to call show on the wrong thing. It's almost like using a dynamic language!
  • Records can't have instances, even though there's no technical reason that they couldn't be printed for debugging.

@jonsterling suggests having print :: forall a. a -> Unit instead, similar to the existing purescript-debug library.

I think we could improve this situation significantly by having a compiler-supported Show replacement.

I propose that we add a type class to the compiler. Let's call it Debug (it wouldn't depend on purescript-foreign, this is just to illustrate):

class Debug a where
  debugShow :: a -> Foreign

Where the return type of debugShow is anything that is suitable to be passed to console.log. Sometimes this will just be a String, but often I think it's a good idea to leave other types as they are, as browsers can offer nicer UIs. Try console.log({foo: 1}) in Firefox or Chrome for instance.

This type class would be invisible to the user, if possible, but we also provide a function which is visible, something like print :: forall a. (Debug a) => a -> Unit, or perhaps closer to the types in purescript-debug, and which internally does console.log <<< debugShow before returning unit.

We add a special case in the compiler so that every type is an instance of Debug, including functions and records. Defining your own instances is not allowed. This way, we don't need to worry about orphans and all that (I think).

These instances would probably do something like:

  • If it's a record, iterate over the properties and call debugShow on each.
  • If it's an array, simply map debugShow.
  • If it's an ADT defined in PureScript code (ie data or newtype), do the same thing as the what the derived Show instance in Haskell would do.
  • Types defined via foreign import data could probably be left alone.
  • Functions could be left alone. Browsers already do a decent job of printing these. Alternatively, we could include the type: print id might produce "<Function: forall a. a -> a>".
    and so on.

If print occurs in your program, it generates a compiler warning, which helps you avoid accidentally leaving them in.

print could also include the current file name and line number.

I realise this is, to some extent, at odds with the current minimalistic approach of the compiler. But to me, it appears promising enough to justify bending the rules a little. And I think it would allow us to get rid of Show. This would not only make me happy not only because Show unnerves me, but also because it would take us most of the way to removing all the runtime dependencies of psci — If we also moved the Eff type into Prim then $PSCI.Support could have no library dependencies, which would resolve #1441.

@garyb garyb added the enhancement label Nov 27, 2015

@garyb garyb added this to the Ideas milestone Nov 27, 2015

@garyb

This comment has been minimized.

Member

garyb commented Nov 27, 2015

I like it. I think a conversion to String is still preferrable though, as if you're debugging you'll often want to output some words, not just a value, as it becomes very difficult to distinguish what log line refers to what otherwise. The fact that the string it generates would be useless for anything other than debugging and cannot be overridden avoids the serialization-temptation problem.

I'm not so sure converting-anything-to-String even counts as an effect then, so we can still use it with purescript-debug or something to actually print the message.

edit: rather, conversion-to-printable-value, perhaps Foreign or some opaque type is better than String, as you say.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Nov 27, 2015

You'll often want to output some words, not just a value

Maybe just creating a record would be a enough? Suppose you want to print x, y, and z, then:

print { x, y, z }

Or even

print { msg: "About to do something", x, y, z }
@garyb

This comment has been minimized.

Member

garyb commented Nov 27, 2015

That's true, or event just provide a print' which accepts a message and a value.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Dec 12, 2015

I've recently come across an instance where it would be useful to get the String, actually: when writing a testing framework. For example, if you were writing an assertEqual function, you'd want to be able to generate a message like "Expected X but got Y", and you would want more control than just being forced to print it immediately.

It's just a bit unfortunate that having a forall a. a -> String would break parametricity, as foo :: Array a -> Array a could then use this debugShow function to inspect its arguments, for instance.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Dec 12, 2015

Oh wait, no, you don't need to convert to String there - in that case you can instead hold on to an some type like Array (exists a. { expected :: a, actual :: a }), using a Writer monad or something, and print stuff in it whenever you want.

Although this specific use case does reveal another thing: if we had a print function which automatically included file name, line number, and column number, we'd probably want to be able to turn that off.

@sharkdp

This comment has been minimized.

Contributor

sharkdp commented Dec 12, 2015

I like your proposal. But what if I really want to convert a value to a String for the actual purpose of the program? How would I convert an Int or a Number to a String?

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Dec 12, 2015

We would add extra functions for those things, I guess. I would argue that they should be monomorphic.

@robotlolita

This comment has been minimized.

robotlolita commented Dec 12, 2015

A big 👍 from me on this. It would be nice if print (x -> x) gave you something like: x -> x :: forall a. a -> a

@sharkdp a ToString type class would be the best thing for that:

class ToString a where
  toString :: a -> Maybe String -- not all things can be converted

This is total and goes nicely with the reverse too:

class ParseFromString a where
  fromString :: String -> Maybe a

But using a specific serialisation format (like JSON or Transit or whatever) might be better suited for your use case. You can also create type classes for that. The problem with Show is just that there are no rules on "what should I output here?", so it's kind of a bad fit for all problems it could be used for.

@paf31

This comment has been minimized.

Member

paf31 commented Feb 7, 2016

Perhaps the way to implement this is to have move Show into Prim, and have the constraint solver insert the correct dictionary automatically during solving. This would lead to some code explosion, but it would be understood that Show is for debugging only. Maybe the name could be changed to reflect that.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Feb 7, 2016

Maybe the name could be changed

👍. Rust called theirs Debug which I think is probably as good a name as any.

@paf31

This comment has been minimized.

Member

paf31 commented Feb 7, 2016

I was going to suggest the same thing 😄

@garyb

This comment has been minimized.

Member

garyb commented Feb 7, 2016

Hah, that would have been my suggestion too.

@greglearns

This comment has been minimized.

greglearns commented Apr 12, 2016

I hope this is related: it seems impossible to log/trace/debug an object that doesn't have a Show. As a newbie to PureScript (but not programming), I just want to be able to show a result to make sure things are working, but I can't because it doesn't have a Show. It's killing me. In JavaScript, I can "console.log({ anything: true })" and it works, but I can't do that in PureScript.

@hdgarrood

This comment has been minimized.

@greglearns

This comment has been minimized.

greglearns commented Apr 12, 2016

Thank you!

@jonsterling

This comment has been minimized.

jonsterling commented Apr 12, 2016

@greglearns I also use Debug.Trace.traceAny, which is like logAny but it can be run outside of Eff.

@greglearns

This comment has been minimized.

greglearns commented Apr 12, 2016

@jonsterling even better! thanks!
I used it likes so (for newbs like me :-)

bower install --save purescript-debug

import Debug.Trace
fnName arg = traceAny arg $ \_ ->  <originalFunctioCode here>
@texastoland

This comment has been minimized.

texastoland commented Apr 13, 2016

@greglearns I think spy is even more useful.

I can't tell where the conversation ended up but I'd prefer the browser's rendering to a vanilla String. Moreover for a type like Set implemented using a balanced tree I most certainly would want a custom rendering. I have a similar pain showing List to beginners currently.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Apr 24, 2016

That's a good point. I guess we would need to allow user defined instances for types like Map and Set somehow.

@dkoontz

This comment has been minimized.

dkoontz commented May 3, 2016

Another 👍 . Not being able to print values is often a big hangup for me, especially when I try to demo something live to a co-worker. I like the Debug typeclass suggestion.

@rightfold

This comment has been minimized.

Contributor

rightfold commented May 4, 2016

I think the debug show shouldn't return a string but an AST. This has a couple advantages:

  • Parentheses can be inserted automatically (think operator precedence) when converting the AST into a string.
  • Can be pretty-printed in various ways, instead of having just one long line of unreadable text (especially (with (really large (and deeply)) (nested data) structures)).
    • GUI debuggers/IDEs may show a collapsible tree view when the tree is very big.
  • Assertion libraries could diff the trees to show exactly what parts are inequal.
@paf31

This comment has been minimized.

Member

paf31 commented May 4, 2016

The issue with automatically looking up Debug instances using a magic path in the typechecker is that you might have a function like

debuggable :: Debug a => a -> a
debuggable a = debug a a

What should the compiler generate there? It doesn't have enough type structure available to generate anything useful.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented May 4, 2016

@paf31 Sorry, I don't quite understand what the problem is there. Can we not generate the same code here that we would for a normal typeclass there?

Since @AppShipIt pointed out that some types (eg, Set) would need human-written Debug instances to be useful, I'm imagining a compiler pass that inserts derived instances for all types which don't have explicitly written ones, and hopefully then we might not need any additional magic?

@paf31

This comment has been minimized.

Member

paf31 commented May 4, 2016

So what's the condition for when we solve vs. derive? When the type is fully instantiated? Sometimes we need a combination of the two:

data Foo a = Foo a

debuggable :: Debug a => Foo a -> Foo a
debuggable a = debug a a

When the compiler sees this, it needs to know to reduce Debug (Foo a) to the subgoal Debug a by deriving a bit of the instance, and then continue. That might be okay though, there's probably a decent algorithm along those lines.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Jan 9, 2017

I've been thinking about this a bit more, and I think we can reduce the magicalness of my original proposal a bit and still retain usefulness. Instead of automatically deriving instances, I think it might be better to use the existing typeclass deriving mechanic where you have to ask for an instance by writing a derive instance declaration - particularly because you sometimes need a custom Debug instance, such as Map, so in that case it's clear what you need to do: just define the instance in the normal way. We would then say in the docs that every type should have a Debug instance, and leave it up to library authors to include them. We might also consider producing warning a whenever you define a type of kind Type which doesn't have a Debug instance.

Here's a concrete proposal for what could be added to Prim:

data Any :: Type
toAny :: forall a. a -> Any

data Debugged
  = Debugged String (Array Debugged)
  -- I'll explain this constructor in a moment
  | DebuggedAsIs Any String (Array Debugged)

class Debug a where
  toDebugged :: a -> Debugged

As @rightfold notes, we could then do handy things like putting parens in the right place, or printing to
a configurable depth, or comparing different values.

So for example we might define:

-- hopefully this is what the derived instance would look like
instance debugMaybe :: Debug a => Debug (Maybe a) where
  toDebugged (Just a) = Debugged "Just" [debug a]
  toDebugged Nothing = Debugged "Nothing" []

-- but this would need to be hand-written
instance debugMap :: (Debug k, Debug v) => Debug (Map k v) where
  toDebugged = Debugged "Map.fromFoldable" <<< Array.singleton <<< toUnfoldable

Where the debugMap instance is borrowing the Debug instance for an array of tuples.

The reason for having the DebugAsIs constructor is that when deciding whether to pass a value as-is to the runtime for displaying, you need to know whether all its children are ok to pass as-is too. For example, browsers are good at displaying arrays. If you have an Array Int, that's perfect; if you have an Array (Map k v), you need to do a bit more processing. I'm therefore imagining that we'd write a function like debug :: forall a. Debug a => a -> Unit which first calls toDebugged and then, if it sees a DebugAsIs constructor, looks at all the children to see if they can be printed as-is, and if so, just takes the Any field and gives it to console.log; if not, it ignores the Any field and prints it in the normal way using the other two fields.

Then for types that browsers are aware of, we would define instances which use the DebuggedAsIs constructor:

instance debugNumber :: Debug Number where
  toDebugged x = DebuggedAsIs x "Number" []

instant debugArray :: Debug a => Debug (Array a) where
  toDebugged xs = DebuggedAsIs xs "Array" (map debug xs)

On the magicalness front, I think it might be better not to try to give debug an 'invisible' Debug constraint, just to treat it as a normal type class member. Then we don't have to worry about breaking parametricity.

One issue I foresee is that if you're trying to debug polymorphic code, you might have to put a whole bunch of Debug constraints in temporarily to get it to compile. To address that we could have the compiler automatically insert those constraints and see if that works? If it does work after adding those constraints, I think we should output a warning, as you don't want the compiler to continue to insert these constraints after you've finished working on the issue.

We would also want to define a forall r. Debug (Record r) instance in Prim, although I suppose we would need to do something like @paf31's idea in one of the discussions about instances for records which was to provide an All constraint, like All :: (k -> Constraint) -> # k -> Constraint which asserts that you have an instance for every label in a row. So if we could define instance debugRecord :: (All Debug r) => Debug (Record r) that would be really nice. However if that's too difficult to do, I think we could leave it out, at least to start with.

One thing I'm not sure about: is it feasible to define a Debug (a -> b) instance which somehow obtains the type of the function and includes that in the result it produces at runtime if we're using this design?

@paf31

This comment has been minimized.

Member

paf31 commented Jan 9, 2017

@hdgarrood This isn't a full reply, but remember that we can't put functions, data types or classes with member functions into Prim, since those would require runtime support in the absence of Prelude.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Jan 9, 2017

Ah of course. Could we put it in Prelude then, except have the deriving built in to the compiler, like we do for Eq and Ord etc?

@paf31

This comment has been minimized.

Member

paf31 commented Jan 9, 2017

It wouldn't need to be in Prelude necessarily, but that would be preferable. As long as it's in core somewhere, I think we could derive it in the compiler. I'd also be fine with deriving Show itself if we can decide what we want the output to be and whether we need showsPrec or not.

@paf31

This comment has been minimized.

Member

paf31 commented Mar 21, 2017

What about just solving Show (Record r) automatically whenever r is closed? I'd be fine with that, since Show is really just for debugging anyway. I wouldn't want to do it for other classes though, since the generated code would be pretty unpleasant and involve some repetition.

Edit: we could even generate a warning telling the user that they used a generated Show instance.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Mar 21, 2017

I think that would be good, but I do worry that Show is too tempting to use for things other than debugging. I'd like to try to investigate and develop the idea of a new type class, similar to what I described in my most recent long-ish comment, although I won't be able to set aside time for that until at least the end of the semester, I think.

@hdgarrood

This comment has been minimized.

Contributor

hdgarrood commented Dec 13, 2017

Having RowToList and Warn changes this quite drastically: I actually don't think we need much magic at all to achieve this now. I'm attempting to do as much as I can in "userland" now. I'm going to close this for now; maybe I'll reopen once I've made some progress.

Edit: if you want to see what I've done, I've made a start at https://github.com/hdgarrood/purescript-debugged.

@hdgarrood hdgarrood closed this Dec 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment