crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.36k stars 1.62k forks source link

Struct#pretty_print Sorts Fields by Name #14653

Open stellarpower opened 4 months ago

stellarpower commented 4 months ago

Feature Request

Currently, Struct#pretty_print sorts the fields on name when printing if #inspect is not overridden.

I can therefore override inspect, or pretty_print, but this means copying in some boilerplate from Struct to my derived class.

I'd like a simpler way to specify if a Struct (or perhaps other classes too) should be pretty-printed in order of definition of the fields (seeing as this does represent the in-memory layout, and so has I think more genreal use as one form of printed representation), whilst leaving the default of sorting the names in place, as this will make sense for many other usecases.

It could be possible to include a one-word macro that flags the class, or something like that that imposes low overhead, and then when pretty_print is expanded, it can check if this flag is set to decide if it sorts or not. An alternative idea might be specifying further option to the global pretty-print macro/function/instance itself, similarly to how we can for indentation, but this I presume would be less popular - however it would decouple the representation from the ordering and allow a user to select one or the other without modifying the class.

Thanks

straight-shoota commented 4 months ago

(seeing as this does represent the in-memory layout, and so has I think more genreal use as one form of printed representation)

This assumption is not correct for regular types. There is no guarantee about the actual memory layout. For example, the compiler is allowed to pack fields in order to reduce overall size.

So I think it's generally a good idea to print struct fields ordered lexically to not insinuate a specific layout.

Memory layout only strictly follows the declaration order in lib structs or structs annotated with @[Extern]. Assuming you expect a specific memory layout, that's what you ought to be using, I guess?

Maybe for lib and extern structs - with explicit memory layout - the print order could represent the actual layout.

straight-shoota commented 4 months ago

I'm also quite confused that Struct#inspect and Struct#pretty_print have different ordering:

record Foo, bar = "bar", foo = "foo", baz = "baz"

p Foo.new  # => Foo(@bar="bar", @foo="foo", @baz="baz")
pp Foo.new # => Foo(@bar="bar", @baz="baz", @foo="foo")
stellarpower commented 4 months ago

As it happens I wasn't aware of lib structs as being different, so yes, presumably I wouldn't want to use the general one - in actual fact, I was using the BinData shard, and it must be packing things internally in the right order as it'd be of no use otherwise, and it seems the pretty printer for Struct was ultimately what was called, but I would have to look at the internals and see.

I guess slightly tangential, but also AFAIK ruby's methods etc. functions return things in the order that they are defined. Given that the Crystal macros return Arrays rather than hashes, I assumed therefore that there was a natural ordering to them, I don't know if there will be other cases where that might be desirable behaviour. Might also be useful for the programmer if they've ordered things in a logical way within their class, even if that doesn't necessarily match the memory layout. Wouldn't change the default but I think it's good to have as a possible option.

I guess the difference you see there is to do with the sort right(?) So inspect is just accessing instance_vars directly, whatever ordering that might be using, but as it's not overridden, pretty_print then sorts that array before processing it.

straight-shoota commented 4 months ago

Wouldn't change the default but I think it's good to have as a possible option.

Struct#inspect and Struct#pretty_print are meant as generic fallbacks in case no more specific solution exists. You always have the option of a custom implementation that caters to the needs of a specific type.

I guess the difference you see there is to do with the sort right(?) So inspect is just accessing instance_vars directly, whatever ordering that might be using, but as it's not overridden, pretty_print then sorts that array before processing it.

Yes, that's the difference.

stellarpower commented 4 months ago

Struct#inspect and Struct#pretty_print are meant as generic fallbacks in case no more specific solution exists. You always have the option of a custom implementation that caters to the needs of a specific type.

Right, but what I mean is, even if there's value in adding a standard way to list the fields in order of declaration, I'm not suggesting overriding the default behaviour of listing alphabetically, but just suggesting that this would be a useful feature to have as an additional option - and without hand-rolling for every struct/class it's used for. I.e. as an opt-in strategy for those who want to use it.

straight-shoota commented 4 months ago

I'm not convinced that enabling an explicit option to alter the stdlib implementation is really worth it. It's easy to copy the default implementation to your code and adapt it to your specific needs.