Open
Bug 1465872
Opened 6 years ago
Updated 2 years ago
Remove (replace?) MOZ_DEV_EDITION check, in the MOZ_DIAGNOSTIC_ASSERT definition
Categories
(Core :: General, enhancement, P3)
Core
General
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox62 | --- | affected |
People
(Reporter: dholbert, Unassigned)
References
(Blocks 1 open bug)
Details
We'd like to make Firefox DevEdition into a repack of beta (bug
1459240). For that to succeed, we can't have any #if/#ifdef MOZ_DEV_EDITION checks in our C++ code.
One such check is for MOZ_DIAGNOSTIC_ASSERT:
========
#if defined(NIGHTLY_BUILD) || defined(MOZ_DEV_EDITION)
# define MOZ_DIAGNOSTIC_ASSERT MOZ_RELEASE_ASSERT
# define MOZ_DIAGNOSTIC_ASSERT_ENABLED 1
#else
# define MOZ_DIAGNOSTIC_ASSERT MOZ_ASSERT
# ifdef DEBUG
# define MOZ_DIAGNOSTIC_ASSERT_ENABLED 1
# endif
#endif
========
https://dxr.mozilla.org/mozilla-central/rev/5866d6685849311f057e7e229b9ace63a2641c29/mfbt/Assertions.h#462-470
In order for MOZ_DIAGNOSTIC_ASSERT to continue working in DevEdition builds, we would need to make its behavior depend on a pref-check (at least in beta builds), I think.
For non-DEBUG non-Nightly case, we'd want...
MOZ_DIAGNOSTIC_ASSERT(condition, "string");
...to evaluate to:
if (IsDevEdition()) {
MOZ_RELEASE_ASSERT(condition, "string");
}
Or something like that. That means these asserts wouldn't be entirely free in release builds, but they'd still be pretty cheap if they use AddBoolVarCache for the pref-lookup. Importantly: if the pref is disabled (in "real" beta biulds), we'd never have to evaluate the possibly-expensive assertion condition, which is one of the main goal of using these macros.
Reporter | ||
Updated•6 years ago
|
Summary: Remove (replace?) MOZ_DEV_EDITION check for MOZ_DIAGNOSTIC_ASSERT → Remove (replace?) MOZ_DEV_EDITION check, in the MOZ_DIAGNOSTIC_ASSERT definition
Comment 1•6 years ago
|
||
Marking as an enhancement ; please change if relevant! Thanks.
Severity: normal → enhancement
Priority: -- → P3
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•