Skip to content

Commit

Permalink
Rollup merge of rust-lang#64136 - crgl:doc-from-parser-lhs, r=Centril
Browse files Browse the repository at this point in the history
Document From trait for LhsExpr in parser

Add doc for From trait for converting P<Expr> and Option<ThinVec<Attribute>> to LhsExpr

As part of issue rust-lang#51430 (cc @skade).

Both of these should just be moving an address and setting a discriminant in an enum. The main thing I'm not sure about is whether it's worth documenting the branch in the From<Option<ThinVec<Attribute>>. As far as I can tell it doesn't seem like it is optimized away (although if the discriminant happened to work out you could just copy the pointer and the discriminant which might be cheaper, but that's not guaranteed). So it seems like if it's being called often, it's doubling the number of possible branch mispredictions on this Option, which could be a significant cost.

Let me know if there's anything that needs fixing and I'll get to it as soon as possible!
  • Loading branch information
tmandry authored Sep 20, 2019
2 parents 29188c5 + 194d357 commit 1807c5d
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/libsyntax/parse/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ pub(super) enum LhsExpr {
}

impl From<Option<ThinVec<Attribute>>> for LhsExpr {
/// Converts `Some(attrs)` into `LhsExpr::AttributesParsed(attrs)`
/// and `None` into `LhsExpr::NotYetParsed`.
///
/// This conversion does not allocate.
fn from(o: Option<ThinVec<Attribute>>) -> Self {
if let Some(attrs) = o {
LhsExpr::AttributesParsed(attrs)
Expand All @@ -76,6 +80,9 @@ impl From<Option<ThinVec<Attribute>>> for LhsExpr {
}

impl From<P<Expr>> for LhsExpr {
/// Converts the `expr: P<Expr>` into `LhsExpr::AlreadyParsed(expr)`.
///
/// This conversion does not allocate.
fn from(expr: P<Expr>) -> Self {
LhsExpr::AlreadyParsed(expr)
}
Expand Down

0 comments on commit 1807c5d

Please sign in to comment.