-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Avoid stack overflow when printing unevaluated const #72341
Conversation
Fixes rust-lang#68104 When printing the DefPath for an unevaluated const, we may end up trying to print the same const again. To avoid infinite recursion, we use the 'verbose' format when we try to re-entrantly print a const. This ensures that the pretty-printing process will always terminate.
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
// Braces around const expression causes crash | ||
impl Num<{5}> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The braces shouldn't matter, I wonder if this is a bug (cc @varkor).
What you want is to use an expression, like 2+3
, so that it definitely requires evaluation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently don't look into blocks for literals, It seems like we only do it for params at the moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will get a PR ready which cleans this up.
edit: we may not actually want this for now, as being more aggressive here may mask some otherwise found issues with ConstKind::Unevaluated
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need braces here: arbitrary expressions should work, and {5}
is an expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The braces are needed to trigger the original ICE, so I included them in the test
// const. This ensures that we never end up trying to recursively | ||
// print a const that we're already printing. | ||
// See issue #68104 for more details | ||
self = with_no_const_path(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try with_forced_impl_filename_line
instead? It should also work since I believe the problem here is the pretty-printing of impl
paths.
Wait. Hang on. Why would this ever print paths?!
The bug is probably in metadata encoding/decoding. AnonConst
s shouldn't turn into DefKind::Const
.
Closing in favor of #72600 as per @eddyb's comment: #72341 (comment) |
Fixes #68104
When printing the DefPath for an unevaluated const, we may end up trying
to print the same const again. To avoid infinite recursion, we use the
'verbose' format when we try to re-entrantly print a const. This ensures
that the pretty-printing process will always terminate.