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

Should .below be marked as private/internal? #4701

Open
semorrison opened this issue Jul 9, 2024 · 4 comments
Open

Should .below be marked as private/internal? #4701

semorrison opened this issue Jul 9, 2024 · 4 comments
Labels
P-low We are not planning to work on this issue RFC Request for comments

Comments

@semorrison
Copy link
Collaborator

See for example

#check List.Sublist.below.cons
List.Sublist.below.cons.{u_1} {α : Type u_1} :
  ∀ {motive : (a a_1 : List α) → a.Sublist a_1 → Prop} {l₁ l₂ : List α} (a : α) {a_1 : l₁.Sublist l₂},
    a_1.below → motive l₁ l₂ a_1 → ⋯.below

In tests/lean/run/constructor_as_variable.lean we now see the error:

error: invalid pattern, constructor or constant marked with '[match_pattern]' expected

Suggestions: 'List.Sublist.below.cons', 'List.Sublist.cons', 'List.cons'

on the test

def ctorSuggestion2 (list : List α) : Nat :=
  match list with
  | nil => 0
  | cons x xs => 1 + ctorSuggestion2 xs

These .below auxiliary functions are rarely (never?) meant to be used by users, and perhaps should be marked as private or internal in some way, so they would not be displayed here.

There is already a check for isPrivateName when generating this message.

@semorrison semorrison added the RFC Request for comments label Jul 9, 2024
@semorrison
Copy link
Collaborator Author

Pinging @arthur-adjedj for an opinion?

@arthur-adjedj
Copy link
Contributor

arthur-adjedj commented Jul 9, 2024

When having Batteries imported, the number of suggestions grows a lot, and not many of them seem to stem from below, it might be useful to restrict these suggestions to names of functions whose final codomains are that of the expected type being matched on (i.e List here):

invalid pattern, constructor or constant marked with '[match_pattern]' expected

Suggestions:
  'Batteries.AssocList.cons',
  'Batteries.BinomialHeap.Imp.Heap.cons',
  'Batteries.RBNode.Stream.cons',
  'IO.AsyncList.cons',
  'LazyList.cons',
  'Lean.AssocList.cons',
  'List.Chain.below.cons',
  'List.Chain.cons',
  'List.Forall₂.below.cons',
  'List.Forall₂.cons',
   (or 7 others)

I don't think there are many cases in which proving a .below property would be useful for the user, so I would be inclined to mark them as private/internal. If .belows were to be marked as private, shouldn't .brecOn also be marked as such ? AFAIK, they're mostly unusable unless one one can provide a proof of .below.
Pinging @nomeata for a second opinion.

@nomeata
Copy link
Contributor

nomeata commented Jul 9, 2024

I agree that it should not be suggested like this. I wouldn't hide it completely, it's useful for learning and for advanced uses, but not a good suggestion.

@leanprover-bot leanprover-bot added the P-low We are not planning to work on this issue label Jul 22, 2024
@nomeata
Copy link
Contributor

nomeata commented Jul 22, 2024

JFTR: I anticipate that at some point we’ll have to get more clarity on the different notions of internal/private/etc that we have floating around, and their relevance for documentation listing/loogle/tactics/error message suggestions. When that comes up hopefully it’ll be clearer what to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low We are not planning to work on this issue RFC Request for comments
Projects
None yet
Development

No branches or pull requests

4 participants