1432300 - Conflicting highlighting when hovering over calls to JS::ProfilingFrameIterator::getPhysicalFrameAndEntry

Status: RESOLVED FIXED

(Webtools :: Searchfox, defect)

Description

[Markus Stange [:mstange]]

Attachment: Screen Shot 2018-01-22 at 4.32.27 PM.png

Steps to reproduce:
 1. Go to https://searchfox.org/mozilla-central/source/js/src/vm/Stack.cpp and search for the string "Maybe<Frame> physicalFrame = getPhysicalFrameAndEntry(&entry);"
 2. Hover over "getPhysicalFrameAndEntry" in that line.

This causes the words "mozilla" in the implementation of getPhysicalFrameAndEntry to be highlighted, for the "return mozilla::Some(frame);" lines.

Conversely, hovering over the "mozilla" words also highlights calls to getPhysicalFrameAndEntry.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

It looks like both of "mozilla" and "getPhysicalFrameAndEntry" have a data-id of _ZN7mozilla5MaybeC1EONS_5MaybeIT_EE which corresponds to the mozilla::Maybe constructor. And in fact, when clicking on those terms, the Maybe constructor shows up in the context menu for both.

The data-id is picked at [1] as the first entry in the list of symbols associated with that token. And looking at the list of symbols for that token:

{"loc":"02112:33-57","source":1,"syntax":"use,constructor","pretty":"constructor mozilla::Maybe::Maybe<T>","sym":"_ZN7mozilla5MaybeC1EONS_5MaybeIT_EE"}
{"loc":"02112:33-57","source":1,"syntax":"use,function","pretty":"function JS::ProfilingFrameIterator::getPhysicalFrameAndEntry","sym":"_ZNK2JS22ProfilingFrameIterator24getPhysicalFrameAndEntryEPN2js3jit18JitcodeGlobalEntryE"}
{"loc":"02112:33-57","target":1,"kind":"use","pretty":"JS::ProfilingFrameIterator::getPhysicalFrameAndEntry","sym":"_ZNK2JS22ProfilingFrameIterator24getPhysicalFrameAndEntryEPN2js3jit18JitcodeGlobalEntryE","context":"JS::ProfilingFrameIterator::extractStack","contextsym":"_ZNK2JS22ProfilingFrameIterator12extractStackEPNS0_5FrameEjj"}
{"loc":"02112:33-57","target":1,"kind":"use","pretty":"mozilla::Maybe::Maybe<T>","sym":"_ZN7mozilla5MaybeC1EONS_5MaybeIT_EE","context":"JS::ProfilingFrameIterator::extractStack","contextsym":"_ZNK2JS22ProfilingFrameIterator12extractStackEPNS0_5FrameEjj"}

we can see that the Maybe constructor is the first one in the list. So that explains why it's happening. The fix here is probably to not just pick the first symbol in the list at [1], but instead pick the most "relevant" symbol (for some definition of relevant). In this case it should pick the function symbol instead of the constructor symbol.

[1] https://github.com/mozsearch/mozsearch/blob/56136fa9446b444fe7c8fe65419354226f642e6c/tools/src/format.rs#L105

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Note also that the output dumped to the analysis file is sorted lexicographically [1] and so the other possible fix of changing the order of symbols in the analysis file is harder to do.

[1] https://github.com/mozsearch/mozsearch/blob/56136fa9446b444fe7c8fe65419354226f642e6c/clang-plugin/MozsearchIndexer.cpp#L452-L455

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I wrote a patch that improves this behaviour a little bit. It's the best heuristic I could come up with, maybe as we find more cases where it falls down we can refine it.

PR is at https://github.com/mozsearch/mozsearch/pull/83 and I deployed it to dev.searchfox.org, so the issue as described in comment 0 is fixed there.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

The fix for this is deployed now. There might still be places where the highlight isn't great, please file additional bugs if you run into them and we can tweak the heuristics further.