From 12640bca82f90f849ff657aaa3dfe179a644f99c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Aug 2019 05:16:55 +0000 Subject: [PATCH] Bug 1563555 - Generate static pref getters usable from Rust code. r=glandium This patch introduces a new Rust crate called `static_prefs`. It also changes generate_static_pref_list.py to generate two new files. - StaticPrefsCGetters.cpp: contains C getters, which are just wrappers around the C++ getters. This is included into Preferences.cpp. - static_prefs.rs: contains declarations for the C getters, plus the `pref!` macro which provides nice syntax for calling the C getters. This is included into static_prefs/src/lib.rs. The new code is only generated for prefs marked with the new `rust` field in the YAML. It's opt-in because there's no point generating additional code for 900+ static prefs when only about 20 are currently used from Rust. This patch only marks a single pref (`browser.display.document_color_use`) with `rust: true`. That pref isn't accessed from Rust code in this patch, but it's necessary because the generated Rust code is invalid if there are zero Rust-accessed prefs. (The next patch will access that pref and others from Rust code. Differential Revision: https://phabricator.services.mozilla.com/D40791 --HG-- extra : moz-landing-system : lando --- Cargo.lock | 5 + modules/libpref/Preferences.cpp | 4 + modules/libpref/docs/index.rst | 21 ++-- modules/libpref/init/StaticPrefList.yaml | 10 +- .../libpref/init/generate_static_pref_list.py | 97 ++++++++++++++++--- modules/libpref/init/static_prefs/Cargo.toml | 9 ++ modules/libpref/init/static_prefs/src/lib.rs | 11 +++ modules/libpref/moz.build | 31 +++--- .../test/test_generate_static_pref_list.py | 60 ++++++++++++ toolkit/library/rust/shared/Cargo.toml | 1 + toolkit/library/rust/shared/lib.rs | 1 + 11 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 modules/libpref/init/static_prefs/Cargo.toml create mode 100644 modules/libpref/init/static_prefs/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 133155f48d65..ac0dd097dbb6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1270,6 +1270,7 @@ dependencies = [ "rsdparsa_capi 0.1.0", "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "shift_or_euc_c 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "static_prefs 0.1.0", "storage 0.1.0", "webrender_bindings 0.1.0", "xpcom 0.1.0", @@ -2916,6 +2917,10 @@ name = "stable_deref_trait" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "static_prefs" +version = "0.1.0" + [[package]] name = "storage" version = "0.1.0" diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 25b7db47a8c8..15d3595ca6c3 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -5605,3 +5605,7 @@ namespace mozilla { void UnloadPrefsModule() { Preferences::Shutdown(); } } // namespace mozilla + +// This file contains the C wrappers for the C++ static pref getters, as used +// by Rust code. +#include "init/StaticPrefsCGetters.cpp" diff --git a/modules/libpref/docs/index.rst b/modules/libpref/docs/index.rst index 9482ca35bc2c..8e3e888f506b 100644 --- a/modules/libpref/docs/index.rst +++ b/modules/libpref/docs/index.rst @@ -109,6 +109,8 @@ However, there are two exceptions to this. The narrow exception is that the Servo traversal thread is allowed to get pref values. This only occurs when the main thread is paused, which makes it safe. +(Note: `bug 1474789 `_ +indicates that this may not be true.) The broad exception is that static prefs can have a cached copy of a pref value that can be accessed from other threads. See below. @@ -142,12 +144,12 @@ bool/int/float values, not strings or complex values. Each mirror variable is read-only, accessible via a getter function. -Mirror variables have two benefits. First, they allow C++ code to get the pref -value directly from the variable instead of requiring a slow hash table -lookup, which is important for prefs that are consulted frequently. Second, -they allow C++ code to get the pref value off the main thread. The mirror -variable must have an atomic type if it is read off the main thread, and -assertions ensure this. +Mirror variables have two benefits. First, they allow C++ and Rust code to get +the pref value directly from the variable instead of requiring a slow hash +table lookup, which is important for prefs that are consulted frequently. +Second, they allow C++ and Rust code to get the pref value off the main thread. +The mirror variable must have an atomic type if it is read off the main thread, +and assertions ensure this. Note that mirror variables could be implemented via vanilla callbacks without API support, except for one detail: libpref gives their callbacks higher @@ -228,6 +230,13 @@ vs. a notebook). Prefs are not synced on mobile. +Rust +---- +Static prefs mirror variables can be accessed from Rust code via the +``static_prefs::pref!`` macro. Other prefs currently cannot be accessed. Parts +of libpref's C++ API could be made accessible to Rust code fairly +straightforwardly via C bindings, either hand-made or generated. + Cost of a pref -------------- The cost of a single pref is low, but the cost of several thousand prefs is diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index ef9eadba4ee1..026c18158746 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -3,7 +3,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. */ # This file defines static prefs, i.e. those that are defined at startup and -# used entirely or mostly from C++ code. +# used entirely or mostly from C++ and/or Rust code. # # The file is separated into sections, where each section contains a group of # prefs that all share the same first segment of their name -- all the "gfx.*" @@ -35,6 +35,7 @@ # mirror: # mandatory # do_not_use_directly: # optional # include: # optional +# rust: # optional # # - `name` is the name of the pref, without double-quotes, as it appears # in about:config. It is used in most libpref API functions (from both C++ @@ -81,7 +82,7 @@ # Note that Rust code must access the mirror variable directly, rather than # via the getter function. # -# - `do_not_use_directly` dictates if `_DoNotUseDirectly` should be appended to +# - `do_not_use_directly` indicates if `_DoNotUseDirectly` should be appended to # the name of the getter function. This is simply a naming convention # indicating that there is some other wrapper getter function that should be # used in preference to the normal static pref getter. Defaults to `false` if @@ -92,6 +93,10 @@ # compile correctly, e.g. because it refers to a code constant. System # headers should be surrounded with angle brackets, e.g. ``. # +# - `rust` indicates if the mirror variable is used by Rust code. If so, it +# will be usable via the `static_prefs::pref!` macro, e.g. +# `static_prefs::pref!("layout.css.font-display.enabled")`. +# # The getter function's base name is the same as the pref's name, but with # '.' or '-' chars converted to '_', to make a valid identifier. For example, # the getter for `foo.bar_baz` is `foo_bar_baz()`. This is ugly but clear, @@ -741,6 +746,7 @@ type: uint32_t value: 0 mirror: always + rust: true - name: browser.display.focus_ring_on_anything type: bool diff --git a/modules/libpref/init/generate_static_pref_list.py b/modules/libpref/init/generate_static_pref_list.py index 883064fd6493..295c011a1a7c 100644 --- a/modules/libpref/init/generate_static_pref_list.py +++ b/modules/libpref/init/generate_static_pref_list.py @@ -19,31 +19,42 @@ VALID_KEYS = { 'mirror', 'do_not_use_directly', 'include', + 'rust', } +# Each key is a C++ type; its value is the equivalent non-atomic C++ type. VALID_BOOL_TYPES = { - 'bool', + 'bool': 'bool', # These ones are defined in StaticPrefsBase.h. - 'RelaxedAtomicBool', - 'ReleaseAcquireAtomicBool', - 'SequentiallyConsistentAtomicBool', + 'RelaxedAtomicBool': 'bool', + 'ReleaseAcquireAtomicBool': 'bool', + 'SequentiallyConsistentAtomicBool': 'bool', } -VALID_TYPES = VALID_BOOL_TYPES.union({ - 'int32_t', - 'uint32_t', - 'float', +VALID_TYPES = VALID_BOOL_TYPES.copy() +VALID_TYPES.update({ + 'int32_t': 'int32_t', + 'uint32_t': 'uint32_t', + 'float': 'float', # These ones are defined in StaticPrefsBase.h. - 'String', - 'RelaxedAtomicInt32', - 'RelaxedAtomicUint32', - 'ReleaseAcquireAtomicInt32', - 'ReleaseAcquireAtomicUint32', - 'SequentiallyConsistentAtomicInt32', - 'SequentiallyConsistentAtomicUint32', - 'AtomicFloat', + 'RelaxedAtomicInt32': 'int32_t', + 'RelaxedAtomicUint32': 'uint32_t', + 'ReleaseAcquireAtomicInt32': 'int32_t', + 'ReleaseAcquireAtomicUint32': 'uint32_t', + 'SequentiallyConsistentAtomicInt32': 'int32_t', + 'SequentiallyConsistentAtomicUint32': 'uint32_t', + 'AtomicFloat': 'float', + 'String': None, }) +# Map non-atomic C++ types to equivalent Rust types. +RUST_TYPES = { + 'bool': 'bool', + 'int32_t': 'i32', + 'uint32_t': 'u32', + 'float': 'f32', +} + FIRST_LINE = '// This file was generated by generate_static_pref_list.py. DO NOT EDIT.' MIRROR_TEMPLATES = { @@ -85,6 +96,12 @@ STATIC_PREFS_GROUP_H_TEMPLATE2 = '''\ #endif // mozilla_StaticPrefs_{group}_h ''' +STATIC_PREFS_C_GETTERS_TEMPLATE = '''\ +extern "C" {typ} StaticPrefs_{full_id}() {{ + return mozilla::StaticPrefs::{full_id}(); +}} +''' + def error(msg): raise ValueError(msg) @@ -180,6 +197,16 @@ def check_pref_list(pref_list): error('`include` value `{}` starts with `<` but does not ' 'end with `>` for pref `{}`'.format(include, name)) + # Check 'rust' if present. + if 'rust' in pref: + rust = pref['rust'] + if type(rust) != bool: + error('non-boolean `rust` value `{}` for pref `{}`' + .format(rust, name)) + if rust and mirror == 'never': + error('`rust` uselessly set with `mirror` value `never` for ' + 'pref `{}`'.format(pref['name'])) + prev_pref = pref @@ -193,6 +220,14 @@ def generate_code(pref_list): # group. static_pref_list_group_h = defaultdict(lambda: [FIRST_LINE, '']) + # StaticPrefsCGetters.cpp contains C getters for all the mirrored prefs, + # for use by Rust code. + static_prefs_c_getters_cpp = [FIRST_LINE, ''] + + # static_prefs.rs contains C getter declarations and a macro. + static_prefs_rs_decls = [] + static_prefs_rs_macro = [] + # Generate the per-pref code (spread across multiple files). for pref in pref_list: name = pref['name'] @@ -201,6 +236,7 @@ def generate_code(pref_list): mirror = pref['mirror'] do_not_use_directly = pref.get('do_not_use_directly') include = pref.get('include') + rust = pref.get('rust') base_id = mk_id(pref['name']) full_id = base_id @@ -236,6 +272,20 @@ def generate_code(pref_list): value=value, )) + if rust: + # Generate the C getter. + static_prefs_c_getters_cpp.append( + STATIC_PREFS_C_GETTERS_TEMPLATE.format(typ=VALID_TYPES[typ], full_id=full_id)) + + # Generate the C getter declaration, in Rust. + decl = ' pub fn StaticPrefs_{full_id}() -> {typ};' + static_prefs_rs_decls.append( + decl.format(full_id=full_id, typ=RUST_TYPES[VALID_TYPES[typ]])) + + # Generate the Rust macro entry. + macro = ' ("{name}") => (unsafe {{ $crate::StaticPrefs_{full_id}() }});' + static_prefs_rs_macro.append(macro.format(name=name, full_id=full_id)) + # Delete this so that `group` can be reused below without Flake8 # complaining. del group @@ -271,6 +321,13 @@ def generate_code(pref_list): static_prefs_group_h[group].append('') static_prefs_group_h[group].append(STATIC_PREFS_GROUP_H_TEMPLATE2.format(group=group)) + # static_prefs.rs contains the Rust macro getters. + static_prefs_rs = [FIRST_LINE, '', 'extern "C" {'] + static_prefs_rs.extend(static_prefs_rs_decls) + static_prefs_rs.extend(['}', '', '#[macro_export]', 'macro_rules! pref {']) + static_prefs_rs.extend(static_prefs_rs_macro) + static_prefs_rs.extend(['}', '']) + def fold(lines): return '\n'.join(lines) @@ -279,6 +336,8 @@ def generate_code(pref_list): 'static_prefs_all_h': fold(static_prefs_all_h), 'static_pref_list_group_h': {k: fold(v) for k, v in static_pref_list_group_h.items()}, 'static_prefs_group_h': {k: fold(v) for k, v in static_prefs_group_h.items()}, + 'static_prefs_c_getters_cpp': fold(static_prefs_c_getters_cpp), + 'static_prefs_rs': fold(static_prefs_rs), } @@ -324,3 +383,9 @@ def emit_code(fd, pref_list_filename): filename = 'StaticPrefs_{}.h'.format(group) with FileAvoidWrite(filename) as fd: fd.write(text) + + with FileAvoidWrite(os.path.join(init_dirname, 'StaticPrefsCGetters.cpp')) as fd: + fd.write(code['static_prefs_c_getters_cpp']) + + with FileAvoidWrite('static_prefs.rs') as fd: + fd.write(code['static_prefs_rs']) diff --git a/modules/libpref/init/static_prefs/Cargo.toml b/modules/libpref/init/static_prefs/Cargo.toml new file mode 100644 index 000000000000..dbfbcb688b25 --- /dev/null +++ b/modules/libpref/init/static_prefs/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "static_prefs" +version = "0.1.0" +authors = ["Nicholas Nethercote "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/modules/libpref/init/static_prefs/src/lib.rs b/modules/libpref/init/static_prefs/src/lib.rs new file mode 100644 index 000000000000..744bb83a74c2 --- /dev/null +++ b/modules/libpref/init/static_prefs/src/lib.rs @@ -0,0 +1,11 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! This module contains getters for static prefs. +//! +//! The contents of this module are generated by +//! `modules/libpref/init/generate_static_pref_list.py`, from +//! `modules/libpref/init/StaticPrefList.yaml`. + +include!(concat!(env!("MOZ_TOPOBJDIR"), "/modules/libpref/static_prefs.rs")); diff --git a/modules/libpref/moz.build b/modules/libpref/moz.build index e1ee9bcf56ca..880aee05d2b0 100644 --- a/modules/libpref/moz.build +++ b/modules/libpref/moz.build @@ -85,17 +85,15 @@ if CONFIG['FUZZING']: ] pref_groups = tuple(sorted(pref_groups)) -UNIFIED_SOURCES += [ - 'Preferences.cpp', - 'SharedPrefMap.cpp', -] +# Note: generate_static_pref_list.py relies on StaticPrefListAll.h being first. +gen_h = ['init/StaticPrefListAll.h'] +gen_h += ['StaticPrefsAll.h'] +gen_h += ['init/StaticPrefList_{}.h'.format(pg) for pg in pref_groups] +gen_h += ['StaticPrefs_{}.h'.format(pg) for pg in pref_groups] -# Note: generate_static_pref_list relies on StaticPrefListAll.h being first. -genfiles = ['init/StaticPrefListAll.h'] -genfiles += ['init/StaticPrefList_{}.h'.format(pg) for pg in pref_groups] -genfiles += ['StaticPrefsAll.h'] -genfiles += ['StaticPrefs_{}.h'.format(pg) for pg in pref_groups] -genfiles_tuple = tuple(genfiles) +gen_cpp = ['init/StaticPrefsCGetters.cpp'] + +gen_rs = ['static_prefs.rs'] EXPORTS.mozilla += [ 'init/StaticPrefListBegin.h', @@ -104,11 +102,18 @@ EXPORTS.mozilla += [ 'Preferences.h', 'StaticPrefsBase.h', ] -EXPORTS.mozilla += sorted(['!' + gf for gf in genfiles]) +EXPORTS.mozilla += sorted(['!' + g for g in gen_h]) -GENERATED_FILES += [genfiles_tuple] +UNIFIED_SOURCES += [ + 'Preferences.cpp', + 'SharedPrefMap.cpp', +] -static_pref_list = GENERATED_FILES[genfiles_tuple] +gen_all_tuple = tuple(gen_h + gen_cpp + gen_rs) + +GENERATED_FILES += [gen_all_tuple] + +static_pref_list = GENERATED_FILES[gen_all_tuple] static_pref_list.script = 'init/generate_static_pref_list.py:emit_code' static_pref_list.inputs = ['init/StaticPrefList.yaml'] diff --git a/modules/libpref/test/test_generate_static_pref_list.py b/modules/libpref/test/test_generate_static_pref_list.py index b1c6a3a9139a..2498c141003c 100644 --- a/modules/libpref/test/test_generate_static_pref_list.py +++ b/modules/libpref/test/test_generate_static_pref_list.py @@ -30,17 +30,20 @@ good_input = ''' value: -123 mirror: once do_not_use_directly: false + rust: false - mirror: always value: 999 type: uint32_t name: my.uint + rust: true - name: my.float # A comment. type: float # A comment. do_not_use_directly: true # A comment. value: 0.0f # A comment. mirror: once # A comment. + rust: true # A comment. # A comment. - name: my.string @@ -60,6 +63,7 @@ good_input = ''' type: RelaxedAtomicBool value: true mirror: always + rust: true # YAML+Python interprets `10 + 10 * 20` as a string, and so it is printed # unchanged. @@ -177,6 +181,39 @@ good['static_prefs_group_h'] = {'my': '''\ #endif // mozilla_StaticPrefs_my_h '''} +good['static_prefs_c_getters_cpp'] = '''\ +// This file was generated by generate_static_pref_list.py. DO NOT EDIT. + +extern "C" uint32_t StaticPrefs_my_uint() { + return mozilla::StaticPrefs::my_uint(); +} + +extern "C" float StaticPrefs_my_float_AtStartup_DoNotUseDirectly() { + return mozilla::StaticPrefs::my_float_AtStartup_DoNotUseDirectly(); +} + +extern "C" bool StaticPrefs_my_atomic_bool() { + return mozilla::StaticPrefs::my_atomic_bool(); +} +''' + +good['static_prefs_rs'] = '''\ +// This file was generated by generate_static_pref_list.py. DO NOT EDIT. + +extern "C" { + pub fn StaticPrefs_my_uint() -> u32; + pub fn StaticPrefs_my_float_AtStartup_DoNotUseDirectly() -> f32; + pub fn StaticPrefs_my_atomic_bool() -> bool; +} + +#[macro_export] +macro_rules! pref { + ("my.uint") => (unsafe { $crate::StaticPrefs_my_uint() }); + ("my.float") => (unsafe { $crate::StaticPrefs_my_float_AtStartup_DoNotUseDirectly() }); + ("my.atomic.bool") => (unsafe { $crate::StaticPrefs_my_atomic_bool() }); +} +''' + # A lot of bad inputs, each with an accompanying error message. Listed in order # of the relevant `error` calls within generate_static_pref_list.py. bad_inputs = [ @@ -298,6 +335,23 @@ bad_inputs = [ include: ` for ' 'pref `include.value.starts.with`'), + + (''' +- name: non-boolean.rust.value + type: bool + value: true + mirror: always + rust: 1 +''', 'non-boolean `rust` value `1` for pref `non-boolean.rust.value`'), + + (''' +- name: rust.uselessly.set + type: int32_t + value: 0 + mirror: never + rust: true +''', '`rust` uselessly set with `mirror` value `never` for pref ' + '`rust.uselessly.set`'), ] @@ -326,6 +380,12 @@ class TestGenerateStaticPrefList(unittest.TestCase): self.assertEqual(good['static_prefs_group_h']['my'], code['static_prefs_group_h']['my']) + self.assertEqual(good['static_prefs_c_getters_cpp'], + code['static_prefs_c_getters_cpp']) + + self.assertEqual(good['static_prefs_rs'], + code['static_prefs_rs']) + def test_bad(self): 'Test various pieces of bad input.' diff --git a/toolkit/library/rust/shared/Cargo.toml b/toolkit/library/rust/shared/Cargo.toml index 5ac2db0faac7..31a09e2c379c 100644 --- a/toolkit/library/rust/shared/Cargo.toml +++ b/toolkit/library/rust/shared/Cargo.toml @@ -15,6 +15,7 @@ nsstring = { path = "../../../../xpcom/rust/nsstring" } netwerk_helper = { path = "../../../../netwerk/base/rust-helper" } xpcom = { path = "../../../../xpcom/rust/xpcom" } prefs_parser = { path = "../../../../modules/libpref/parser" } +static_prefs = { path = "../../../../modules/libpref/init/static_prefs" } profiler_helper = { path = "../../../../tools/profiler/rust-helper", optional = true } mozurl = { path = "../../../../netwerk/base/mozurl" } webrender_bindings = { path = "../../../../gfx/webrender_bindings", optional = true } diff --git a/toolkit/library/rust/shared/lib.rs b/toolkit/library/rust/shared/lib.rs index 430643e7dbc9..7257de1830fd 100644 --- a/toolkit/library/rust/shared/lib.rs +++ b/toolkit/library/rust/shared/lib.rs @@ -13,6 +13,7 @@ extern crate nserror; extern crate xpcom; extern crate netwerk_helper; extern crate prefs_parser; +extern crate static_prefs; #[cfg(feature = "gecko_profiler")] extern crate profiler_helper; extern crate mozurl;