-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Fix various aspects around let
bindings inside const functions
#56160
Changes from 20 commits
4d2bed9
dba5ba0
df2123c
52b67b1
ef38afc
59c6c49
7ec3c10
d62bcad
507ea97
75ce28a
d8ece18
866664c
8937faa
16d2a92
ac47bd7
25d1c07
172b428
42e5317
9d2f97b
e6e08c6
07345f0
2cb5e3d
6ca2ad5
b678238
d815e2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -372,13 +372,21 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, | |
// FIXME(eddyb) use logical ops in constants when | ||
// they can handle that kind of control-flow. | ||
(hir::BinOpKind::And, hir::Constness::Const) => { | ||
cx.control_flow_destroyed.push(( | ||
op.span, | ||
"`&&` operator".into(), | ||
)); | ||
ExprKind::Binary { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Woah. I don't think I realized we do this sort of conversion. |
||
op: BinOp::BitAnd, | ||
lhs: lhs.to_ref(), | ||
rhs: rhs.to_ref(), | ||
} | ||
} | ||
(hir::BinOpKind::Or, hir::Constness::Const) => { | ||
cx.control_flow_destroyed.push(( | ||
op.span, | ||
"`||` operator".into(), | ||
)); | ||
ExprKind::Binary { | ||
op: BinOp::BitOr, | ||
lhs: lhs.to_ref(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -243,7 +243,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { | |
return; | ||
} | ||
|
||
if self.tcx.features().const_let { | ||
if self.const_let_allowed() { | ||
let mut dest = dest; | ||
let index = loop { | ||
match dest { | ||
|
@@ -320,6 +320,10 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { | |
} | ||
} | ||
|
||
fn const_let_allowed(&self) -> bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a great place for a comment =) in particular, documenting the logic behind these rules. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like: Currently, without a feature gate, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We decided on not doing this dual-scheme. Instead we're poisoning constants that use short circuiting operators so they can't also have I'll remove the function and move back to checking the feature gate directly |
||
self.tcx.features().const_let | ||
} | ||
|
||
/// Qualify a whole const, static initializer or const fn. | ||
fn qualify_const(&mut self) -> (Qualif, Lrc<BitSet<Local>>) { | ||
debug!("qualifying {} {:?}", self.mode, self.def_id); | ||
|
@@ -357,7 +361,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { | |
TerminatorKind::FalseUnwind { .. } => None, | ||
|
||
TerminatorKind::Return => { | ||
if !self.tcx.features().const_let { | ||
if !self.const_let_allowed() { | ||
// Check for unused values. This usually means | ||
// there are extra statements in the AST. | ||
for temp in mir.temps_iter() { | ||
|
@@ -464,7 +468,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | |
LocalKind::ReturnPointer => { | ||
self.not_const(); | ||
} | ||
LocalKind::Var if !self.tcx.features().const_let => { | ||
LocalKind::Var if !self.const_let_allowed() => { | ||
if self.mode != Mode::Fn { | ||
emit_feature_err(&self.tcx.sess.parse_sess, "const_let", | ||
self.span, GateIssue::Language, | ||
|
@@ -552,7 +556,11 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | |
this.super_place(place, context, location); | ||
match proj.elem { | ||
ProjectionElem::Deref => { | ||
this.add(Qualif::NOT_CONST); | ||
if context.is_mutating_use() { | ||
this.not_const() | ||
} else { | ||
this.qualif = Qualif::NOT_CONST; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentionally overriding all other There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is an accidental leftover from the larger refactorings. I undid the change and added comments |
||
} | ||
let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx); | ||
match this.mode { | ||
Mode::Fn => {}, | ||
|
@@ -1158,8 +1166,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | |
if let (Mode::ConstFn, &Place::Local(index)) = (self.mode, dest) { | ||
if self.mir.local_kind(index) == LocalKind::Var && | ||
self.const_fn_arg_vars.insert(index) && | ||
!self.tcx.features().const_let { | ||
|
||
!self.const_let_allowed() { | ||
// Direct use of an argument is permitted. | ||
match *rvalue { | ||
Rvalue::Use(Operand::Copy(Place::Local(local))) | | ||
|
@@ -1170,7 +1177,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | |
} | ||
_ => {} | ||
} | ||
|
||
// Avoid a generic error for other uses of arguments. | ||
if self.qualif.contains(Qualif::FN_ARGUMENT) { | ||
let decl = &self.mir.local_decls[index]; | ||
|
@@ -1329,6 +1335,34 @@ impl MirPass for QualifyAndPromoteConstants { | |
// Do the actual promotion, now that we know what's viable. | ||
promote_consts::promote_candidates(mir, tcx, temps, candidates); | ||
} else { | ||
if !mir.control_flow_destroyed.is_empty() { | ||
let mut locals = mir.vars_iter(); | ||
if let Some(local) = locals.next() { | ||
let span = mir.local_decls[local].source_info.span; | ||
let mut error = tcx.sess.struct_span_err( | ||
span, | ||
&format!( | ||
"new features like let bindings are not permitted in {} \ | ||
which also use short circuiting operators", | ||
mode, | ||
), | ||
); | ||
for (span, kind) in mir.control_flow_destroyed.iter() { | ||
error.span_note( | ||
*span, | ||
&format!("use of {} here", kind), | ||
); | ||
} | ||
for local in locals { | ||
let span = mir.local_decls[local].source_info.span; | ||
error.span_note( | ||
span, | ||
"more locals defined here", | ||
); | ||
} | ||
error.emit(); | ||
} | ||
} | ||
let promoted_temps = if mode == Mode::Const { | ||
// Already computed by `mir_const_qualif`. | ||
const_promoted_temps.unwrap() | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
error[E0080]: could not evaluate static initializer | ||
error[E0019]: static contains unimplemented expression type | ||
--> $DIR/assign-to-static-within-other-static-2.rs:27:5 | ||
| | ||
LL | *FOO.0.get() = 5; //~ ERROR could not evaluate static initializer | ||
| ^^^^^^^^^^^^^^^^ tried to modify a static's initial value from another static's initializer | ||
LL | *FOO.0.get() = 5; //~ ERROR contains unimplemented expression type | ||
| ^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0080`. | ||
For more information about this error, try `rustc --explain E0019`. |
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.
cc @nikomatsakis
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.
I believe this is not the correct reason. The reason we add
FakeRead
is documented onFakeReadCause::ForLet
-- basically it is to ensure that we give errors in some cases where we generate more optimized IR (not just for diagnostics).