It is not possible to disable or enable this warning by attribute. [@warning "-74"] (and friends) do not disable the warning if the warning is enabled with -w +74.

I believe a similar problem existed for warning 68 while it still existed. (That's the "match-on-mutable-state-prevent-uncurry" warning that went away with syntactic function arity.)

I suspect we would need to store a Warnings.t field in the typedtree for Texp_match and Texp_function (and then use it in matching.ml) to do this properly.

It may well be that the cost-benefit analysis is in favor of merging this as-is — typedtree changes involve some churn.

I looked at why the warning cannot be silenced with attributes. This is a general issue with warnings emitted during the typedtree->lambda phase. Currently we use Builtin_attributes.warning_scope to evolve the warning scope locally during the type-checking phase (parsetree->typedtree), but no such discipline is followed during the typedtree->lambda phase. I think we could fix this globally, but this is fairly orthogonal to this specific warning, and I agree that tackling the two issues separately is the best course of action -- so I consider it out of scope of the present PR.

At a highlevel, this looks good to me - I have one very minor comment on the warning name. I agree with your rationale for changing the description (and removing the references to mutability, etc.) but the use of "unexpected" in the name implies to me that this warning is always some kind of potential bug. Especially as we now have correct test examples which we expect to emit an unexpected warning, "unexpected" feels wrong!

Couldn't degraded-[to-]partial-match be used instead of unexpected-partial-match?

@dra27 I applied your suggestion so now the name is degraded-to-partial-match. But, come to think of it, I am thinking of changing to match-degraded-to-partial instead, I find that it reads better. Does your native-speaking self agree?

@Octachron, @dra27: this is a ping, in case one of you would be willing to revisit -- I applied all review suggestions so far.

(gentle ping again)

@Octachron the current wording of the documentation (which most people will never read, so maybe it's no big deal if there are two more paragraphs than you would have like) comes from my experience of writing a first version that clearly blamed mutable state for all instances of this warning, and then immediately finding counter-examples where polymorphic variants would result in this warning as well.

The counter-examples that we found were fixed, but structurally the compiler does not raise the warning when it degrades partiality under a mutable match, it raises the warning at toplevel for all situations where it generated a jump to the final handler -- without specific feedback on what caused these situations. To the best of my knowledge, this situation currently only arises due to mutable fields, but I am not 100% confident that this is the case today, and I am willing to bet that this will become wrong in the future again as we keep evolving the language and the pattern-matching compiler.

You can keep the information that the pessimization might happen in unknown cases, while mentioning earlier the single known case. But I do believe that the sentences about your reluctance to describe the pattern-matching compiler should be removed.

For instance, copy-editing a bit your text:

There are a few rare cases where this totality analysis is insufficient: the
type-checker believes the pattern-matching to be total, but certain values may
try clauses in order and fail to match any of them. In those cases, the
pattern-matching compiler conservatively compiles the pattern-matching as
partial, and emits this warning

(wrong sounds a bit scary in a documentation, situations conflict with the later This situation )

This situation happens rarely: the only known case requires matching on mutable
fields. Moreover, the vast majority of programs will see no performance
difference at all if it were to happen. For these reasons, the warning is
disabled by default: we believe that the right approach when this situation
occurs is to do nothing at all.

Thanks! I included your proposed modification.

A long time to agree with a name, but degraded-to-partial-match indeeds looks fine to me!

My point of view is that this new warning should be cherry-picked to 5.3: the core pattern-matching change is in 5.3, and it doesn't make sense to introduce the warning one version later when it is already implemented.

I agree, and I think that it would be nice to also include #13154, which is a relatively minor improvement, has already been reviewed and approved, and whose existence we took into account when reasoning about the desiderability of #13152. On the other hand, no terrible harm would occur if we delay that one to 5.4, so feel free to ask for this.

Agreed for #13154 in 5.3, since this would decrease the number of versions of the pattern matching compiler present in released compilers.

I observe the following failure of the test memory-model/forbidden.ml (in bytecode) on MSVC 64 on this PR:

-Observation MP+PA Never
+Observation MP+PA Sometimes
+Invalid behaviour on test MP+PA

@gasche, the warning is not documented in the man page (man ocamlc)