Skip to content

Commit 272aaea

Browse files
RenjiSannRenjiSann
committed
coverage: Prepare MIR boolean expression building for condition coverage
1 parent d1b5c3f commit 272aaea

File tree

1 file changed

+83
-35
lines changed
  • compiler/rustc_mir_build/src/build/expr

1 file changed

+83
-35
lines changed

‎compiler/rustc_mir_build/src/build/expr/into.rs‎

Lines changed: 83 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -148,43 +148,91 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
148148
ExprKind::LogicalOp { op, lhs, rhs } => {
149149
let condition_scope = this.local_scope();
150150
let source_info = this.source_info(expr.span);
151-
// We first evaluate the left-hand side of the predicate ...
152-
let (then_block, else_block) =
153-
this.in_if_then_scope(condition_scope, expr.span, |this| {
154-
this.then_else_break(
155-
block,
156-
lhs,
157-
Some(condition_scope), // Temp scope
158-
source_info,
159-
// This flag controls how inner `let` expressions are lowered,
160-
// but either way there shouldn't be any of those in here.
161-
true,
162-
)
163-
});
164-
let (short_circuit, continuation, constant) = match op {
165-
LogicalOp::And => (else_block, then_block, false),
166-
LogicalOp::Or => (then_block, else_block, true),
151+
152+
let push_constant_bool = |this: &mut Builder<'a, 'tcx>, bb, dest, value| {
153+
this.cfg.push_assign_constant(
154+
bb,
155+
source_info,
156+
dest,
157+
ConstOperand {
158+
span: expr.span,
159+
user_ty: None,
160+
const_: Const::from_bool(this.tcx, value),
161+
},
162+
);
167163
};
168-
// At this point, the control flow splits into a short-circuiting path
169-
// and a continuation path.
170-
// - If the operator is `&&`, passing `lhs` leads to continuation of evaluation on `rhs`;
171-
// failing it leads to the short-circuting path which assigns `false` to the place.
172-
// - If the operator is `||`, failing `lhs` leads to continuation of evaluation on `rhs`;
173-
// passing it leads to the short-circuting path which assigns `true` to the place.
174-
this.cfg.push_assign_constant(
175-
short_circuit,
176-
source_info,
177-
destination,
178-
ConstOperand {
179-
span: expr.span,
180-
user_ty: None,
181-
const_: Const::from_bool(this.tcx, constant),
182-
},
183-
);
184-
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
164+
// A simple optimization on boolean expression with short-circuit
165+
// operators is to not create a branch for the last operand.
166+
// Example:
167+
// let x: bool = a && b;
168+
// would be compiled into something semantically closer to
169+
// let x = if a { b } else { false };
170+
// rather than
171+
// let x = if a && b { true } else { false };
172+
//
173+
// In case `a` is true, evaluate `b` and assign it to `x`,
174+
// thus there is no need to create an actual branch for `b`.
175+
// Otherwise, assign false to `x`.
176+
//
177+
// The exception is when we instrument the code for condition coverage,
178+
// which tracks the outcome of all operands of boolean expressions.
179+
180+
// FIXME(dprn): change true with correct cli flag check
181+
let (outcome1, outcome2) = if this.tcx.sess.instrument_coverage_condition() {
182+
// We first evaluate the left-hand side of the predicate ...
183+
let (then_block, else_block) =
184+
this.in_if_then_scope(condition_scope, expr.span, |this| {
185+
this.then_else_break(
186+
block,
187+
expr_id,
188+
Some(condition_scope), // Temp scope
189+
source_info,
190+
// This flag controls how inner `let` expressions are lowered,
191+
// but either way there shouldn't be any of those in here.
192+
true,
193+
)
194+
});
195+
196+
// Write true on expression success...
197+
push_constant_bool(this, then_block, destination, true);
198+
// ...and false on failure.
199+
push_constant_bool(this, else_block, destination, false);
200+
201+
(then_block, else_block)
202+
} else {
203+
// We first evaluate the left-hand side of the predicate ...
204+
let (then_block, else_block) =
205+
this.in_if_then_scope(condition_scope, expr.span, |this| {
206+
this.then_else_break(
207+
block,
208+
lhs,
209+
Some(condition_scope), // Temp scope
210+
source_info,
211+
// This flag controls how inner `let` expressions are lowered,
212+
// but either way there shouldn't be any of those in here.
213+
true,
214+
)
215+
});
216+
let (short_circuit, continuation, constant) = match op {
217+
LogicalOp::And => (else_block, then_block, false),
218+
LogicalOp::Or => (then_block, else_block, true),
219+
};
220+
// At this point, the control flow splits into a short-circuiting path
221+
// and a continuation path.
222+
// - If the operator is `&&`, passing `lhs` leads to continuation of evaluation on `rhs`;
223+
// failing it leads to the short-circuting path which assigns `false` to the place.
224+
// - If the operator is `||`, failing `lhs` leads to continuation of evaluation on `rhs`;
225+
// passing it leads to the short-circuting path which assigns `true` to the place.
226+
push_constant_bool(this, short_circuit, destination, constant);
227+
228+
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
229+
230+
(short_circuit, rhs)
231+
};
232+
185233
let target = this.cfg.start_new_block();
186-
this.cfg.goto(rhs, source_info, target);
187-
this.cfg.goto(short_circuit, source_info, target);
234+
this.cfg.goto(outcome1, source_info, target);
235+
this.cfg.goto(outcome2, source_info, target);
188236
target.unit()
189237
}
190238
ExprKind::Loop { body } => {

0 commit comments

Comments
 (0)