1583635 - Show expansion of C++ macros

Status: NEW

(Webtools :: Searchfox, enhancement)

Description

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

Trying to figure out what ISUPPORTS and related cycle-collecting macros do could be made easier by showing what the macros expand to (in places where compilation expands the macro).

Comment 1

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

Shelving for now. Various in-progress attempts all muddled together are at https://github.com/asutherland/mozsearch/tree/expand-macros

The main complication seemed to be that:

• Macro expansion hooks fired prior to the actual work of the expansion.
• The preprocessing record that was potentially supposed to exist no longer existed at the end of the file analysis pass.
• I didn't quite intuitively understand what the sourcemanager was up to.

The branch has some hacks that know how to produce a post-macro expansion pretty print of impacted code, but that doesn't really suffice for our use-cases.

I think a way that might work would basically be to:

• Scan the sourcemanager's data structure for the file and for any line that experienced a macro expansion, emit a new record type that provides the post-expansion version of the line.
• When emitting any token that was also present in macro expansions, provide the post-expansion location(s?) of the token as secondary metadata on the normal records. Right now we normalize to pre-expansion locations (so the argument), but I think we'd want to be able to provide at least clickability even if we can't syntax highlight. (Although we probably could syntax highlight if we view macro expansions as an alternate parse stream. Also, it's probably fine if macro expansion syntax highlighting is super buggy.)
• When rendering to HTML, put the macro expanded lines in collapsible divs with an alternate presentation.

Comment 2

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

One thing I don't think I looked at sufficiently is if there's existing machinery relating to the clang -E output that just stops after the preprocessor stage that can be reused. Even if that output is fundamentally lossy, it seems likely a correspondence could be hackily re-established between that output and the SourceManager's canonical locations.

Comment 3

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

It turns out clang has an HTMLRewriter class which has a method clang::html::HighlightMacros that knows how to produce an HTML formatted version of the macro expansions. A simple starting point might be to just try and have it generate that HTML for us and include that as a monolithic "diagnostic" record or some other kind of meta-data. With significant extra work we could create a modified version of the logic that would emit a token and analysis record stream, I guess nested inside a single JSON record again for sanity purposes which output-file would know how to handle (semi-recursively).

Comment 4

[Nicolas Guichard]

Attachment: WIP GitHub Pull Request

Comment 5

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

Unfortunately we ran out of funding for Nicolas to continue his fantastic work here so I am going to land the work from 1583635 (on a hobby basis) to try and get this landed as an alpha-gated feature[1] given how useful this capability already is.

1: Blockers to advancing to beta would be:

• Accessibility verification with NVDA. The general plan has been that the screen reader UX should resemble blame.
• Handle macro expansions that have been expanded multiple times but with different underlying definitions as part of an ".inc" file that is itself multiply included.
• Improved matching of tokens in the expanded text. As noted in https://github.com/mozsearch/mozsearch/pull/679#issuecomment-1866207558 a really amazing aspect of the existing implementation is that it attempts to match up records from within the expansion with tokens in the expansion so that the expansion is more than just the expanded, syntax-highlighted text (which would itself still be incredibly useful).

The feature gate rationale is documented in https://searchfox.org/mozilla-central/pages/settings.html although my rationale here is more like:

• Accessibility needs to be tested with NVDA before reaching beta (and honestly before reaching alpha).
• If there's stuff people would file correctness bugs for, it definitely needs to be alpha. This is partially covered by the existing "brittle" phrasing.