Missed ignored partial application due to ignored named binding

[madgen]

Missed ignored partial application due to ignored named binding

[gasche]

The Common module looks like this:

  module Common = struct
    let _absname = set Clflags.absname
    let _alert = Warnings.parse_alert_option
    let _alias_deps = clear transparent_modules
  end

Here the _foo variables are not named with a leading underscore to be ignored, but because they may conflict with keyword names -- there is _open down below.

An important difference between these two use-cases is that _a in your example is unused, while _alert (or others) above are used. This suggests a new heuristic that is not among your list:

• check ignored-partial-application for all unused variables, even those that start with an underscore

(If we decide to go for this, we should probably extend the logic in #12094 as well for the case match e with _a -> ....)

[nojb]

Apologies for the delay.

Should I interpret the silence as this is something folks don't want?

No, just that there is limited reviewing capacity in the team :) In general we are quite happy to have suggestions and proposals from users (especially so when such proposals come with offers to help with the implementation).

I think the suggestion by @gasche makes sense:

• check ignored-partial-application for all unused variables, even those that start with an underscore

This is a mostly backwards-compatible change that should also address your use-case; do you agree? I didn't look in detail, but I think it shouldn't be very hard to extend the existing logic when typechecking let expressions to implement this check. If this works out, there is a similar logic when typing match expressions that should be extended similarly.

If you agree, would you like to give it a try?

[gasche]

(cc @nojb, our expert on ignoring partial applications.)

[madgen]

Should I interpret the silence as this is something folks don't want?

[madgen]

No, just that there is limited reviewing capacity in the team :)

Of course, no problem!

This is a mostly backwards-compatible change that should also address your use-case; do you agree?

I think it does, yes.

If you agree, would you like to give it a try?

Yes, I would love to.

I'll have a look at the implementation, try to come up with a plan this week, and post it here for feedback before I go ahead with coding (unless it is trivial).

[nojb]

I got bitten by this today; now am looking forward to this patch :)