1601715 - The order of "Go to definition" is not intuitive

Status: RESOLVED DUPLICATE of bug 1776522

(Webtools :: Searchfox, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Clicking on CreateMaskLayer here https://searchfox.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp#5841

Gives a menu like this:

Go to definition of RefPtr::RefPtr<T>
Go to definition of mozilla::ContainerState::CreateMaskLayer
Search for constructor RefPtr::RefPtr<T>
Search for function mozilla::ContainerState::CreateMaskLayer
Search for the substring CreateMaskLayer
Sticky highlight

It would be better if Go to definition of mozilla::ContainerState::CreateMaskLayer
was above Go to definition of RefPtr::RefPtr<T>

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

I think the current ordering is just that of the analysis file, the excerpts for which currently look like this in its on-disk order, noting that when we ingest the records with read_analysis we sort by loc before de-duping. I'm including line-numbers here because again, the on-disk ordering is somewhat non-intuitive[1].

grep -n 5841:10 FrameLayerBuilder.cpp.json
5466:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"RefPtr::RefPtr<T>","sym":"_ZN6RefPtrC1EO16already_AddRefedIT_E","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
5467:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
7466:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeImEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
8831:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIyEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
17920:{"loc":"05841:10-25","source":1,"syntax":"constructor,use","pretty":"constructor RefPtr::RefPtr<T>","sym":"_ZN6RefPtrC1EO16already_AddRefedIT_E"}
17921:{"loc":"05841:10-25","source":1,"syntax":"function,use","pretty":"function mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE,_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeImEE,_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIyEE"}

The logic in the source code is at https://github.com/mozsearch/mozsearch/blob/cf247e4b010b5da900345e253e105e7410c866ac/tools/src/format.rs#L200 where you can see 2 passes. In all cases, we're processing "source" records from d.

• menu_jumps is processing the jumps data which comes from "target" records.
• items is just from those "source" records in d.

The jumps entries in question are:

["_ZN6RefPtrC1EO16already_AddRefedIT_E","mfbt/RefPtr.h",120,"RefPtr::RefPtr<T>"]
["_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE","layout/painting/FrameLayerBuilder.cpp",7405,"mozilla::ContainerState::CreateMaskLayer"]

While we have limited ability to order on the information here, in my graph-support branch I track more per-symbol metadata and we can maybe do something more semantic than just longest string or counting the namespace delimiters.

1: C++ analysis files undergo 2 orderings in their lives:

• MozsearchIndexer.cpp's sort merging is string-based ordering.
• The merge-analyses step is what we're looking at here. Its logic is at https://github.com/mozsearch/mozsearch/blob/cf247e4b010b5da900345e253e105e7410c866ac/tools/src/bin/merge-analyses.rs#L42 and means file contents basically look like:
  • All of the target records from the first platform's analysis file.
  • The target records for each additional platform file that wasn't seen before as the file is processed. So multiple rounds of sorted target records.
  • The source records in sorted order, after merging.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Uh, and I mention all this in case you want to try your hand at a fix. Although I mention the graph-support branch, it's far from imminent. https://github.com/mozsearch/mozsearch has (just updated!) docs on how to set up searchfox in a VM (no longer requires virtualbox!). If the existing test corpus doesn't have coverage for this case, it should be reasonably feasible to add one. (I can also set you up with AWS creds if you want to do dev pushes to iterate once you've gotten things looking okay locally on the test corpus.)

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

I'm fixing this in bug 1776522 now as we now will make all decisions about menu ordering in the JS content logic, although it's possible I'll bungle the heuristics!