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

Using open or export does not make names accessible for extended field notation (dot notation) #3031

Closed
kmill opened this issue Dec 5, 2023 · 4 comments · Fixed by #6189
Closed
Labels
P-medium We may work on this issue if we find the time RFC Request for comments

Comments

@kmill
Copy link
Collaborator

kmill commented Dec 5, 2023

Description

If you try to use open or export to import names to use for dot notation, it does not work. Confusingly, you can have String.a "x" succeed but "x".a fail.

Context

Zulip thread where this was brought up, during an Advent of Code problem.

Steps to Reproduce

Observe that "x".a produces an error in the following

namespace Common
namespace String

def a (s : String) : Nat := s.length

end String
end Common

open Common (String.a)

#eval String.a "x"

/-
invalid field 'a', the environment does not contain 'String.a'
  "x"
has type
  String
-/
#eval "x".a

export Common (String.a)

/-
invalid field notation, function 'Common.String.a' does not have argument
with type (Common.String ...) that can be used, it must be explicit or
implicit with a unique name
-/
#eval "x".a

Expected behavior: The second and third #evals have the same interpretation as the first.

Actual behavior: Only the first succeeds. The second #eval (when the name is just open) doesn't see String.a at all, and the third #eval (when the name is exported) it sees it but then fails to process it correctly.

Versions

MacOS arm64

/- "4.3.0-rc2" -/
#eval Lean.versionString

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@kmill kmill added the bug Something isn't working label Dec 5, 2023
@digama0
Copy link
Collaborator

digama0 commented Dec 5, 2023

I would love this feature, it would be a way to avoid the name-clash-inducing practice of defining functions outside the project namespace so that you get dot notation on types you don't own. (PS: I would describe this as a feature request / RFC, not a bug.)

@Kha
Copy link
Member

Kha commented Dec 6, 2023

I think this is an RFC :) . It is a sensible enhancement suggestion but it is not an accidental oversight in the implementation.

@Kha
Copy link
Member

Kha commented Dec 7, 2023

I should also mention a much older proposal of allowing any top-level name in scope as a target for extended field notation. This is especially interesting for typeclasses where it would allow us to use x.toString uniformly instead of the status quo where it only works sometimes depending on whether that exact declaration name was used to construct the instance. It is similar to how in Rust you need to use a trait to get dot access to its members.
It might be that these two proposals are independent(ly desirable) but they are similar enough that I wanted to mention this one in case we say that it would be confusing to have both.

github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2023
This definition was clearly meant to be in the `List` namespace, but it
is also in a `namespace Lean` so it ended up as `Lean.List.toSMap`
instead of `List.toSMap`. It would be nice if #3031 made this
unnecessary, but for now this seems to be the convention.

I noticed this because of another side effect: it defines `Lean.List` as
a namespace, which means that
```lean
import Std

namespace Lean
open List

#check [1] <+ [2]
```
does not work as expected, it opens the `Lean.List` namespace instead of
the `List` namespace. Should there be a regression test to ensure that
the `Lean.List` namespace (and maybe others) are not accidentally
created? (Unfortunately this puts a bit of a damper on #3031.)
@Kha Kha added RFC Request for comments and removed bug Something isn't working labels Sep 27, 2024
@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Sep 27, 2024
kmill added a commit to kmill/lean4 that referenced this issue Nov 23, 2024
This PR changes how generalized field notation ("dot notation") resolves the name of the function, adds a feature where terms such as `x.toString` can resolve as `toString x` as a last resort, and modifies the `Function` dot notation to always use the first explicit argument rather than the first function argument. The new rule is that if `x : S`, then `x.f` resolves the name `S.f` relative to the root namespace (hence it now responds to `export` and `open). Breaking change: aliases now resolve differently. Before, if `x : S`, and `S.f` is an alias for `S'.f`, then `x.f` would use `S'.f` and look for an argument of type `S'`. Now, it looks for an argument of type `S`, which is more generally useful behavior. Code making use of the old behavior should consider defining `S` or `S'` in terms of the other, since dot notation can unfold definitions during resolution.

This also fixes a bug in explicit field notation (`@x.f`) where `x` could be passed as the wrong argument.

Closes leanprover#3031
@kmill
Copy link
Collaborator Author

kmill commented Nov 23, 2024

#6189 implements this along with @Kha's suggestion.

New rule: given x : S, then x.f tries resolving S.f with respect to the root namespace. If it can't be resolved, then it tries it with each parent in the resolution order of S as well, if S is a structure.

Additionally, as a last resort, after all attempts of unfolding S to find a resolution, it looks for f in the root namespace (either as an exact match or as an alias) and then does f S. I made it pay no attention to open, since I couldn't think of any examples where you would want that, but it does resolve x.toString to toString x since toString is exported.

(This is all still pending seeing if building mathlib uncovers any problems with this scheme! Edit: Mathlib only needed a few lines changed. Quaternion was using the old version of the aliasing feature, but deleting the export and fully qualifying re in one place fixed it.)

github-merge-queue bot pushed a commit that referenced this issue Nov 25, 2024
This PR changes how generalized field notation ("dot notation") resolves
the function. The new resolution rule is that if `x : S`, then `x.f`
resolves the name `S.f` relative to the root namespace (hence it now
affected by `export` and `open`). Breaking change: aliases now resolve
differently. Before, if `x : S`, and if `S.f` is an alias for `S'.f`,
then `x.f` would use `S'.f` and look for an argument of type `S'`. Now,
it looks for an argument of type `S`, which is more generally useful
behavior. Code making use of the old behavior should consider defining
`S` or `S'` in terms of the other, since dot notation can unfold
definitions during resolution.

This also fixes a bug in explicit-mode generalized field notation
(`@x.f`) where `x` could be passed as the wrong argument. This was not a
bug for explicit-mode structure projections.

Closes #3031. Addresses the `Function` namespace issue in #1629.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium We may work on this issue if we find the time RFC Request for comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants