- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
Suggest removing the tuple struct field for the unwrapped value #99593
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
Suggest removing the tuple struct field for the unwrapped value #99593
Conversation
| (rust-highfive has picked a reviewer for you, use r? to override) | 
e01518c    to
    1f799ae      
    Compare
  
    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.
FnCtxt has self.typeck_results which is not Option
| if let ty::Adt(expected_adt, substs) = expected.kind() { | |
| if let hir::ExprKind::Field(base, ident) = expr.kind | |
| && let Some(typeck_results) = self.in_progress_typeck_results | |
| { | |
| let base_ty = typeck_results.borrow().expr_ty(base); | |
| if let ty::Adt(expected_adt, substs) = expected.kind() { | |
| if let hir::ExprKind::Field(base, ident) = expr.kind | |
| { | |
| let base_ty = self.typeck_results.borrow().expr_ty(base); | 
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.
Can we use something like can_eq instead?
| if base_ty == expected { | |
| if self.can_eq(self.param_env, base_ty, expected).is_ok() { | 
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.
alternatively we could use same_type_modulo_regions, but since we have access to can_eq, it's a bit more accurate.
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.
What does this do if base comes from a macro expansion? For example:
// run-rustfix
pub struct MyWrapper(u32);
macro_rules! my_wrapper {
  ($expr:expr) => { MyWrapper($expr) }
}
fn main() {
    some_fn(my_wrapper!(123).0); //~ ERROR mismatched types
}
fn some_fn(wrapped: MyWrapper) {
    drop(wrapped);
}I think base.span should probably be adjusted to account for macros. Try matching if let Some(base_span)  = base.span.find_ancestor_inside(expr.span) first to make sure that expr span and base span overlap.
add a test case for macro
1f799ae    to
    f85f375      
    Compare
  
    | @rustbot ready | 
| Wonderful. Thanks for addressing comments. @bors r+ | 
Rollup of 7 pull requests Successful merges: - rust-lang#98211 (Implement `fs::get_path` for FreeBSD.) - rust-lang#99353 (Slightly improve mismatched GAT where clause error) - rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value) - rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`) - rust-lang#99711 (Remove reachable coverage without counters) - rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions) - rust-lang#99720 (Sync rustc_codegen_cranelift) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - rust-lang#98211 (Implement `fs::get_path` for FreeBSD.) - rust-lang#99353 (Slightly improve mismatched GAT where clause error) - rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value) - rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`) - rust-lang#99711 (Remove reachable coverage without counters) - rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions) - rust-lang#99720 (Sync rustc_codegen_cranelift) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
fixes #99416