JavaScript:SpiderMonkey:C++ Coding Style: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
 
(10 intermediate revisions by 5 users not shown)
Line 1: Line 1:
== Basics ==
#REDIRECT [[JavaScript:SpiderMonkey:Coding_Style]]
 
The [[JavaScript:SpiderMonkey:C_Coding_Style|C Coding Style Guidelines]] apply unless overridden here. If this becomes awkward we should fork and diverge instead of referencing.
 
The rest of this page is a strawman proposal. Feel free to add, amend, or even correct if something is obviously wrong. Raise issues in the [http://groups.google.com/group/mozilla.dev.tech.js-engine/ newsgroup].
 
C++ is powerful, with many degrees of freedom and some attractive nuisances. We should aspire to use it in a lean, close-to-the-metal fashion, and avoid style-over-substance [http://www.bikeshed.com/ bikeshedding].
 
== Includes ==
 
The following order is used for includes:
 
#include "jsbar.h"
#include "jsfoo.h"
#include "ds/Baz.h"
#include "vm/Bat.h"
#include "jssquee-inl.h"
#include "jswoot-inl.h"
#include "frontend/Thingamabob-inl.h"
#include "vm/VirtualReality-inl.h"
 
Including X-inl.h implies that X.h is included as well. If possible, keep lexicographic order.
 
== Conditionals ==
 
The following style is used for bracing conditionals:
 
If the consequent (and, if present, the alternate) is a single statement, no braces are used.
if (today == "Tuesday")
    puts("I don't have my wallet on me.");
else
    puts("I would gladly pay you on Tuesday for a hamburger today.");
 
However, if ''either'' the consequent or alternate is a block of multiple statements, braces are used on both.
if (canSwingFromWeb) {
    p->swingFromWeb();
} else {
    JS_ASSERT(p->isSpiderPig());
    p->doWhateverSpiderPigDoes();
}
 
Conditions with multi-line tests should put the brace on the new line to provide a visual separation.
 
  types::TypeSet *types = frame.extra(lhs).types;
  if (JSOp(*PC) == JSOP_SETPROP && id == types::MakeTypeId(cx, id) &&
      types && !types->unknownObject() &&
      types->getObjectCount() == 1 &&
      types->getTypeObject(0) != NULL &&
      !types->getTypeObject(0)->unknownProperties())
  {
      JS_ASSERT(usePropCache);
      types::TypeObject *object = types->getTypeObject(0);
      types::TypeSet *propertyTypes = object->getProperty(cx, id, false);
      ...
  } else {
      ...
  }
 
However, if there is already a visual separation between the condition and the body, putting the { on a new line isn't necessary:
 
  if (forHead->pn_kid1 && NewSrcNote2(cx, cg, SRC_DECL,
                                      (forHead->pn_kid1->isOp(JSOP_DEFVAR))
                                      ? SRC_DECL_VAR
                                      : SRC_DECL_LET) < 0) {
      return false;
  }
 
== Namespaces ==
 
* Instead of JS/js and JS_ prefixing for public function and type names, respectively, use
namespace JS { ... }
* Instead of js_ prefixing for library-private and "friend" functions,
namespace js { ... }
* Compile-time-evaluated functions (i.e., template meta-functions) are kept in the "template library" namespace to avoid collision with their runtime counterparts.
namespace js { namespace tl { ... } }
* Avoid unnamed namespaces unless they are necessary to give a class's members internal linkage.  Although the C++ standard officially deprecates 'static' on  functions, 'static' has the following advantages:
** It is difficult to name functions in unnamed namespaces in gdb.
** Static says "this is a translation-unit-local helper function" on the function, without having to look around for an enclosing "namespace {".
* Other namespaces? Are these names too short and likely to collide?
 
== Macros ==
 
* Line continuation characters should all line up, in column 79 if that exceeds the width of all the macro text.
* Macro parameters should be of the form name_ (instead of something like __name).
 
== Enums ==
 
Older code uses SHOUT_REALLY_LOUD for enum values, newer code uses InterCaps. Enums should be preferred to boolean arguments for ease of understanding at the invocation site.
 
== Classical OOP ==
 
* Most structs can become classes, with associated functions becoming methods. This already started for the [https://bugzilla.mozilla.org/show_bug.cgi?id=upvar2 upvar2 bug].
** Style detailing: mrbkap suggests left brace before class body on its own line in column 1 to match function style and facilitate vim navigation. People do this already when inheriting, since the left brace can get lost in the superclass(es).
class JSObject
{
      ...
}
* Member variable names, private or public, are '''not''' decorated. Examples of improper decoration include:
class Fail
{
    size_t capacity_;  // unsightly
    T *mBegin;        // makes babies cry
    T *m_end;          // much typing
}
Sometimes a canonical argument name may conflict with a member name.  In this case, one can disambiguate with "this->", although such explicit qualification should only be added when necessary:
class C
{
    size_t count;
    bool fresh;
    void change(size_t count) {
        this->count = count;
        this->fresh = false;  // verbose and unnecessary
    }
};
* Most macros become inline helpers. With LiveConnect gone on mozilla-central, we can make the helpers methods of relevant structs or classes finally, instead of static inline functions.
* Based on 20+ years of bad experiences, we are going to go slow and resist virtual methods and bases, MI, and the like. Function pointer tables for now, as before -- see [https://bugzilla.mozilla.org/show_bug.cgi?id=408416 JSObjectOps needs a make-over].
* Templates are good, Mozilla has positive experience and the portability is there for the kind of [http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization RAII] helpers we need.
* No exceptions, so std is hard to use. There is initial work underway to make STL-like containers that mesh well with the rest of the JS engine (see js::Vector, js::HashMap, js::Pool).
** There are still improvements to be made to the new hash table: double-hash implementation; improve bit-mixing into multiplicative hash if the cycle costs can be supported (measurement is required and we should understand down to the bits what is going on); a js::HashSet or js::HashMap<T,void> specialization such that set-like use of hashtables do not waste any space on values.
 
=== Initializer lists ===
 
Initializer lists can break in one of two ways. The first may be preferable when constructors take few arguments:
class Ninja : public WeaponWeilder, public Camouflagible,
              public Assassinatable, public ShapeShiftable
{
    Ninja() : WeaponWeilder(Weapons::SHURIKEN),
              Camouflagible(Garments::SHINOBI_SHOZOKU),
              Assassinatable(AssassinationDifficulty::HIGHLY_DIFFICULT),
              ShapeShiftable(MPCost(512)) {}
}
The other permitted style mitigates longer-identifiers-squishing-text-against-the-right-side-of-the-screen-syndrome by using a half-indented colon:
class Ninja
  : public WeaponWeilder, public Camouflagible, public Assassinatable,
    public ShapeShiftable
{
    Ninja()
      : WeaponWeilder(Weapons::SHURIKEN),
        Camouflagible(Garments::SHINOBI_SHOZOKU),
        Assassinatable(AssassinationDifficulty::HIGHLY_DIFFICULT),
        ShapeShiftable(MPCost(512)) {}
}
 
=== Inline methods ===
 
If the method in question fits on one line and is branch free, do a one liner:
class Eater
{
    void eat(Eaten &other) { other.setConsumed(); }
};
If it's too long, put the type, declarator including formals (unless they overflow), and left brace all on the first line:
class Eater
{
    Food *obtainFoodFromEatery(Eatery &eatery) {
        if (!eatery.hasFood())
            return NULL;
        return eatery.purchaseFood();
    }
};
For out-of-line inlines (when the definitions are too unwieldy to place in the class definition) use the inline keyword as an indicator that there's an out-of-line inline definition:
class SpaceGoo
{
    inline BlobbyWrapper *enblob(Entity &other);
};
inline BlobbyWrapper *
SpaceGoo::enblob(Entity &other)
{
    /* ... */
}
 
== Boolean Types ==
 
* We would like to use the C++ 'bool' type wherever possible, but we must avoid breaking public and 'friend' APIs that use JSBool.  So public header files (and semi-public headers, like 'jsopcodes.h') should continue to use 'JSBool', 'JS_TRUE', and 'JS_FALSE'.  In all SpiderMonkey '.cpp' files, use 'true' and 'false', never 'JS_TRUE' or 'JS_FALSE'.  Use 'bool' whenever doing so would not conflict with a declaration in a public or semi-public header.
 
== References ==
 
* [https://developer.mozilla.org/en/Mozilla_Coding_Style_Guide Mozilla's coding style guide].
* [http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml Google's C++ coding style guide].

Latest revision as of 00:49, 26 April 2013