Page MenuHomePhabricator Dev
Differential D715

Bug 1387052 - Make Firefox code worse intentionally 2/2.
Needs ReviewPublic
Actions

Authored by janx on Oct 5 2018, 8:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 8, 4:26 PM
Unknown Object (File)
Aug 6 2026, 12:02 PM
Unknown Object (File)
Jul 20 2026, 2:12 PM
Unknown Object (File)
Jul 16 2026, 9:22 PM
Unknown Object (File)
Jun 15 2026, 4:01 PM
Unknown Object (File)
Jun 8 2026, 4:22 AM
Unknown Object (File)
Jun 8 2026, 4:21 AM
Unknown Object (File)
Jun 5 2026, 11:18 AM
View All 34 Files
Subscribers
None

Details

Reviewers
ReviewBot
Bugzilla Bug ID
1387052
Summary

Depends on D714

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ReviewBot added inline comments.Oct 10 2018, 12:51 PM
js/src/jsapi.cpp
237

Warning: Use '= default' to define a trivial default constructor [clang-tidy: modernize-use-equals-default]

B() {} // after: modernize-use-equals-default
^   ~~
    = default;
238

Warning: Use '= default' to define a trivial copy constructor [clang-tidy: modernize-use-equals-default]

B(const B&) {} // after: modernize-use-equals-default
^           ~~
            = default;
244

Warning: Use '= default' to define a trivial copy constructor [clang-tidy: modernize-use-equals-default]

D(const D &RHS) : B(RHS) {} // after: modernize-use-equals-default
^
245

Warning: Move constructor initializes base class by calling a copy constructor [clang-tidy: performance-move-constructor-init]

D(D &&RHS) : B(RHS) {} // after: performance-move-constructor-init; performance-noexcept-move-constructor
             ^
376

Warning: The type of the loop variable 'p' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const std::pair<const int, std::vector<std::cxx11::basic_string<char>, std::allocator<std::cxx11::basic_string<char> > > > &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [clang-tidy: performance-implicit-conversion-in-loop]

for (const std::pair<int, std::vector<std::string>>& p : my_map) {} // after: performance-implicit-conversion-in-loop
     ^
419

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!obj) {
    ~^~~~
     == nullptr
532

Warning: Implicit conversion 'JSRuntime *' -> bool [clang-tidy: readability-implicit-bool-conversion]

while (parentRuntime && parentRuntime->parentRuntime) {
       ^~~~~~~~~~~~~
       ( != nullptr)
532

Warning: Implicit conversion 'JSRuntime *' -> bool [clang-tidy: readability-implicit-bool-conversion]

while (parentRuntime && parentRuntime->parentRuntime) {
                        ^~~~~~~~~~~~~               ~
                        (                            != nullptr)
584

Warning: Implicit conversion 'JSRuntime *' -> bool [clang-tidy: readability-implicit-bool-conversion]

return cx->runtime()->parentRuntime ? cx->runtime()->parentRuntime : cx->runtime();
       ^
                                    != nullptr
614

Warning: Implicit conversion 'js::jit::JitRuntime *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!rt->getJitRuntime(cx)) {
    ~^~                   ~
                           == nullptr
623

Warning: Implicit conversion 'JSRuntime *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!rt->parentRuntime && !rt->initMainAtomsTables(cx)) {
    ~^~
    (                  == nullptr)
751

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (targetOrNull) {
    ^           ~
                 != nullptr
1082

Warning: Implicit conversion 'const JSStdName *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (stdnm && GlobalObject::skipDeselectedConstructor(cx, stdnm->key)) {
    ^~~~~
    ( != nullptr)
1088

Warning: Implicit conversion 'const JSStdName *' -> bool [clang-tidy: readability-implicit-bool-conversion]

JSProtoKey key = stdnm ? stdnm->key : JSProto_Null;
                 ^
                       != nullptr
1091

Warning: Implicit conversion 'unsigned int' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!clasp || !(clasp->flags & JSCLASS_IS_ANONYMOUS)) {
              ~^                                    ~
              (                                      == 0u)
1117

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!maybeObj || !maybeObj->staticPrototype()) {
                 ~^~~~~~~~                   ~
                 (                            == nullptr)
1131

Warning: Implicit conversion 'const JSStdName *' -> bool [clang-tidy: readability-implicit-bool-conversion]

LookupStdName(names, atom, standard_class_names) ||
^~~~~~~~~~~~~
(                                                != nullptr)
1132

Warning: Implicit conversion 'const JSStdName *' -> bool [clang-tidy: readability-implicit-bool-conversion]

LookupStdName(names, atom, builtin_property_names);
^~~~~~~~~~~~~                                     ~
(                                                  != nullptr)
1168

Warning: Implicit conversion 'unsigned int' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (clasp->flags & JSCLASS_IS_ANONYMOUS) {
    ^~~~~                              ~
    (                                  ) != 0u
1249

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!proto) {
    ~^~~~~~
     == nullptr
1306

Warning: Implicit conversion 'js::LexicalEnvironmentObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

return obj->is<GlobalObject>() || ObjectRealm::get(obj).getNonSyntacticLexicalEnvironment(obj);
                                  ^~~~~~~~~~~                                                 ~
                                  (                                                            != nullptr)
1327

Warning: Implicit conversion 'JS::Realm *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!cx->realm()) {
    ~^~         ~
                 == nullptr
1723

Warning: Escaped string literal can be written as a raw string literal [clang-tidy: modernize-raw-string-literal]

"\"string\", \"number\", or \"default\"",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R"("string", "number", or "default")"
1757

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!source) {
    ~^~~~~~~
     == nullptr
2005

Warning: Implicit conversion 'const js::Class *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!clasp) {
    ~^~~~~~
     == nullptr
2222

Warning: Implicit conversion 'unsigned int' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (attrs & (JSPROP_GETTER | JSPROP_SETTER)) {
    ^~~~~                                  ~
    (                                      ) != 0u
2243

Warning: Implicit conversion 'JSNative' (aka 'bool (*)(JSContext *, unsigned int, JS::Value *)') -> bool [clang-tidy: readability-implicit-bool-conversion]

if (get.op) {
    ^     ~
           != nullptr
2245

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!atom) {
    ~^~~~~
     == nullptr
2249

Warning: Implicit conversion 'JSFunction *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!getter) {
    ~^~~~~~~
     == nullptr
2253

Warning: Implicit conversion 'const JSJitInfo *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (get.info) {
    ^       ~
             != nullptr
2271

Warning: Implicit conversion 'const JSJitInfo *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (set.info) {
    ^       ~
             != nullptr
3201

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!getterNameAtom) {
    ~^~~~~~~~~~~~~~~
     == nullptr
3207

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!name) {
    ~^~~~~
     == nullptr
3221

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (setterName) {
    ^         ~
               != nullptr
3223

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!setterNameAtom) {
    ~^~~~~~~~~~~~~~~
     == nullptr
3255

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!nobj) {
    ~^~~~~
     == nullptr
3365

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

for (; ps->name; ps++) {
       ^       ~
                != nullptr
3554

Warning: Implicit conversion 'const JSSecurityCallbacks *' -> bool [clang-tidy: readability-implicit-bool-conversion]

cx->runtime()->securityCallbacks = scb ? scb : &NullSecurityCallbacks;
                                   ^
                                       != nullptr
3597

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (name) {
    ^   ~
         != nullptr
3643

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (fs->selfHostedName) {
    ^                 ~
                       != nullptr
3648

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!shAtom) {
    ~^~~~~~~
     == nullptr
3653

Warning: Implicit conversion 'JSAtom *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!name) {
    ~^~~~~
     == nullptr
3671

Warning: Implicit conversion 'JSNative' (aka 'bool (*)(JSContext *, unsigned int, JS::Value *)') -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!fs->call.op) {
    ~^~         ~
                 == nullptr
3673

Warning: Implicit conversion 'unsigned int' -> bool [clang-tidy: readability-implicit-bool-conversion]

} else if (fs->flags & JSFUN_CONSTRUCTOR) {
           ^~                           ~
           (                            ) != 0u
3678

Warning: Implicit conversion 'JSFunction *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!fun) {
    ~^~~~
     == nullptr
3682

Warning: Implicit conversion 'const JSJitInfo *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (fs->call.info) {
    ^            ~
                  != nullptr
3727

Warning: Implicit conversion 'JSScript *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!JSFunction::getOrCreateScript(cx, fun)) {
    ~^~~~~~~~~~                            ~
                                            == nullptr
3977

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (f) {
    ^~
      != nullptr
4006

Warning: Implicit conversion 'const char16_t *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (s) {
    ^~
      != nullptr
4145

Warning: Implicit conversion 'JSFunction *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (fun) {
    ^  ~
        != nullptr
4203

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

return !!module;
        ~^~~~~~~
        ( == nullptr)
4375

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!executor) {
    ~^~~~~~~~~
     == nullptr
4409

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!promiseObj || !promiseObj->is<PromiseObject>()) {
    ~^~~~~~~~~~
    ( == nullptr)
4503

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!unwrappedPromiseObj) {
    ~^~~~~~~~~~~~~~~~~~~~
     == nullptr
4553–4554

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (resultObj) {
    ^        ~
              != nullptr
4689

Warning: Implicit conversion 'void (*)(JSContext *, JS::Handle<JSObject *>, void *, unsigned char, unsigned long)' -> bool [clang-tidy: readability-implicit-bool-conversion]

return cx->runtime()->readableStreamDataRequestCallback;
       ^                                               ~
                                                        != nullptr
5097

Warning: Implicit conversion 'JSObject *' -> bool [clang-tidy: readability-implicit-bool-conversion]

oldAsyncStack ? &oldAsyncStack->as<SavedFrame>() : nullptr;
^
              != nullptr
5302

Warning: Implicit conversion 'JSLinearString *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!linear) {
    ~^~~~~~~
     == nullptr
5433

Warning: Implicit conversion 'JSLinearString *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!linearStr) {
    ~^~~~~~~~~~
     == nullptr
5485

Warning: Implicit conversion 'char16_t *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!dst) {
    ~^~~~
     == nullptr
5574

Warning: Implicit conversion 'JSString *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (description) {
    ^          ~
                != nullptr
5615

Warning: Implicit conversion 'char' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!*s) {
    ~^ ~
        == 0
5618

Warning: Implicit conversion 'char' -> bool [clang-tidy: readability-implicit-bool-conversion]

for (; *s; s++) {
       ^ ~
          != 0
5677

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

StringifyBehavior::RestrictedSafe))
                                   ^
                                    {
6047

Warning: Implicit conversion 'js::RegExpStatics *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!res) {
    ~^~~~
     == nullptr
6086

Warning: Implicit conversion 'JSLinearString *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!input) {
    ~^~~~~~
     == nullptr
6130

Warning: Implicit conversion 'js::RegExpShared *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!shared) {
    ~^~~~~~~
     == nullptr
6131

Warning: Implicit conversion bool -> 'unsigned int' [clang-tidy: readability-implicit-bool-conversion]

return false;
       ^~~~~~
       0u
6342

Warning: Implicit conversion 'const char16_t *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (ownsLinebuf_ && linebuf_) {
                    ^~~~~~~~~
                    ( != nullptr)
6373

Warning: Use '= default' to define a trivial destructor [clang-tidy: modernize-use-equals-default]

JSErrorNotes::~JSErrorNotes()
              ^
6652

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.disableOptimizationTracking = !value;
                                              ~^~~~~~
                                              ( == 0u)
6653–6654

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.enableTraceLogger = !!value;
                                     ~^~~~~~
                                     ( == 0u)
6655

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.spectreIndexMasking = !!value;
                                       ~^~~~~~
                                       ( == 0u)
6658

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.spectreObjectMitigationsBarriers = !!value;
                                                    ~^~~~~~
                                                    ( == 0u)
6661

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.spectreObjectMitigationsMisc = !!value;
                                                ~^~~~~~
                                                ( == 0u)
6667

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.spectreValueMasking = !!value;
                                       ~^~~~~~
                                       ( == 0u)
6670

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.spectreJitToCxxCalls = !!value;
                                        ~^~~~~~
                                        ( == 0u)
6673

Warning: Implicit conversion 'uint32_t' (aka 'unsigned int') -> bool [clang-tidy: readability-implicit-bool-conversion]

jit::JitOptions.wasmFoldOffsets = !!value;
                                   ~^~~~~~
                                   ( == 0u)
6703

Warning: Implicit conversion bool -> 'uint32_t' (aka 'unsigned int') [clang-tidy: readability-implicit-bool-conversion]

*valueOut = jit::JitOptions.forceInlineCaches;
            ^~~                              ~
            static_cast<uint32_t>(           )
6706

Warning: Implicit conversion bool -> 'uint32_t' (aka 'unsigned int') [clang-tidy: readability-implicit-bool-conversion]

*valueOut = jit::JitOptions.checkRangeAnalysis;
            ^~~                               ~
            static_cast<uint32_t>(            )
6709

Warning: Implicit conversion bool -> 'uint32_t' (aka 'unsigned int') [clang-tidy: readability-implicit-bool-conversion]

*valueOut = JS::ContextOptionsRef(cx).ion();
            ^~                             ~
            static_cast<uint32_t>(         )
6718

Warning: Implicit conversion bool -> 'uint32_t' (aka 'unsigned int') [clang-tidy: readability-implicit-bool-conversion]

*valueOut = rt->canUseOffthreadIonCompilation();
            ^~                                 ~
            static_cast<uint32_t>(             )
6798

Warning: Implicit conversion 'js::ScriptSource *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (ss_) {
    ^  ~
        != nullptr
6814

Warning: Implicit conversion 'js::ScriptSource *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (p) {
    ^~
      != nullptr
6823

Warning: Implicit conversion 'const char *' -> bool [clang-tidy: readability-implicit-bool-conversion]

filename_.as<const char*>() = filename ? filename : "";
                              ^
                                       != nullptr
6847

Warning: Implicit conversion 'unsigned int *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (lineno) {
    ^     ~
           != nullptr
6850

Warning: Implicit conversion 'unsigned int *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (column) {
    ^     ~
           != nullptr
6854

Warning: Implicit conversion 'JS::Compartment *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!cx->compartment()) {
    ~^~               ~
                       == nullptr
6886

Warning: Implicit conversion 'unsigned int *' -> bool [clang-tidy: readability-implicit-bool-conversion]

} else if (column) {
           ^     ~
                  != nullptr
6965

Warning: Implicit conversion 'js::Activation *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!activation) {
    ~^~~~~~~~~~~
     == nullptr
7100

Warning: Implicit conversion 'JSScript *' -> bool [clang-tidy: readability-implicit-bool-conversion]

if (!script) {
    ~^~~~~~~
     == nullptr
janx updated this revision to Diff 1205.Oct 12 2018, 1:29 PM
Harbormaster completed remote builds in B922: Diff 1205.Oct 12 2018, 1:29 PM
ReviewBot added a comment.Oct 12 2018, 1:44 PM

Code analysis found 6 defects in this patch:

  • 6 defects found by clang-tidy

You can run this analysis locally with:

  • ./mach static-analysis check path/to/file.cpp (C/C++)

If you see a problem in this automated review, please report it here: https://bit.ly/2IyNRy2

js/src/jsapi.cpp
237

Warning: Use '= default' to define a trivial default constructor [clang-tidy: modernize-use-equals-default]

B() {} // after: modernize-use-equals-default
^   ~~
    = default;
238

Warning: Use '= default' to define a trivial copy constructor [clang-tidy: modernize-use-equals-default]

B(const B&) {} // after: modernize-use-equals-default
^           ~~
            = default;
239

Warning: Move constructors should be marked noexcept [clang-tidy: performance-noexcept-move-constructor]

B(B &&) {} // after: performance-noexcept-move-constructor
^
244

Warning: Use '= default' to define a trivial copy constructor [clang-tidy: modernize-use-equals-default]

D(const D &RHS) : B(RHS) {} // after: modernize-use-equals-default
^
245

Warning: Move constructor initializes base class by calling a copy constructor [clang-tidy: performance-move-constructor-init]

D(D &&RHS) : B(RHS) {} // after: performance-move-constructor-init; performance-noexcept-move-constructor
             ^
245

Warning: Move constructors should be marked noexcept [clang-tidy: performance-noexcept-move-constructor]

D(D &&RHS) : B(RHS) {} // after: performance-move-constructor-init; performance-noexcept-move-constructor
^

Revision Contents
Changeset List

PathSize
js/
src/

Diff 1205

View Options

js/src/jsapi.cpp

Loading...
Privacy · Cookies · Legal