feat: jump to definition proof of concept #911

rtetley · 2024-09-20T16:57:58Z

WIP

cf coq/coq#19584

gares · 2024-09-21T10:31:39Z

Changes to coq look good too.
I wonder if there is a way to circumvent jumping to dune's copy of the sources.
I mean, should the logic be in Coq or here?
@SkySkimmer added the print of the Loc to a user visible message, so I guess it should be in Coq

SkySkimmer · 2024-09-21T10:39:06Z

@SkySkimmer added the print of the Loc to a user visible message, so I guess it should be in Coq

That logic is on the loc -> pp side though, vscoq only calls the qualid -> loc part from coq and does the rest on its own.

SkySkimmer · 2024-09-21T10:41:37Z

language-server/dm/documentManager.ml

+        match get_context st o_pos with
+        | None -> log "No context found"; None
+        | Some _ ->
+          match parse_entry st loc (Pcoq.Prim.smart_global) pattern with


Suggested change

match parse_entry st loc (Pcoq.Prim.smart_global) pattern with

match parse_entry st loc Pcoq.Prim.qualid pattern with

since you're only using the AN case of smart_global

gares · 2024-09-21T11:18:57Z

It may still make sense to have the api on the coq side and let vscoq call it on the loc, no?

Anyway, good job!

YaZko · 2024-10-23T14:24:39Z

I wonder if there is a way to circumvent jumping to dune's copy of the sources.

I do not know if it's feasible, but as a user that would be fantastic if possible. Company-coq does indeed jumps to dune source, and it has always been a major pain.

TheoWinterhalter · 2024-10-23T14:52:26Z

I guess in the meantime something that works in most cases but not all is better than nothing?

SkySkimmer · 2024-11-13T12:36:07Z

language-server/dm/documentManager.ml

+            begin match Nametab.locate qid with
+              | ConstRef x -> begin match Declare.get_loc x with


this will need updating as I moved get_loc and made it apply to more than constants
something like

Suggested change

begin match Nametab.locate qid with

| ConstRef x -> begin match Declare.get_loc x with

let extref = Nametab.locate_extended qid in

match Nametab.cci_src_loc extref with

should work

SkySkimmer · 2024-11-13T12:39:49Z

Not sure how well this works with an editable document, I mean start with

(* some comment *)
Definition foo := 0.

Check foo.

evaluate the Definition, then edit the comment, then jump to def of the foo in Check

if editing the comment doesn't force re-evaluating the Definition then the loc will be wrong, but we don't want to re execute after editing comments (or whatever other edits we can detect as being trivial)

gares · 2024-11-13T14:20:15Z

That is certainly true, although it seems a minor problem in practice.

I'm thinking loud here: if we manage to perform some pieces of the globalization phase at parse time, then parsing "Definition foo := ..." could tell us that we will generate Some.Logical.Path.foo and we could store the KN->loc map in a synterp summary (rather than an interp one). We could complement the map at interp time with interp-time-generated stuff we really want to be precise, and that part would need to be shifted/shiftable by the UI when it tries to reuse an old state on a new text. Exactly as we do for diagnostics.

In a way, since the map is about locs, it should be part of the parsing state, not the interp one.

SkySkimmer · 2024-11-13T14:39:07Z

I'm thinking loud here: if we manage to perform some pieces of the globalization phase at parse time

That's impossible IMO

In a way, since the map is about locs, it should be part of the parsing state, not the interp one.

The map is about objects like globrefs which are part of the interp state.

gares · 2024-11-13T17:06:56Z

That's impossible IMO

AFAIK synterp already computes the current modpath. From that one can compute the final KN of "foo". Am I missing something? (I know today the kn data type is generated using the env, etc... but in principle we know all the data to compute it even without the env)

rtetley · 2024-12-04T07:43:52Z

@gares @SkySkimmer could you review before merge ?

gares · 2024-12-04T08:02:28Z

flake.nix

@@ -4,7 +4,7 @@
  inputs = {
    flake-utils.url = "github:numtide/flake-utils";

-    coq-master = { url = "github:coq/coq/668dc3ca9735a2b008f5ce52c14969af59cc24e1"; }; # Should be kept in sync with PIN_COQ in CI workflow
+    coq-master = { url = "github:rtetley/coq/d6dd8a618cbf628a6580c9a42db0cf5f44606bea"; }; # Should be kept in sync with PIN_COQ in CI workflow


should not be necessary anymore

gares · 2024-12-04T08:05:24Z

language-server/dm/documentManager.ml

+        let o_pos = RawDocument.position_of_loc raw_doc stop in
+        match get_context st o_pos with
+        | None -> log "No context found"; None
+        | Some _ ->


this is fishy, why we check for a context and then don't use it?
was this preliminary code to use the proof context when available?
At least it deserves a comment

Yeah, I think its just a way to check that we are indeed in an executed context.
If I just jump to def with no execution state than I just get an uncaught Not_found exception. But I suppose I should probably catch the exception and return None in that case. Or check that the sentence at the cursor is executed ?

rtetley · 2024-12-04T09:06:10Z

I still get errors if I try to use Jump to Def in certain cases. For example:
If I try to jump_to_def on True I will get the usual:

Error while opening document. Cannot load a library with the same name as the current one
(Stdlib.Init.Datatypes).

Was that not fixed ?

SkySkimmer · 2024-12-04T09:07:41Z

language-server/dm/documentManager.ml

+    log ("jumpToDef: found word at cursor: \"" ^ pattern ^ "\"");
+    try
+    let qid = parse_entry st loc (Procq.Prim.qualid) pattern in
+      let ref = Nametab.locate_extended qid in


I guess this is where Not_found can come from

Yeah I just enclosed it in a big try [...] with and it no longer breaks the server so that's okay

SkySkimmer · 2024-12-04T09:51:14Z

If I try to jump_to_def on True I will get the usual:

in the vscoq log do you see No project file found for .../Logic.v or Arguments from project file ...?

rtetley · 2024-12-04T12:24:48Z

If I try to jump_to_def on True I will get the usual:

in the vscoq log do you see No project file found for .../Logic.v or Arguments from project file ...?

Yes you are right. I get

No project file found for /Users/rtetley/.opam/vscoq-dev/lib/coq/theories/Init/Datatypes.v

SkySkimmer · 2024-12-04T12:41:20Z

Right, we don't install _CoqProject so there's nothing to find.
HoTT/Coq-HoTT#2146 is a similar issue
also coq/coq#15924

rtetley · 2024-12-04T14:22:43Z

Right, we don't install _CoqProject so there's nothing to find. HoTT/Coq-HoTT#2146 is a similar issue also coq/coq#15924

I'm not sure I understand how that pertains to the error I find ? I was under the impression that this is the error you get if you load it twice ? Unless you mean the _CoqProject contains a -no-init ?

rtetley · 2024-12-04T14:26:10Z

Right, we don't install _CoqProject so there's nothing to find. HoTT/Coq-HoTT#2146 is a similar issue also coq/coq#15924

I'm not sure I understand how that pertains to the error I find ? I was under the impression that this is the error you get if you load it twice ? Unless you mean the _CoqProject contains a -no-init ?

Answered my own question... https://github.com/coq/coq/blob/master/theories/Init/_CoqProject

rtetley · 2024-12-04T14:26:33Z

I suppose this is a Coq/Rocq issue then ?

gares · 2024-12-05T07:03:20Z

Yes, but we should be sure someone is fixing it ..

rtetley · 2024-12-05T07:06:25Z

Yes, but we should be sure someone is fixing it ..

Agreed, just double checking !

rtetley · 2024-12-05T14:39:50Z

Yes, but we should be sure someone is fixing it ..

Should I just merge in the mean time ?

gares · 2024-12-05T21:10:38Z

I guess so

rtetley · 2024-12-06T07:37:44Z

I'll merge and wait from news in coq/coq#15924

SkySkimmer reviewed Sep 21, 2024

View reviewed changes

SkySkimmer mentioned this pull request Nov 12, 2024

Provide source location for declared objects coq/coq#19584

Merged

SkySkimmer reviewed Nov 13, 2024

View reviewed changes

rtetley force-pushed the jump-to-def branch 2 times, most recently from 9c40be5 to 337844c Compare December 3, 2024 13:40

rtetley marked this pull request as ready for review December 3, 2024 13:41

gares approved these changes Dec 4, 2024

View reviewed changes

SkySkimmer reviewed Dec 4, 2024

View reviewed changes

rtetley added 2 commits December 5, 2024 15:38

feat: jump to definition is now supported (From Rocq 9 onwards)

4a9ca18

fix: correct coq pin in flake and catch errors

def952f

rtetley force-pushed the jump-to-def branch from 0c13f2d to def952f Compare December 5, 2024 14:39

rtetley merged commit 5c60b46 into main Dec 6, 2024
27 of 28 checks passed

rtetley deleted the jump-to-def branch December 13, 2024 14:24

rtetley mentioned this pull request Jan 17, 2025

jump to definition #214

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: jump to definition proof of concept #911

feat: jump to definition proof of concept #911

rtetley commented Sep 20, 2024 •

edited by SkySkimmer

Loading

gares commented Sep 21, 2024

SkySkimmer commented Sep 21, 2024

SkySkimmer Sep 21, 2024

gares commented Sep 21, 2024

YaZko commented Oct 23, 2024

TheoWinterhalter commented Oct 23, 2024

SkySkimmer Nov 13, 2024

SkySkimmer commented Nov 13, 2024

gares commented Nov 13, 2024

SkySkimmer commented Nov 13, 2024

gares commented Nov 13, 2024 •

edited

Loading

rtetley commented Dec 4, 2024

gares Dec 4, 2024

gares Dec 4, 2024

rtetley Dec 4, 2024

rtetley commented Dec 4, 2024

SkySkimmer Dec 4, 2024 •

edited

Loading

rtetley Dec 4, 2024

SkySkimmer commented Dec 4, 2024

rtetley commented Dec 4, 2024 •

edited

Loading

SkySkimmer commented Dec 4, 2024

rtetley commented Dec 4, 2024

rtetley commented Dec 4, 2024

rtetley commented Dec 4, 2024

gares commented Dec 5, 2024

rtetley commented Dec 5, 2024

rtetley commented Dec 5, 2024

gares commented Dec 5, 2024

rtetley commented Dec 6, 2024

	match parse_entry st loc (Pcoq.Prim.smart_global) pattern with
	match parse_entry st loc Pcoq.Prim.qualid pattern with

		begin match Nametab.locate qid with
		\| ConstRef x -> begin match Declare.get_loc x with

feat: jump to definition proof of concept #911

feat: jump to definition proof of concept #911

Conversation

rtetley commented Sep 20, 2024 • edited by SkySkimmer Loading

gares commented Sep 21, 2024

SkySkimmer commented Sep 21, 2024

SkySkimmer Sep 21, 2024

Choose a reason for hiding this comment

gares commented Sep 21, 2024

YaZko commented Oct 23, 2024

TheoWinterhalter commented Oct 23, 2024

SkySkimmer Nov 13, 2024

Choose a reason for hiding this comment

SkySkimmer commented Nov 13, 2024

gares commented Nov 13, 2024

SkySkimmer commented Nov 13, 2024

gares commented Nov 13, 2024 • edited Loading

rtetley commented Dec 4, 2024

gares Dec 4, 2024

Choose a reason for hiding this comment

gares Dec 4, 2024

Choose a reason for hiding this comment

rtetley Dec 4, 2024

Choose a reason for hiding this comment

rtetley commented Dec 4, 2024

SkySkimmer Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

rtetley Dec 4, 2024

Choose a reason for hiding this comment

SkySkimmer commented Dec 4, 2024

rtetley commented Dec 4, 2024 • edited Loading

SkySkimmer commented Dec 4, 2024

rtetley commented Dec 4, 2024

rtetley commented Dec 4, 2024

rtetley commented Dec 4, 2024

gares commented Dec 5, 2024

rtetley commented Dec 5, 2024

rtetley commented Dec 5, 2024

gares commented Dec 5, 2024

rtetley commented Dec 6, 2024

rtetley commented Sep 20, 2024 •

edited by SkySkimmer

Loading

gares commented Nov 13, 2024 •

edited

Loading

SkySkimmer Dec 4, 2024 •

edited

Loading

rtetley commented Dec 4, 2024 •

edited

Loading