I've pushed a repair for module+ and synatx-original?. Thanks for the example!
What happens is that add-arrow is called twice, once with require-arrow? false and once with it non-false (#t or module-lang).
Although that's not necessarily intuitive -- two arrows, claiming the use has two different "sources"? -- that's OK. I can handle this pragmatically: When this occurs, trust the one arrow (saying it's an import) and disregard the other, i.e. don't produce my extra "defined locally" mouse-over annotation.
It took me a few days to get around to doing the raco pkg update --clone thing and trying out your branch.
The improvements look good!
One thing I noticed is rename-in is still causing at least one weird result.
In this program:
#lang racket/base
(require racket/list)
first
(require (rename-in racket/list [first 1st]))
1st
It seems non-intuitive that there is an arrow between the use of first and the 1st alias introduced in the rename-in clause.
Instead I would have guessed any of the following:
- No extra arrow at all.
- An extra arrow to the
firstwithinrename-in. - An extra arrow to the
racket/listwithinrename-in.
I guess this is just the free-identifier=?-keyed identifier table working as-intended.
Probably a tool consuming these arrows could ignore some of these arrows that aren't symbol-equal (as I mentioned above). I think I could do that and it would work OK.
But I wanted to point it out. Do you think it's weird for DrRacket to show such arrows? If so, then I guess we could try to address it in traversals.rkt, somehow.
(Maybe connect-syntaxes could filter these?? But that might require some kind of flag on things like 1st that were introduced by rename-in, and do the symbol-equal check IFF that's set??)
Ha! It's funny you mention that. 
The way the code is structured, there's kind of a separate flaw in it that leads to those arrows and I was just thinking about how to tackle fixing it. Well, actually, I was more focused on the arrow from 1st to racket/list but they are related things.
As for the options, I have gotten as far as thinking we shouldn't have an arrow from first to the racket/list inside the rename-in because this program is a syntax error:
#lang racket/base
first
(require (rename-in racket/list [first 1st]))
That is, rename-in doesn't bring in the names in racket/list in general; it brings in only the ones that are explicitly mentioned, I believe.
Anyway, I'm planning on starting by plumbing through the information about which identifiers are actually coming in from which requires and then seeing how far we can get with a good set of arrows and was planning to post here when I made some progress.
1 Like
Oh, cool, good timing then. 
rename-indoesn't bring in the names inracket/listin general; it brings in only the ones that are explicitly mentioned
Derp. Of course you're right.
My original intuition was "no extra arrow at all". Unfortunately I edited the post to "improve" it by itemizing more options... sigh.
What do you think about the arrow between the 1st and racket/list that's inside the rename-in? I'm wondering if it should go away.
For rename-in, I think ideally there should be arrows between the module path and the old name, and also between the old and new names.
In other words, resorting to ASCII art arrows for the little program we talked about above:
#lang racket/base
(require racket/list)
;; ________/
;; /
first
;; ________ ___
;; / \ / \
(require (rename-in racket/list [first 1st]))
;; ___________________________________/
;; /
1st
And similar for rename-out.
I say "ideally" because I want to think of the arrows as showing the "provenance" graph of things generally.
[I'm actually not sure if "bindings" per se is a subset of "provenance" generally. Maybe conceptually they are the same, and it's just that check-syntax doesn't include some, today? OTOH if conceptually they're distinct ideas, I'd like to see provenance generally, not just bindings strictly, if that seems desirable and practical, to you.]
Just wanted to say that I pushed something that improves this. After some separate discussion with @mflatt I've understood that there are some cases that will still be wrong; here's one he supplied:
#lang racket/base
(require racket/list)
(let ([first #f])
(local-require racket/list)
first)
This one has an arrow from the first require to the first at the end of the program and it doesn't seem like the information needed to remove it is available in the fully expanded form.
Still, doing the check that you advocate, @greghendershott, seems like a good idea as it covers at least some cases.
Still thinking about how these chained arrows would work.
Just wanted to say that I pushed something that improves this.
Nice! I'll fetch and try.
After some separate discussion with @mflatt I've understood that there are some cases that will still be wrong
Interesting. My arrow expectations come from (mis)believing imports work something like (define normal-fresh-identifier using-some-import-magic). However it seems the reality is that imports want to "reuse existing identifiers" as much as possible; so we get cases like this?
If so, I guess one path is accept that the arrows tell the truth, and people like me need to fix their intuition (and some tools for things like multi-file renaming might be subject to some more failure modes).
Otherwise I don't see how to fix this broadly, other than rewriting the fully expanded program to fit the intuition (which seems, even if doable, unwise).
Still thinking about how these chained arrows would work.
Possibly there could just be some new hash-table for them? The examples I have in mind all arise from rename clauses in #%require and #%provide. The end points include items that aren't even identifiers per se (the module path, and the old imported name), so AFAICT identifier tables wouldn't work. Possibly we could just add these to a new side dict, and emit more arrows as an extra step?
[In a prototype "project database" tool -- which dumps check-syntax results for multiple files into a sqlite db -- I actually did something like this. As an extra pass independent of check-syntax, I synthesized extra arrows.
But since we're talking about arrows, including in Dr Racket, I thought it would be good to mention, in case you thought it would be good to add. (If you don't, no worries, I could still keep doing that as an extra step, in that tool, if/when I ever get back to working on it for real.)]
Unlike only-in, rename-in does bind all of the unmentioned names. For example, this works:
#lang racket/base
(require (rename-in racket/list [first 1st]))
second
Ah, right. Thanks @ryanc .
re extra arrows @greghendershott I hadn't thought of those new arrows on my own and that's part of my hesitation but I should just get over that part, I think. -- that's a dumb rationale! Is the basic idea that each identifier that is brought into scope should get just one arrow that goes either to the library (if there's no renaming) or to the name mentioned in the require? And then, if the name that is used isn't the one exported, there should be two more arrows that transitively connect to the name of the module? (And a similar dual thing for renaming on provide?) I guess those arrows can help with the "remove unused requires" functionality too. Did you consider just one extra arrow from the module name to the renamed name (since the renamed name and the exported name are always right next to each other anyway)? I'm not sure there are other "transitive" arrows currently; are there other candidates we should also be considering?
Another thing that I think will work out (but need to test out to see) is renaming. I think the situation should actually improve for that, tho!
In something like (require (rename-in mod [old new])):
As consumed by a human:
It might seem silly/noisy to draw these extra arrows. Of course old comes from mod; where else. As for old and new, as you say, proximity. Good points. OTOH if someone doesn't hover, they don't get drawn, so maybe not so noisy?
As consumed by a tool:
What I had in mind was a DAG tracing every step, even through indefinitely long chains of renaming imports or exports.
Whereas identifier-binding's superpower is eliding all that and letting you jump directly to definition, this would be the opposite superpower, seeing the all the steps one by one.
It would help support things like renaming a definition and updating correctly across multiple files.
Finally, a tool needs srcloc, it can't grok visual proximity.
Admittedly, I was sketching this out; I can't assert that every arrow is necessary. But I also didn't know all the possible applications, so I was erring on the side of capturing everything I could think of, for the database. (And these extra arrows are a small added percentage, in my experiments, compared to all the other data generated by syncheck.)
p.s. The project is GitHub - greghendershott/pdb: Multi-file check-syntax database. I worked on it pretty intensely for awhile, then set it aside to work on other things (more immediately related to Racket Mode). Recently I went to dust it off, and was surprised to see many tests failing, that I could have sworn used to pass. As one part of looking into that, I started this thread.
Not to prolong this very long thread -- on the contrary I wanted to report back success and thanks to @robby!
Given this commit for drracket-tool-text-lib I was able to get all tests passing again for that "pdb" project. (Well, modulo temporarily disabling one sets of tests that apparently depended on a pre-release notion of for-space, about which I need to loop back to and re-learn.)
That's great to hear! Something that's caching the check syntax results for multi-file check syntax would be awesome to have!