- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
Exhaustively handle expressions in patterns #134228
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
Conversation
| rustbot has assigned @compiler-errors. Use  | 
| HIR ty lowering was modified cc @fmease Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in match checking cc @Nadrieril | 
763003c    to
    9af4537      
    Compare
  
    | HIR ty lowering was modified cc @fmease Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in match checking cc @Nadrieril | 
9af4537    to
    42752cc      
    Compare
  
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
42752cc    to
    7cee193      
    Compare
  
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
| @rustbot author | 
7cee193    to
    37cf874      
    Compare
  
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
37cf874    to
    20f5e3f      
    Compare
  
    | ☔ The latest upstream changes (presumably #134243) made this pull request unmergeable. Please resolve the merge conflicts. | 
20f5e3f    to
    c3783c4      
    Compare
  
    | ☔ The latest upstream changes (presumably #134788) made this pull request unmergeable. Please resolve the merge conflicts. | 
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.
Other than the naming, which I think we should rename to PatExpr to make it clear that it's the subset of expressions allowed in pats, I think this looks fine.
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
Merge `PatKind::Path` into `PatKind::Lit` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now. subsequently we can also nuke `visit_pattern_type_pattern`
Rollup merge of rust-lang#135251 - oli-obk:push-lmpyvvyrtplk, r=ytmimi Only treat plain literal patterns as short See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
Only treat plain literal patterns as short See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
Only treat plain literal patterns as short See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
Only treat plain literal patterns as short See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
Rollup merge of rust-lang#135251 - oli-obk:push-lmpyvvyrtplk, r=ytmimi Only treat plain literal patterns as short See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
Merge `PatKind::Path` into `PatKind::Lit` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
Merge `PatKind::Path` into `PatKind::Lit` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
Merge `PatKind::Path` into `PatKind::Lit` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
Merge `PatKind::Path` into `PatKind::Expr` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
Merge `PatKind::Path` into `PatKind::Expr` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
Merge `PatKind::Path` into `PatKind::Expr` Follow-up to rust-lang#134228 We always had a duplication where `Path`s could be represented as `PatKind::Path` or `PatKind::Lit(ExprKind::Path)`. We had to handle both everywhere, and still do after rust-lang#134228, so I'm removing it now.
…, r=<try> Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
…eril Reject negative literals for unsigned or char types in pattern ranges and literals It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic. In rust-lang#134228 I caused ```rust fn main() { match 42_u8 { -10..255 => {}, _ => {} } } ``` to just compile without even a lint. I can't believe we didn't have tests for this Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
…ks, r=compiler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
…eril Reject negative literals for unsigned or char types in pattern ranges and literals It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic. In rust-lang#134228 I caused ```rust fn main() { match 42_u8 { -10..255 => {}, _ => {} } } ``` to just compile without even a lint. I can't believe we didn't have tests for this Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
…ks, r=compiler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
…eril Reject negative literals for unsigned or char types in pattern ranges and literals It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic. In rust-lang#134228 I caused ```rust fn main() { match 42_u8 { -10..255 => {}, _ => {} } } ``` to just compile without even a lint. I can't believe we didn't have tests for this Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
Rollup merge of rust-lang#136304 - oli-obk:push-ymxoklvzrpvx, r=Nadrieril Reject negative literals for unsigned or char types in pattern ranges and literals It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic. In rust-lang#134228 I caused ```rust fn main() { match 42_u8 { -10..255 => {}, _ => {} } } ``` to just compile without even a lint. I can't believe we didn't have tests for this Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
…ks, r=compiler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
…ks, r=compiler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
Rollup merge of rust-lang#136393 - oli-obk:pattern-type-lit-oflo-checks, r=compiler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
…piler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang/rust#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
Only treat plain literal patterns as short See rust-lang/rust#134228 (comment) and rust-lang/rust#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals. I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
This matches the HIR changes in rust-lang#134228, which introduced `PatExpr` to hold the subset of "expressions" that can appear in a pattern.
This matches the HIR changes in rust-lang#134228, which introduced `PatExpr` to hold the subset of "expressions" that can appear in a pattern.
This matches the HIR changes in rust-lang#134228, which introduced `PatExpr` to hold the subset of "expressions" that can appear in a pattern.
…piler-errors Fix accidentally not emitting overflowing literals lints anymore in patterns This was regressed in rust-lang/rust#134228 (not in beta yet). The issue was that previously we nested `hir::Expr` inside `hir::PatKind::Lit`, so it was linted by the expression code. So now I've set it up for visitors to be able to directly visit literals and get all literals
We currently have this invariant in HIR that a
PatKind::Litor aPatKind::Rangeonly containsExprKind::LitExprKind::UnOp(Neg, ExprKind::Lit)ExprKind::PathExprKind::ConstBlockSo I made
PatKind::LitandPatKind::Rangestop containingExpr, and instead created aPatLittype whosekindenum only contains those variants.The only place code got more complicated was in clippy, as it couldn't share as much anymore with
ExprhandlingIt may be interesting on merging
ExprKind::{Path,Lit,ConstBlock}in the future and using the samePatLittype (under a new name).Then it should also be easier to eliminate any and all
UnOp(Neg, Lit) | Litmatching that we have across the compiler. Some day we should fold the negation into the literal itself and just store it on the numeric literals