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

API: Rename QuestionMarkExpr -> TryExpr (Breaking change) #241

Merged
merged 2 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions marker_api/src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub enum ExprKind<'ast> {
UnaryOp(&'ast UnaryOpExpr<'ast>),
Ref(&'ast RefExpr<'ast>),
BinaryOp(&'ast BinaryOpExpr<'ast>),
QuestionMark(&'ast QuestionMarkExpr<'ast>),
Try(&'ast TryExpr<'ast>),
Assign(&'ast AssignExpr<'ast>),
As(&'ast AsExpr<'ast>),
Path(&'ast PathExpr<'ast>),
Expand Down Expand Up @@ -180,7 +180,7 @@ pub enum ExprPrecedence {
Fn = 0x1000_0000,
Index = 0x1000_0001,

QuestionMark = 0x0F00_0000,
Try = 0x0F00_0000,

/// The unary `-` operator
Neg = 0x0E00_0000,
Expand Down Expand Up @@ -249,7 +249,7 @@ macro_rules! impl_expr_kind_fn {
impl_expr_kind_fn!((ExprKind) $method() -> $return_ty,
IntLit, FloatLit, StrLit, CharLit, BoolLit,
Block, Closure,
UnaryOp, Ref, BinaryOp, QuestionMark, As, Assign,
UnaryOp, Ref, BinaryOp, Try, As, Assign,
Path, Index, Field,
Call, Method,
Array, Tuple, Ctor, Range,
Expand Down Expand Up @@ -384,7 +384,7 @@ mod test {
assert_eq!(40, size_of::<UnaryOpExpr<'_>>(), "UnaryOpExpr<'_>");
assert_eq!(40, size_of::<RefExpr<'_>>(), "RefExpr<'_>");
assert_eq!(56, size_of::<BinaryOpExpr<'_>>(), "BinaryOpExpr<'_>");
assert_eq!(32, size_of::<QuestionMarkExpr<'_>>(), "QuestionMarkExpr<'_>");
assert_eq!(32, size_of::<TryExpr<'_>>(), "TryExpr<'_>");
assert_eq!(80, size_of::<AssignExpr<'_>>(), "AssignExpr<'_>");
assert_eq!(48, size_of::<AsExpr<'_>>(), "AsExpr<'_>");
assert_eq!(96, size_of::<PathExpr<'_>>(), "PathExpr<'_>");
Expand Down
42 changes: 38 additions & 4 deletions marker_api/src/ast/expr/op_exprs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,23 +142,57 @@ impl<'ast> RefExpr<'ast> {
}
}

/// The `?` operator that unwraps valid values or propagates erroneous values to
/// the the calling function.
///
/// Here is an example of the operator:
///
/// ```
/// fn try_option_example(opt: Option<i32>) -> Option<String> {
/// // The `?` operator unwraps the value if `opt` is `Some` or
/// // propagates `None` if `opt` is empty.
/// // v
/// let value = opt?;
/// // `value` has the type `i32`
///
/// // [...]
/// # Some(value.to_string())
/// }
///
/// fn try_result_example(res: Result<i32, ()>) -> Result<String, ()> {
/// // The `?` operator unwraps the value if `res` is `Ok` or
/// // propagates the value of `Err` if `res` is an error.
/// // v
/// let value = res?;
/// // `value` has the type `i32`
///
/// // [...]
/// # Ok(value.to_string())
/// }
/// ```
///
/// This operator is also known as the *question mark* or *error propagation* operator.
///
/// See <https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator>
#[repr(C)]
#[derive(Debug)]
pub struct QuestionMarkExpr<'ast> {
pub struct TryExpr<'ast> {
data: CommonExprData<'ast>,
expr: ExprKind<'ast>,
}

impl<'ast> QuestionMarkExpr<'ast> {
impl<'ast> TryExpr<'ast> {
/// The expression that might produce an error, that would be propagated by
/// this operator.
pub fn expr(&self) -> ExprKind<'ast> {
self.expr
}
}

super::impl_expr_data!(QuestionMarkExpr<'ast>, QuestionMark);
super::impl_expr_data!(TryExpr<'ast>, Try);

#[cfg(feature = "driver-api")]
impl<'ast> QuestionMarkExpr<'ast> {
impl<'ast> TryExpr<'ast> {
pub fn new(data: CommonExprData<'ast>, expr: ExprKind<'ast>) -> Self {
Self { data, expr }
}
Expand Down
12 changes: 6 additions & 6 deletions marker_rustc_driver/src/conversion/marker/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use marker_api::{
ArrayExpr, AsExpr, AssignExpr, AwaitExpr, BinaryOpExpr, BinaryOpKind, BlockExpr, BoolLitExpr, BreakExpr,
CallExpr, CaptureKind, CharLitExpr, ClosureExpr, ClosureParam, CommonExprData, ConstExpr, ContinueExpr,
CtorExpr, CtorField, ExprKind, ExprPrecedence, FieldExpr, FloatLitExpr, FloatSuffix, ForExpr, IfExpr,
IndexExpr, IntLitExpr, IntSuffix, LetExpr, LoopExpr, MatchArm, MatchExpr, MethodExpr, PathExpr,
QuestionMarkExpr, RangeExpr, RefExpr, ReturnExpr, StrLitData, StrLitExpr, TupleExpr, UnaryOpExpr,
UnaryOpKind, UnstableExpr, WhileExpr,
IndexExpr, IntLitExpr, IntSuffix, LetExpr, LoopExpr, MatchArm, MatchExpr, MethodExpr, PathExpr, RangeExpr,
RefExpr, ReturnExpr, StrLitData, StrLitExpr, TryExpr, TupleExpr, UnaryOpExpr, UnaryOpKind, UnstableExpr,
WhileExpr,
},
pat::PatKind,
Ident, Safety, Syncness,
Expand Down Expand Up @@ -210,7 +210,7 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
ExprKind::Match(self.alloc(MatchExpr::new(data, self.to_expr(scrutinee), self.to_match_arms(arms))))
},
hir::ExprKind::Match(_scrutinee, [_early_return, _continue], hir::MatchSource::TryDesugar(_)) => {
ExprKind::QuestionMark(self.alloc(self.to_try_expr_from_desugar(expr)))
ExprKind::Try(self.alloc(self.to_try_expr_from_desugar(expr)))
},
hir::ExprKind::Match(_scrutinee, [_awaitee_arm], hir::MatchSource::AwaitDesugar) => {
ExprKind::Await(self.alloc(self.to_await_expr_from_desugar(expr)))
Expand Down Expand Up @@ -552,10 +552,10 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
}
}

fn to_try_expr_from_desugar(&self, try_desugar: &hir::Expr<'tcx>) -> QuestionMarkExpr<'ast> {
fn to_try_expr_from_desugar(&self, try_desugar: &hir::Expr<'tcx>) -> TryExpr<'ast> {
if let hir::ExprKind::Match(scrutinee, [_ret, _con], hir::MatchSource::TryDesugar(_)) = try_desugar.kind {
if let hir::ExprKind::Call(_try_path, [tested_expr]) = scrutinee.kind {
return QuestionMarkExpr::new(
return TryExpr::new(
CommonExprData::new(self.to_expr_id(try_desugar.hir_id), self.to_span_id(try_desugar.span)),
self.to_expr(tested_expr),
);
Expand Down
8 changes: 4 additions & 4 deletions marker_uilints/tests/ui/print_cond_expr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -1018,8 +1018,8 @@ warning: print test
48 | let _print_option_match = x?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: QuestionMark(
QuestionMarkExpr {
= note: Try(
TryExpr {
data: CommonExprData {
_lifetime: PhantomData<&()>,
id: ExprId(..),
Expand Down Expand Up @@ -1063,8 +1063,8 @@ warning: print test
54 | let _print_result_match = x?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: QuestionMark(
QuestionMarkExpr {
= note: Try(
TryExpr {
data: CommonExprData {
_lifetime: PhantomData<&()>,
id: ExprId(..),
Expand Down
6 changes: 3 additions & 3 deletions marker_utils/src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ pub fn traverse_expr<'ast, B>(
traverse_expr(cx, visitor, e.left())?;
traverse_expr(cx, visitor, e.right())?;
},
ExprKind::QuestionMark(e) => {
ExprKind::Try(e) => {
traverse_expr(cx, visitor, e.expr())?;
},
ExprKind::Assign(e) => {
Expand Down Expand Up @@ -448,14 +448,14 @@ impl_traversable_for!(&'ast Body<'ast>, traverse_body);
pub trait BoolTraversable<'ast>: Traversable<'ast, bool> {
/// Checks if the given node contains an early return, in the form of an
/// [`ReturnExpr`](marker_api::ast::expr::ReturnExpr) or
/// [`QuestionMarkExpr`](marker_api::ast::expr::QuestionMarkExpr).
/// [`TryExpr`](marker_api::ast::expr::TryExpr).
///
/// This function is useful, for lints which suggest moving code snippets into
/// a closure or different function. Return statements might prevent the suggested
/// refactoring.
fn contains_return(&self, cx: &'ast AstContext<'ast>) -> bool {
self.for_each_expr(cx, |expr| {
if matches!(expr, ExprKind::Return(_) | ExprKind::QuestionMark(_)) {
if matches!(expr, ExprKind::Return(_) | ExprKind::Try(_)) {
ControlFlow::Break(true)
} else {
ControlFlow::Continue(())
Expand Down