Fix for #31267 and additional zero-width constant bug #31326
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| // should be able to just discard it, since const expressions are guaranteed not to | ||
| // have side effects. This seems to be reached through tuple struct constructors being | ||
| // passed zero-size constants. | ||
| if let hir::ExprPath(..) = expr.node { |
There was a problem hiding this comment.
Hmm, it'd be nice to have an assertion here about the fact that the type is zero-sized.
|
@sdleffler you up for adding an assertion? I'm not sure what the best helper method is, but I can dig around if you want :) |
|
@nikomatsakis added one, should do the trick I think! |
|
@bors r+ Looks good! |
|
📌 Commit fb00e60 has been approved by |
After the truly incredible and embarrassing mess I managed to make in my last pull request, this should be a bit less messy. Fixes #31267 - with this change, the code mentioned in the issue compiles. Found and fixed another issue as well - constants of zero-size types, when used in ExprRepeats inside associated constants, were causing the compiler to crash at the same place as #31267. An example of this: ``` struct Bar; const BAZ: Bar = Bar; struct Foo([Bar; 1]); struct Biz; impl Biz { const BAZ: Foo = Foo([BAZ; 1]); } fn main() { let foo = Biz::BAZ; println!("{:?}", foo); } ``` However, I'm fairly certain that my fix for this is not as elegant as it could be. The problem seems to occur only with an associated constant of a tuple struct containing a fixed size array which is initialized using a repeat expression, and when the element to be repeated provided to the repeat expression is another constant which is of a zero-sized type. The fix works by looking for constants and associated constants which are zero-width and consequently contain no data, but for which rustc is still attempting to emit an LLVM value; it simply stops rustc from attempting to emit anything. By my logic, this should work fine since the only values that are emitted in this case (according to the comments) are for closures with side effects, and constants will never have side effects, so it's fine to simply get rid of them. It fixes the error and things compile fine with it, but I have a sneaking suspicion that it could be done in a far better manner. r? @nikomatsakis
After the truly incredible and embarrassing mess I managed to make in my last pull request, this should be a bit less messy.
Fixes #31267 - with this change, the code mentioned in the issue compiles.
Found and fixed another issue as well - constants of zero-size types, when used in ExprRepeats inside associated constants, were causing the compiler to crash at the same place as #31267. An example of this:
However, I'm fairly certain that my fix for this is not as elegant as it could be. The problem seems to occur only with an associated constant of a tuple struct containing a fixed size array which is initialized using a repeat expression, and when the element to be repeated provided to the repeat expression is another constant which is of a zero-sized type. The fix works by looking for constants and associated constants which are zero-width and consequently contain no data, but for which rustc is still attempting to emit an LLVM value; it simply stops rustc from attempting to emit anything. By my logic, this should work fine since the only values that are emitted in this case (according to the comments) are for closures with side effects, and constants will never have side effects, so it's fine to simply get rid of them. It fixes the error and things compile fine with it, but I have a sneaking suspicion that it could be done in a far better manner.
r? @nikomatsakis