Remove preprocessor directives from browser code

NEW
Unassigned

Status

()

Firefox
Build Config
3 years ago
2 years ago

People

(Reporter: mikedeboer, Unassigned)

Tracking

(Depends on: 5 bugs, {meta})

32 Branch
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Preprocessor directive in Javascript and CSS code to include and/ or exclude code from a firefox build creates the following list of - non exhaustive - problems:

 - In JS, stack traces often do not match the actual line numbers, which adds a manual extra step to backtrace where the code is actually hit.
 - A build step is necessary to apply changes made in the source code.
 - The previous point also means that a developer needs to memorize for which part of the browser she/ he is working on a (re-)build is necessary.
 - A build step slows down fast (re-)iterative development and rapid prototyping.

Wherever possible we should be trying minimize or even eliminate the use of `#include` and `#ifdef` preprocessors throughout the 'browser/' sub-tree.

'Plan de campagne' as it currently stands:
 - Write a static analysis tool in JS, running on XPCShell to read, parse and analyze the browser JS source files. It will operate in three phases:
    1) List & count the current scope of use of preprocessor directives
    2) For source files that are #include'd, list & count the number of implicit global variables
    3) Track the numbers over time.
 - Depending on practical feasibility, write a source refactor tool in JS, running on XPCShell to use the data produced by aforementioned script to apply (minor) code modifications that are verifiable correct.
 - Hook the analysis script to automation as a pre-receive hook in Mercurial, to prevent adding more preprocessor dependent code. It is possible to use another hook - the earlier we can run the script the better - but for that to know we'd like to request gps' help.
 - Spin off bugs based on the initial results of the analysis script; the intention is to slowly iterate towards moving all #include'd files to be JS modules that are retrieved lazily by browser.js. The #ifdef removal is second priority.

djc and mikedeboer are working on the analysis script - based on Reflect.parse - which will be the cornerstone of this endeavor. The bugs that will be filed based on the results it yields will be filed blocking this (meta) bug.
Flags: qe-verify-
Flags: firefox-backlog+
Depends on: 1150860
(In reply to Mike de Boer [:mikedeboer] from comment #0)
> Preprocessor directive in Javascript and CSS code to include and/ or exclude
> code from a firefox build creates the following list of - non exhaustive -
> problems:

I meant that the *list* is non-exhaustive.
Note that many preprocessor directives in JS code can be replaced with
- AppConstants.jsm (which relies itself on preprocessor directives, but we could reimplement it in C++ if needed);
- OS.Constants (which is a bit heavyweight to use right now, because it is tied to OS.File, but we could easily extract it to a more lightweight module).
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #2)
> Note that many preprocessor directives in JS code can be replaced with
> - AppConstants.jsm (which relies itself on preprocessor directives, but we
> could reimplement it in C++ if needed);

I think that's great! Keeping that file in JS is fine, methinks.
I wonder if it makes sense to separate the eradication of #include and #ifdef directives into separate bugs, since they will probably need different strategies.
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #2)
> Note that many preprocessor directives in JS code can be replaced with
> - AppConstants.jsm (which relies itself on preprocessor directives, but we
> could reimplement it in C++ if needed);

Hey, look, I already started on that work:
https://dxr.mozilla.org/mozilla-central/source/xpcom/system/nsIXULRuntime.idl#139

I needed this to remove preprocessing in a bunch of test files. Adding more things there is trivial, we could easily use that for everything in AppConstants.jsm:
https://hg.mozilla.org/mozilla-central/rev/a34e6d5519ef
Depends on: 1195780
Depends on: 1195962
Depends on: 1195965
I've started filing deps to remove these. I'll try to finish this tomorrow / the rest of this week. Please feel free to take and/or sign up as mentors for any of them. I'll post to fx-dev once browser/ is covered.
Just a reminder: as we eradicate preprocessor directives completely from files, we should also update the associated moz.build/jar.mn to avoid the preprocessing and hence speeding up build times and get the full benefits of the removal of the directives.
Depends on: 1197745
Depends on: 1197754
Depends on: 1197755
Depends on: 1197758
Depends on: 1197763
Depends on: 1197764
Depends on: 1228655
Depends on: 1228627
You need to log in before you can comment on or make changes to this bug.