default_field_values #21408
default_field_values #21408
Conversation
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-review
kouhe3
force-pushed
the
default_field_values
branch
from
c06113d to
046077b
Compare
rustbot
added
the
S-waiting-on-review
| } | ||
| } | ||
| _ => { | ||
| let missing_fields = ctx.sema.record_literal_missing_fields(record_expr); |
There was a problem hiding this comment.
Due to miss != match commit. I suggest here use new record_literal_matched_fields function
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq)] |
| #[derive(Debug, Clone, Eq, PartialEq)] | ||
| pub enum RecordSpread { | ||
| None, | ||
| Empty, |
There was a problem hiding this comment.
I dislike calling this Empty as it does not explain what it does, especially to users unfamiliar with the unstable feature. However I don't really have a good name... rustc calls this Rest (in the context of them calling the enum StructRest) which is also not a good name. Perhaps pub enum RecordSpread { None, FieldDefaults, FromBase(ExprId) }?
| pub type_ref: TypeRefId, | ||
| pub visibility: RawVisibility, | ||
| pub is_unsafe: bool, | ||
| pub has_default_value: bool, |
There was a problem hiding this comment.
We will likely want the default for some things, so it makes sense to store an Option<ExprId> here and lower the expression.
| } | ||
| if let &Some(expr) = spread { | ||
| f(expr); | ||
| if let RecordSpread::Expr(ref expr) = *spread { |
There was a problem hiding this comment.
Redundant ref then *, ditto below.
|
|
||
| for (_, data) in fields.fields().iter() { | ||
| let FieldData { name, type_ref, visibility, is_unsafe } = data; | ||
| let FieldData { name, type_ref, visibility, is_unsafe, .. } = data; |
There was a problem hiding this comment.
Please ignore only specific fields (no .. patterns, only field: _), we do not want to miss new fields here.
Also we probably want to print the default value, but we can't since we don't store it - relevant to what I said in the other comment.
| Some((variant_def, missed_fields)) | ||
| } | ||
|
|
||
| pub fn record_literal_matched_fields( |
There was a problem hiding this comment.
This and its friend record_pattern_matched_fields() don't belong in this file, record_literal_missing_fields() and record_pattern_missing_fields() are here because they're used by diagnostics.
Also "missing" vs. "matched" doesn't really capture the distinction, both are both missing and non-matched (which I suppose was the intent). Something like missing_considering_spread and missing_without_spread is better.
| Expr::RecordLit { path: _, fields, spread, .. } => { | ||
| self.infer_mut_not_expr_iter(fields.iter().map(|it| it.expr)); | ||
| if let RecordSpread::Expr(expr) = *spread { | ||
| self.infer_mut_not_expr_iter(std::iter::once(expr)); |
There was a problem hiding this comment.
| self.infer_mut_not_expr_iter(std::iter::once(expr)); | |
| self.infer_mut_expr(expr, Mutability::Not); |
| Some(p) | ||
| } | ||
| None => None, | ||
| RecordSpread::None | RecordSpread::Empty => None, |
There was a problem hiding this comment.
I think we should separate them, and for RecordSpread::Empty cause not_supported!(), because it needs a different implementation.
| return; | ||
| } | ||
| missing_fields | ||
| if update_exists { |
There was a problem hiding this comment.
This if should not exist here, instead call record_literal_matched_fields() to begin with.
3 participants
fix #19780
Add enum and field to track
=and..Create a new function to determine match and missing msg
behavior