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

v0: replace skip_* methods with print_* methods in a "skip printing" mode. #53

Merged
merged 3 commits into from
Jul 20, 2021

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jul 20, 2021

My motivation for this change is that I wanted to implement structural constant value demangling (for rust-lang/rust#87194) and having to implement both skip_const and print_const for the new constant values seemed redundant, and also potentially risky (i.e. the two could go out of sync).

FWIW, the C demangler I wrote for libiberty just doesn't have this problem, as it took the "skip printing" route from the start, and I believe other implementations do something similar as well.

While the print_* methods may be less efficient when the out field is None, than the old skip_* implementation, I don't think the performance difference warrants dealing with the maintenance burden of duplication.

Worst case, if performance does become a concern, we can make Printer generic over a type that provides the Option<&mut fmt::Formatter> through a method, which could let LLVM optimize print_* methods to be more or less the same as the old skip_* methods (for Printer<SkipFmt>).

@alexcrichton alexcrichton merged commit d860281 into rust-lang:main Jul 20, 2021
@alexcrichton
Copy link
Member

Seems reasonable to me!

@eddyb eddyb deleted the v0-simpler-skip-printing branch July 20, 2021 21:15
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