Closed Bug 1282172 Opened 3 years ago Closed 2 years ago

Enums generated by macros: macro doesn't show up as definition

Categories

(Webtools :: Searchfox, defect)

defect
Not set

Tracking

(firefox59 fixed)

RESOLVED FIXED
Tracking Status
firefox59 --- fixed

People

(Reporter: billm, Assigned: kats)

References

Details

Attachments

(1 file)

I'll take a look at this
Assignee: nobody → bugmail
I have a fix that seems to work, topmost patch at: https://github.com/staktrace/mozsearch/commits/analysis

However I'd like to get a better testing infrastructure in place to make sure this doesn't regress stuff before I try to get it merged.
I ran the searchfox indexing taskcluster job (coming in bug 1418188) without [1] and with [2] my patch for this bug. I then downloaded the index artifacts from both jobs and compared the results. The diff showed pretty much what I was hoping for - we get a bunch of enum constant defs showing up in places where the enum constants are generated by macros.

It also moves some existing enum constant definitions, in cases where the entire enum is generated by a macro. For example in lz4.c, it moves the def of the enum constant LZ4_createStream::LZ4_static_assert from line 292 [3] to line 940 [4]. This sort of makes sense since the enum constant is defined at the use site of the macro, rather than the definition of the macro. And even if this is undesirable, there's only three files where this happens (lz4.c, TestTypedEnum.cpp, gmock-internal-utils.h) and ~50 files where we get new enum constant definitions being found, so overall I think the change is a pretty good win.

The only question I have is about the landing procedure for this patch, since we now have two copies of the indexer plugin - one is in m-c and one is in the mozsearch repo. Do I need patches for both, or are we mirroring one to the other? I'm not sure which is canonical if so.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=206f2175a511101353d6cb1807f8c48c05993db8
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=3741a89d1cbc3464e7d70bd05f601e11d84c0612
[3] https://searchfox.org/mozilla-central/rev/c9214daa09e3eb8246b1448e469df1652a140825/mfbt/lz4.c#292
[4] https://searchfox.org/mozilla-central/rev/c9214daa09e3eb8246b1448e469df1652a140825/mfbt/lz4.c#940
I think we should land in both places for now. I'm not sure how long we'll have to preserve two copies. I've already turned off semantic indexing for comm-central since it takes forever and I doubt anyone cares. I'm not sure what to do about NSS though.
Comment on attachment 8929800 [details]
Bug 1282172 - Detect enum constants generated by macros when generating the Searchfox index.

https://reviewboard.mozilla.org/r/201032/#review206204
Attachment #8929800 - Flags: review?(wmccloskey) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/db0886825a54
Detect enum constants generated by macros when generating the Searchfox index. r=billm
https://hg.mozilla.org/mozilla-central/rev/db0886825a54
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.