Closed Bug 1417586 Opened 7 years ago Closed 9 months ago

Add support for preprocessing of IPDL files

Categories

(Webtools :: Searchfox, enhancement)

Product:
Webtools
Component:
Searchfox
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mccr8, Assigned: asuth)

References

(Blocks 1 open bug)

Details

There's a patch up in bug 1274518 for preprocessing IPDL files, which will presumably require SearchFox to be updated. It looks like the preprocessing happens during the build step, so hopefully SearchFox can simply grab the preprocessed output.

This eventually landed, and is being used for PMediaTransport.ipdl. Ideally, I could just pass in the path for the preprocessed version of the file from the objdir instead of the other one. I'll have to figure out how to do that. Hopefully the preprocessing just dumps everything in a single directory in the objdir, and then I could just look there and refer to that version of any IPDL file instead of the other one. On the other hand, this may mess up line numbers...

Assignee: nobody → continuation
Assignee: continuation → nobody

I have a fairly simple hacky fix to the IPDL parser for this (with "test coverage") that just has us treat preprocessor lines like they're comments. This is effectively the same thing we do in searchfox's JS analysis which also is processed on the indexer without any per-platform context. If we implemented bug 1661067 having us use the in-tree codegen to generate the analysis files as part of the build and then ran merge-analyses on the resulting per-platform analysis files, then it could make more sense for us to care about what the preprocessor did. (Note that while our IPDL processing does care about the C++ symbols, it's acting on the post-merge C++ files.)

Because the fix is fairly minimal and git submodules are a major PITA that pose real potential regression risk when jumping around between branches, I'm just going to proactively land my changes but will happily address any post-landing review comments.

The fix makes both PContent.ipdl and the examplar PMediaTransport.ipdl parse, but we still need to figure out the right place to find the C++ files we ingest. I have a patch that has us use the objdir-files list to try and find the file. This provides semantic linkage for PContent as can be seen on the asf PContent.ipdl, but although the asf PMediaTransport.ipdl parses, it's not useful because we get no semantic linkage from the files we pick. This may be a direct call thing as covered by Bug 1555857? Honestly, it's not immediately clear to me, but the output from the indexer for PMT is:

Analyzing "/mnt/index-scratch/mozilla-central/git/dom/media/webrtc/PMediaTransport.ipdl"
  Reading Parent header "/mnt/index-scratch/mozilla-central/analysis/__GENERATED__/ipc/ipdl/_ipdlheaders/mozilla/dom/PMediaTransportParent.h"
  Reading Parent impl header "/mnt/index-scratch/mozilla-central/analysis/dom/media/webrtc/MediaTransportParent.h"
  Reading Child header "/mnt/index-scratch/mozilla-central/analysis/__GENERATED__/ipc/ipdl/_ipdlheaders/mozilla/dom/PMediaTransportChild.h"
  Reading Child impl header "/mnt/index-scratch/mozilla-central/analysis/dom/media/webrtc/MediaTransportChild.h"
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent13RecvGetIceLogE or _ZN7mozilla3dom21PMediaTransportParent13RecvGetIceLogE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent15RecvClearIceLogE or _ZN7mozilla3dom21PMediaTransportParent15RecvClearIceLogE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent20RecvEnterPrivateModeE or _ZN7mozilla3dom21PMediaTransportParent20RecvEnterPrivateModeE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent19RecvExitPrivateModeE or _ZN7mozilla3dom21PMediaTransportParent19RecvExitPrivateModeE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent16RecvCreateIceCtxE or _ZN7mozilla3dom21PMediaTransportParent16RecvCreateIceCtxE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent16RecvSetIceConfigE or _ZN7mozilla3dom21PMediaTransportParent16RecvSetIceConfigE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent18RecvSetProxyConfigE or _ZN7mozilla3dom21PMediaTransportParent18RecvSetProxyConfigE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent30RecvEnsureProvisionalTransportE or _ZN7mozilla3dom21PMediaTransportParent30RecvEnsureProvisionalTransportE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent41RecvSetTargetForDefaultLocalAddressLookupE or _ZN7mozilla3dom21PMediaTransportParent41RecvSetTargetForDefaultLocalAddressLookupE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent21RecvStartIceGatheringE or _ZN7mozilla3dom21PMediaTransportParent21RecvStartIceGatheringE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent21RecvActivateTransportE or _ZN7mozilla3dom21PMediaTransportParent21RecvActivateTransportE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent26RecvRemoveTransportsExceptE or _ZN7mozilla3dom21PMediaTransportParent26RecvRemoveTransportsExceptE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent18RecvStartIceChecksE or _ZN7mozilla3dom21PMediaTransportParent18RecvStartIceChecksE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent14RecvSendPacketE or _ZN7mozilla3dom21PMediaTransportParent14RecvSendPacketE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent19RecvAddIceCandidateE or _ZN7mozilla3dom21PMediaTransportParent19RecvAddIceCandidateE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent22RecvUpdateNetworkStateE or _ZN7mozilla3dom21PMediaTransportParent22RecvUpdateNetworkStateE
No analysis target found for recv: _ZN7mozilla3dom20MediaTransportParent15RecvGetIceStatsE or _ZN7mozilla3dom21PMediaTransportParent15RecvGetIceStatsE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild15RecvOnCandidateE or _ZN7mozilla3dom20PMediaTransportChild15RecvOnCandidateE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild20RecvOnAlpnNegotiatedE or _ZN7mozilla3dom20PMediaTransportChild20RecvOnAlpnNegotiatedE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild26RecvOnGatheringStateChangeE or _ZN7mozilla3dom20PMediaTransportChild26RecvOnGatheringStateChangeE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild27RecvOnConnectionStateChangeE or _ZN7mozilla3dom20PMediaTransportChild27RecvOnConnectionStateChangeE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild20RecvOnPacketReceivedE or _ZN7mozilla3dom20PMediaTransportChild20RecvOnPacketReceivedE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild22RecvOnEncryptedSendingE or _ZN7mozilla3dom20PMediaTransportChild22RecvOnEncryptedSendingE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild17RecvOnStateChangeE or _ZN7mozilla3dom20PMediaTransportChild17RecvOnStateChangeE
No analysis target found for recv: _ZN7mozilla3dom19MediaTransportChild21RecvOnRtcpStateChangeE or _ZN7mozilla3dom20PMediaTransportChild21RecvOnRtcpStateChangeE
Assignee: nobody → bugmail
Status: NEW → ASSIGNED

Landed:

I'll keep an eye on the config1 nightly which should start shortly.

That's a clever hack. It'll for sure break on some files, but if it gets us more files working that's great. I had a few minor comments on your patch.

(In reply to Andrew McCreight [:mccr8] from comment #4)

That's a clever hack. It'll for sure break on some files, but if it gets us more files working that's great. I had a few minor comments on your patch.

It definitely helps that files with any preprocessing are already 100% broken, so things can only improve!

Are there any specific files where you expect breakage? (Well, they should already be broken on asuth.searchfox.org, although I think I was a few commits behind trunk there, so such things could be orthogonal.)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #5)

(In reply to Andrew McCreight [:mccr8] from comment #4)
Are there any specific files where you expect breakage? (Well, they should already be broken on asuth.searchfox.org, although I think I was a few commits behind trunk there, so such things could be orthogonal.)

I was concerned about what might happen around #else stuff like if a message is defined slightly differently on different platforms. However, looking at the uses of # in .ipdl files, it looks like that might only be an issue for the definition of the IPDL struct SystemFontListEntry so hopefully it'll be fine.

Blocks: 1859884

I landed post-landing review fixes:

Bug 1859884 has been followed for future work in this area and so we can close this bug for clarity since I've also verified PContent.ipdl is now happy on searchfox.org

Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.