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
Centril committed Sep 21, 2019
2 parents 8646c81 + 194d357 commit d7e511a
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 d7e511a

Please sign in to comment.