1421364 - Code inside C++ blocks in XPIDL not linked to anything

Status: ASSIGNED

(Webtools :: Searchfox, enhancement)

Description

[Andrew McCreight [:mccr8]]

nsIXPConnect.idl has this for the interface nsIXPConnectWrappedNative

%{C++
    /**
     * Faster access to the native object from C++.  Will never return null.
     */
    nsISupports* Native() const { return mIdentity; }

protected:
    nsCOMPtr<nsISupports> mIdentity;
public:
%}

Native() and mIdentity are not links that I can use to search for their uses.

Comment 1

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

This seems like something where the XPIDL binding generator could be emitting C(++) preprocessor #line directives and then things maybe will work magically?

Comment 2

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

It seems like bug 1792334 mooted the comment 0 use case, but a search of context:8 path:.idl %{C++ shows there's plenty of chaotic lawful uses of the mechanism remaining.

The #line mechanism I propose in comment 1 I think would work if ipdl.py understood how to merge its outputs, but I think the side effects would be too weird because then there would be no analysis records

I think a way to make this work now would be to define a transclusion-ish record so that the IDL analysis could be like "hey, for this locRange, the actual analysis records are found in the generated header file". This could do double-duty:

• For output-file, we would use the "source" records to process the tokens.
• For crossref, we could accumulate the transclusion records and perform a fixup pass on the "target" records in a pass after ingesting all the analysis files so that we re-target references for defs/decls in the generated header file inside the transclusion range to the IDL file range. (There are also ways we could avoid doing it in a fix-up pass which could resources used slightly, but the post-pass is simpler.)

Note that tree-sitter's ability to parse nested languages is not sufficient for something like this where the C++ inclusions are making assumptions about the surrounding context that the parser cannot reasonably know. If we're giving tree-sitter both the IDL file and the generated header to let tree-sitter be able to parse the C++ in context, that's just the transclusion mechanism above, but worse. Also, it wouldn't provide any benefits for working with only the git source without analysis data (the main reason to favor tree-sitter based approaches), so there would be little point in trying this. That said, it could still make sense to move IPDL parsing to using tree-sitter and it could generate the transclusion records, we just never would use them to try and parse the C++.

Comment 3

[Tooru Fujisawa [:arai]]

Given that we'll want to reflect the analysis record to both the generated header and the IDL, just copying the analysis records from header to IDL would be a simple approach.
Also, if we're to use #line directive, we need to restore to the original file/line after the CDATA section, which is slightly complex in the current xpidl code.

I'll let the xpidl header file generator to put a dedicate comment // CDATA: {first_line}:{last_line} above the CDATA section, so that we can map the lines, and the idl-analyze script can copy the analysis record from the header to the IDL file, while mapping the line numbers.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1421364 - Part 1: Do not remove newlines from CDATA to make the line numbers match between the XPIDL file and the generated header. r?mccr8!

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1421364 - Part 2: Put comments for CDATA in the XPIDL headers. r?mccr8!

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: GitHub Pull Request

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1421364 - Part 3: Fix the line count for CDATA. r?mccr8!

Comment 8

[Andrew McCreight [:mccr8]]

IMO these patches should really go in a new XPCOM bug blocking this one, for greater visibility. I'm not too familiar with XPIDL code gen so it may take me a bit to get to the reviews. Sorry.

Comment 9

[Phabricator Automation]

Comment on attachment 9543862 [details]
Bug 1421364 - Part 1: Do not remove newlines from CDATA to make the line numbers match between the XPIDL file and the generated header. r?mccr8!

Revision D282597 was moved to bug 2016615. Setting attachment 9543862 [details] to obsolete.

Comment 10

[Phabricator Automation]

Comment on attachment 9543863 [details]
Bug 1421364 - Part 2: Put comments for CDATA in the XPIDL headers. r?mccr8!

Revision D282598 was moved to bug 2016615. Setting attachment 9543863 [details] to obsolete.

Comment 11

[Phabricator Automation]

Comment on attachment 9544511 [details]
Bug 1421364 - Part 3: Fix the line count for CDATA. r?mccr8!

Revision D282998 was moved to bug 2016615. Setting attachment 9544511 [details] to obsolete.