Skip to content

Use pattern matching to make Debug impl for Cfg more robust#3767

Merged
djc merged 1 commit intomasterfrom
cfg-debug
Apr 9, 2024
Merged

Use pattern matching to make Debug impl for Cfg more robust#3767
djc merged 1 commit intomasterfrom
cfg-debug

Conversation

@djc
Copy link
Copy Markdown
Contributor

@djc djc commented Apr 9, 2024

Follow-up from #3763:

Its nice to have the compile times improved (potentially - not measured), but this increases cognitive load when changes to the struct are made :(, so I'd really rather we solved that by choosing a non-syn backed crate, rather than removing the proc-macro entirely.

This change should help reduce cognitive overload when changes to the struct are made.

For me, crates like derivative also have cognitive load because their exact workings are kind of magic -- compared to a few lines of code doing a manual impl, that doesn't seem like a good deal to me.

@djc djc requested review from rami3l and rbtcollins April 9, 2024 18:51
@rbtcollins
Copy link
Copy Markdown
Contributor

Thank you, this does address much of my concern. It will still require manual effort but at least it won't require thinking ;).

I wish Rust had a better story for compile time code execution, because I do totally get the its-magic aspect particularly for troubleshooting issues that arise when the generators go wrong.

OTOH, it is very convenient, and I've learned to embrace the magic much of the time :)

@djc
Copy link
Copy Markdown
Contributor Author

djc commented Apr 9, 2024

Don't get me wrong, I love my procedural macros. I just think that all of the little derive utility crates generally add very limited value over manual implementations, which are pretty straightforward and entirely "in view".

@djc djc enabled auto-merge April 9, 2024 18:59
@djc djc added this pull request to the merge queue Apr 9, 2024
Merged via the queue into master with commit 49ddaae Apr 9, 2024
@djc djc deleted the cfg-debug branch April 9, 2024 19:51
@rami3l rami3l added this to the 1.27.1 milestone Apr 14, 2024
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.

3 participants