Skip to content
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

missing (?) kind of warning #7788

Open
vicuna opened this Issue May 1, 2018 · 5 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

vicuna commented May 1, 2018

Original bug ID: 7788
Reporter: kosik
Status: acknowledged (set by @xavierleroy on 2018-05-23T18:11:22Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.06.1
Category: misc

Bug description

Ocaml compiler(s) currently support the following warning:

4 ... Fragile pattern matching: matching that will remain complete even if additional constructors are added to one of the variant types matched.

Wouldn't it make sense to also provide an analogous warning for records?

For example, if I write the following pattern:

{f1; f2; _}

the compiler would warn me:

"Fragile pattern matching: matching will remain complete even if additional fields are added to a record type".

What do you think?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 1, 2018

Comment author: @gasche

In fact, the pattern "; _" was added precisely to disable warning 9 (Missing fields in a record pattern), when you use a precise record pattern on a value with more fields than the pattern.

I don't think that the situation for the sums/variants and the products/records are symmetric.

For sums/variants, it is the default when you add a new case to want to review existing pattern matches (because cases become differences in control-flow, and no one but the user knows what to do with a new flow), and on the contrary it is convenient (often the shortes thing) to use a "| _ ->" catch-all case in a series of clauses -- so you need to fight that convenience by adding a warning to prevent people from the easy thing.

For products/records, adding a new field is adding more information to each record argument; there is an effective default on what to do, which is to just ignore this new information (you didn't need it before). Finally, using the "; _" construction to silence the "some fields are not matched" warning is the hard path, not the shortest/easy one, so we can safely assume that it explicitly expresses the intent to ignore all other fields -- as opposed to not writing anything at all, which already warn.

For these reasons, I am not sure that a new warning when "; _" is used would be very useful. Do you have an example of a use-case?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 1, 2018

Comment author: kosik

Thank you Gabriel for the reply.

One of the things I do not get in your response is a claim that:

using the "; _" construction to silence the "some fields are not matched"
warning is the hard path.

Do you mean that if I have a record with 5 fields: f1, f2, f3, f4, f5, then this:

(* here I only bind two fields: "f1" and "f2" *)
{f1; f2; _}

is a harder path than:

(* here, like above, I only bind two fields: "f1" and "f2" *)
{f1; f2; f3 = _; f4 = _; f5 = _}

?

Isn't it exactly the other way around?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 10, 2018

Comment author: @stedolan

I think the analogy to pattern matching on variants is better made with record construction, rather than record matching.

One could imagine an alternate version of OCaml which allowed records to be constructed by only specifying values for a subset of the record's fields, causing a runtime error to be raised if anyone tries to access an unfilled field. This would be symmetric with OCaml variants, which allow variants to be matched by only specifying actions for a subset of the variant's cases, causing a runtime error to be raised if anyone tries to supply an unhandled case.

To deal effectively with such "partial records", we'd need a copy of all of the partial-matching machinery: warnings when records are constructed with some fields missing (like nonexhaustive matches), warnings on fragile record constructions, etc.

However, actual OCaml demands that every field is present when records are constructed, but does allow some cases to be missing when variants are matched. So, the machinery for dealing with partiality and fragility is only needed for variants, not records.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 10, 2018

Comment author: kosik

@stedolan

Alternate versions of Ocaml might make sense,
but my original post is related to the current/existing/actual Ocaml version.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 23, 2018

Comment author: @xavierleroy

I've never been a big fan of warning 4 and I see no good reason to discourage programmers from using the "; _ }" syntax to mark their intention of not matching all the fields of a record. I move to close this feature request. Anyone wants to argue otherwise?

@vicuna vicuna added the feature-wish label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.