C++ Coding style

This document attempts to explain the basic styles and patterns used inthe Mozilla codebase. New code should try to conform to these standards,so it is as easy to maintain as existing code. There are exceptions, butit’s still important to know the rules!

This article is particularly for those new to the Mozilla codebase, andin the process of getting their code reviewed. Before requesting areview, please read over this document, making sure that your codeconforms to recommendations.

The Firefox code base adopts parts of theGoogle Coding style for C++code, but not all of its rules.A few rules are followed across the code base, others are intended to befollowed in new or significantly revised code. We may extend this list in thefuture, when we evaluate the Google Coding Style for C++ Code further and/or updateour coding practices. However, the plan is not to adopt all rules of the Google CodingStyle for C++ Code. Some rules are explicitly unlikely to be adopted at any time.

Followed across the code base:

  • Formatting,except for subsections noted here otherwise

  • Implicit Conversions,which is enforced by a custom clang-plugin check, unless explicitly overridden usingMOZ_IMPLICIT

Followed in new/significantly revised code:

Unlikely to be ever adopted:

This list reflects the state of the Google Google Coding Style for C++ Code as of2020-07-17. It may become invalid when the Google modifies its Coding Style.

Formatting code

Formatting is done automatically via clang-format, and controlled via in-treeconfiguration files. SeeFormatting C++ Code With clang-formatfor more information.

Unix-style linebreaks (\n), not Windows-style (\r\n). You canconvert patches, with DOS newlines to Unix via thedos2unix utility,or your favorite text editor.

Static analysis

Several of the rules in the Google C++ coding styles and the additions mentioned belowcan be checked via clang-tidy (some rules are from the upstream clang-tidy, some areprovided via a mozilla-specific plugin). Some of these checks also allow fixes tobe automatically applied.

machstatic-analysis provides a convenient way to run these checks. For example,for the check calledgoogle-readability-braces-around-statements, you can run:

./machstatic-analysischeck--checks="-*,google-readability-braces-around-statements"--fix<file>

It may be necessary to reformat the files after automatically applying fixes, seeFormatting C++ Code With clang-format.

Additional rules

The norms in this section should be followed for new code. For existing code,use the prevailing style in a file or module, ask the owner if you arein another team’s codebase or it’s not clear what style to use.

Control structures

Always brace controlled statements, even a single-line consequent ofifelseelse. This is redundant, typically, but it avoids danglingelse bugs, so it’s safer at scale than fine-tuning.

Examples:

if(...){}elseif(...){}else{}while(...){}do{}while(...);for(...;...;...){}switch(...){case1:{// When you need to declare a variable in a switch, put the block in braces.intvar;break;}case2:...break;default:break;}

else should only ever be followed by{ orif; i.e., othercontrol keywords are not allowed and should be placed inside braces.

Note

For this rule, clang-tidy provides thegoogle-readability-braces-around-statementscheck with autofixes.

C++ namespaces

Mozilla project C++ declarations should be in themozillanamespace. Modules should avoid adding nested namespaces undermozilla. A couple of exceptions to this rule are:

  • Names which have a high probability of colliding with other names in thecode base. For example,Point,Path, etc. Such symbols can be putunder module-specific namespaces, undermozilla, with shortall-lowercase names.

  • Classes that implement WebIDL bindings tend to live inmozilla::dom,though this is not strictly required and can be customized viaBindings.conf. SeeWeb IDL bindings for more information.

Other global namespaces besidesmozilla are not allowed.

Nousing directives are allowed in header files, except inside classdefinitions or functions. (We don’t want to pollute the global scope ofcompilation units that use the header file.)

Note

For parts of this rule, clang-tidy provides thegoogle-global-names-in-headerscheck. It only detectsusingnamespace directives in the global namespace.

usingnamespace...; is only allowed in.cpp files after all#includes. Prefer to wrap code innamespace...{...};instead, if possible.usingnamespace...;should always specifythe fully qualified namespace. That is, to useFoo::Bar do notwriteusingnamespaceFoo;usingnamespaceBar;, writeusingnamespaceFoo::Bar;

Use nested namespaces (ex:namespacemozilla::widget{

Note

clang-tidy provides themodernize-concat-nested-namespacescheck with autofixes.

Anonymous namespaces

We prefer usingstatic, instead of anonymous C++ namespaces. This maychange once there is better debugger support (especially on Windows) forplacing breakpoints, etc. on code in anonymous namespaces. You may stilluse anonymous namespaces for things that can’t be hidden withstatic,such as types, or certain objects which need to be passed to templatefunctions.

C++ classes

namespacemozilla{classMyClass:publicA{...};classMyClass:publicX,publicY{public:MyClass(intaVar,intaVar2):mVar(aVar),mVar2(aVar2){...}// Special member functions, like constructors, that have default bodies// should use '= default' annotation instead.MyClass()=default;// Unless it's a copy or move constructor or you have a specific reason to allow// implicit conversions, mark all single-argument constructors explicit.explicitMyClass(OtherClassaArg){...}// This constructor can also take a single argument, so it also needs to be marked// explicit.explicitMyClass(OtherClassaArg,AnotherClassaArg2=AnotherClass()){...}intLargerFunction(){......}private:intmVar;};}// namespace mozilla

Define classes using the style given above.

Note

For the rule on=default, clang-tidy provides themodernize-use-defaultcheck with autofixes.

For the rule on explicit constructors and conversion operators, clang-tidyprovides themozilla-implicit-constructor check.

Existing classes in the global namespace are named with a short prefix(For example,ns) as a pseudo-namespace.

Methods and functions

C/C++

In C/C++, method names should useUpperCamelCase.

Getters that never fail, and never return null, are namedFoo(),while all other getters useGetFoo(). Getters can return an objectvalue, via aFoo**aResult outparam (typical for an XPCOM getter),or as analready_AddRefed<Foo> (typical for a WebIDL getter,possibly with anErrorResult&rv parameter), or occasionally as aFoo* (typical for an internal getter for an object with a knownlifetime). Seethe bug 223255for more information.

XPCOM getters always return primitive values via an outparam, whileother getters normally use a return value.

Method declarations must use, at most, one of the following keywords:virtual,override, orfinal. Usevirtual to declarevirtual methods, which do not override a base class method with the samesignature. Useoverride to declare virtual methods which dooverride a base class method, with the same signature, but can befurther overridden in derived classes. Usefinal to declare virtualmethods which do override a base class method, with the same signature,but can NOT be further overridden in the derived classes. This shouldhelp the person reading the code fully understand what the declarationis doing, without needing to further examine base classes.

Note

For the rule onvirtual/override/final, clang-tidy provides themodernize-use-override check with autofixes.

Operators

The unary keyword operatorsizeof, should have its operand parenthesizedeven if it is an expression; e.g.int8_tarr[64];memset(arr,42,sizeof(arr));.

Literals

Use\uXXXX unicode escapes for non-ASCII characters. The characterset for XUL, script, and properties files is UTF-8, which is not easilyreadable.

Prefixes

Follow these naming prefix conventions:

Variable prefixes

  • k=constant (e.g.kNC_child). Not all code uses this style; someusesALL_CAPS for constants.

  • g=global (e.g.gPrefService)

  • a=argument (e.g.aCount)

  • C++ Specific Prefixes

    • s=static member (e.g.sPrefChecked)

    • m=member (e.g.mLength)

    • e=enum variants (e.g.enumFoo{eBar,eBaz}). Enum classesshould useCamelCase instead (e.g.enumclassFoo{Bar,Baz}).

Global functions/macros/etc

  • Macros begin withMOZ_, and are all caps (e.g.MOZ_WOW_GOODNESS). Note that older code uses theNS_ prefix;while these aren’t being changed, you should only useMOZ_ fornew macros. The only exception is if you’re creating a new macro,which is part of a set of related macros still using the oldNS_prefix. Then you should be consistent with the existing macros.

Error Variables

  • Local variables that are assignednsresult result codes should be namedrv(i.e., e.g., notres, notresult, notfoo).rv should not beused for bool or other result types.

  • Local variables that are assignedbool result codes should be namedok.

C/C++ practices

  • Have you checked for compiler warnings? Warnings often point toreal bugs.Many of themare enabled by default in the build system.

  • In C++ code, usenullptr for pointers. In C code, usingNULLor0 is allowed.

Note

For the C++ rule, clang-tidy provides themodernize-use-nullptr checkwith autofixes.

  • Don’t usePRBool andPRPackedBool in C++, useboolinstead.

  • For checking if astd container has no items, don’t usesize(), instead useempty().

  • When testing a pointer, use(!myPtr) or(myPtr);don’t usemyPtr!=nullptr ormyPtr==nullptr.

  • Do not comparex==true orx==false. Use(x) or(!x) instead.if(x==true) may have semantics different fromif(x)!

Note

clang-tidy provides thereadability-simplify-boolean-expr checkwith autofixes that checks for these and some other boolean expressionsthat can be simplified.

  • In general, initialize variables withnsFooaFoo=bFoo, and notnsFooaFoo(bFoo).

    • For constructors, initialize member variables with :nsFooaFoo(bFoo) syntax.

  • To avoid warnings created by variables used only in debug builds, usetheDebugOnly<T>helper when declaring them.

  • You shoulduse the static preferenceAPI forworking with preferences.

  • One-argument constructors, that are not copy or move constructors,should generally be marked explicit. Exceptions should be annotatedwithMOZ_IMPLICIT.

  • Global variables with runtime initialization should be avoided. Flaggingthem asconstexpr orMOZ_CONSTINIT is a good way to make sure theinitialization happens at compile-time. If runtime initialization cannot beavoided, use the attributeMOZ_RUNINIT to identify those and tell thelinter to ignore that variable. If a variable is flagged asMOZ_RUNINITwhile the linter detects it could beMOZ_CONSTINIT, you get an error. Incase where the status of the global variable varies (e.g. depending ontemplate parameter), just flag itMOZ_GLOBINIT.

  • Usechar32_t as the return type or argument type of a method thatreturns or takes as argument a single Unicode scalar value. (Don’tuse UTF-32 strings, though.)

  • Prefer unsigned types for semantically-non-negative integer values.

  • When operating on integers that could overflow, useCheckedInt.

  • Avoid the usage oftypedef, instead, please useusing instead.

Note

For parts of this rule, clang-tidy provides themodernize-use-usingcheck with autofixes.

Header files

Since the Firefox code base is huge and uses a monolithic build, it isof utmost importance for keeping build times reasonable to limit thenumber of included files in each translation unit to the required minimum.Exported header files need particular attention in this regard, since theirincluded files propagate, and many of them are directly or indirectlyincluded in a large number of translation units.

  • Include guards are named per the Google coding style (i.e. upper snakecase with a single trailing underscore). They should not include aleadingMOZ_ orMOZILLA_. For example,dom/media/foo.hwould use the guardDOM_MEDIA_FOO_H_.

  • Forward-declare classes in your header files, instead of includingthem, whenever possible. For example, if you have an interface with avoidDoSomething(nsIContent*aContent) function, forward-declarewithclassnsIContent; instead of#include"nsIContent.h".If a “forwarding header” is provided for a type, include that instead ofputting the literal forward declaration(s) in your header file. E.g. forsome JavaScript types, there isjs/TypeDecls.h, for the string typesthere isStringFwd.h. One reason for this is that this allowschanging a type to a type alias by only changing the forwarding header.The following uses of a type can be done with a forward declaration only:

    • Parameter or return type in a function declaration

    • Member/local variable pointer or reference type

    • Use as a template argument (not in all cases) in a member/local variable type

    • Defining a type alias

    The following uses of a type require a full definition:

    • Base class

    • Member/local variable type

    • Use with delete or new

    • Use as a template argument (not in all cases)

    • Any uses of non-scoped enum types

    • Enum values of a scoped enum type

    Use as a template argument is somewhat tricky. It depends on how thetemplate uses the type. E.g.mozilla::Maybe<T> andAutoTArray<T>always require a full definition ofT because the size of thetemplate instance depends on the size ofT.RefPtr<T> andUniquePtr<T> don’t require a full definition (because theirpointer member always has the same size), but their destructorrequires a full definition. If you encounter a template that cannotbe instantiated with a forward declaration only, but it seemsit should be possible, please file a bug (if it doesn’t exist yet).

    Therefore, also consider the following guidelines to allow using forwarddeclarations as widely as possible.

  • Inline function bodies in header files often pull in a lot of additionaldependencies. Be mindful when adding or extending inline function bodies,and consider moving the function body to the cpp file or to a separateheader file that is not included everywhere.Bug 1677553 intends to providea more specific guideline on this.

  • Consider the use of thePimpl idiom,i.e. hide the actual implementation in a separateImpl class that isdefined in the implementation file and only expose aclassImpl; forwarddeclaration andUniquePtr<Impl> member in the header file.

  • Do not use non-scoped enum types. These cannot be forward-declared. Usescoped enum types instead, and forward declare them when possible.

  • Avoid nested types that need to be referenced from outside the class.These cannot be forward declared. Place them in a namespace instead, maybein an extra inner namespace, and forward declare them where possible.

  • Avoid mixing declarations with different sets of dependencies in a singleheader file. This is generally advisable, but even more so when some of thesedeclarations are used by a subset of the translation units that include thecombined header file only. Consider such a badly mixed header file like:

    /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- *//* vim: set ts=8 sts=2 et sw=2 tw=80: *//* 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/. */#ifndef BAD_MIXED_FILE_H_#define BAD_MIXED_FILE_H_// Only this include is needed for the function declaration below.#include"nsCOMPtr.h"// These includes are only needed for the class definition.#include"nsIFile.h"#include"mozilla/ComplexBaseClass.h"namespacemozilla{classWrappedFile:publicnsIFile,ComplexBaseClass{// ... class definition left out for clarity};// Assuming that most translation units that include this file only call// the function, but don't need the class definition, this should be in a// header file on its own in order to avoid pulling in the other// dependencies everywhere.nsCOMPtr<nsIFile>CreateDefaultWrappedFile(nsCOMPtr<nsIFile>&&aFileToWrap);}// namespace mozilla#endif// BAD_MIXED_FILE_H_

An example header file based on these rules (with some extra comments):

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- *//* vim: set ts=8 sts=2 et sw=2 tw=80: *//* 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/. */#ifndef DOM_BASE_FOO_H_#define DOM_BASE_FOO_H_// Include guards should come at the very beginning and always use exactly// the style above. Otherwise, compiler optimizations that avoid rescanning// repeatedly included headers might not hit and cause excessive compile// times.#include<cstdint>#include"nsCOMPtr.h"  // This is needed because we have a nsCOMPtr<T> data member.classnsIFile;// Used as a template argument only.enumclassnsresult:uint32_t;// Used as a parameter type only.template<classT>classRefPtr;// Used as a return type only.namespacemozilla::dom{classDocument;// Used as a template argument only.// Scoped enum, not as a nested type, so it can be// forward-declared elsewhere.enumclassFooKind{Small,Big};classFoo{public:// Do not put the implementation in the header file, it would// require including nsIFile.hFoo(nsCOMPtr<nsIFile>aFile,FooKindaFooKind);RefPtr<Document>CreateDocument();voidSetResult(nsresultaResult);// Even though we will default this destructor, do this in the// implementation file since we would otherwise need to include// nsIFile.h in the header.~Foo();private:nsCOMPtr<nsIFile>mFile;};}// namespace mozilla::dom#endif// DOM_BASE_FOO_H_

Corresponding implementation file:

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- *//* vim: set ts=8 sts=2 et sw=2 tw=80: *//* 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/. */#include"mozilla/dom/Foo.h"  // corresponding header#include"mozilla/Assertions.h"  // Needed for MOZ_ASSERT.#include"mozilla/dom/Document.h" // Needed because we construct a Document.#include"nsError.h"  // Needed because we use NS_OK aka nsresult::NS_OK.#include"nsIFile.h"  // This is needed because our destructor indirectly calls delete nsIFile in a template instance.namespacemozilla::dom{// Do not put the implementation in the header file, it would// require including nsIFile.hFoo::Foo(nsCOMPtr<nsIFile>aFile,FooKindaFooKind):mFile{std::move(aFile)}{}RefPtr<Document>Foo::CreateDocument(){returnMakeRefPtr<Document>();}voidFoo::SetResult(nsresultaResult){MOZ_ASSERT(aResult!=NS_OK);// do something with aResult}// Even though we default this destructor, do this in the// implementation file since we would otherwise need to include// nsIFile.h in the header.Foo::~Foo()=default;}// namespace mozilla::dom

Include directives

  • Ordering:

    • In an implementation file (cpp file), the very first include directiveshould include the corresponding header file, followed by a blank line.

    • Any conditional includes (depending on some#ifdef or similar) followafter non-conditional includes. Don’t mix them in.

    • Don’t place comments between non-conditional includes.

    Bug 1679522 addresses automating the ordering via clang-format, whichis going to enforce some stricter rules. Expect the includes to be reordered.If you include third-party headers that are not self-contained, and thereforeneed to be included in a particular order, enclose those (and only those)between//clang-formatoff and//clang-formaton. This should not bedone for Mozilla headers, which should rather be made self-contained if theyare not.

  • Brackets vs. quotes: C/C++ standard library headers are included usingbrackets (e.g.#include<cstdint>), all other include directives use(double) quotes (e.g.#include"mozilla/dom/Document.h).

  • Exported headers should always be included from their exported path, notfrom their source path in the tree, even if available locally. E.g. alwaysdo#include"mozilla/Vector.h", not#include"Vector.h", evenfrom withinmfbt.

  • Generally, you should include exactly those headers that are needed, notmore and not less. Unfortunately this is not easy to see. Maybe C++20modules will bring improvements to this, but it will take a long timeto be adopted.

  • The basic rule is that if you literally use a symbol in your file thatis declared in a header A.h, include that header. In particular in headerfiles, check if a forward declaration or including a forwarding header issufficient, see sectionHeader files.

    There are cases where this basic rule is not sufficient. Some cases whereyou need to include additional headers are:

    • You reference a member of a type that is not literally mentioned in yourcode, but, e.g. is the return type of a function you are calling.

    There are also cases where the basic rule leads to redundant includes. Notethat “redundant” here does not refer to “accidentally redundant” headers,e.g. at the time of writingmozilla/dom/BodyUtil.h includesmozilla/dom/FormData.h, but it doesn’t need to (it only needs a forwarddeclaration), so includingmozilla/dom/FormData.h is “accidentallyredundant” when includingmozilla/dom/BodyUtil.h. The includes ofmozilla/dom/BodyUtil.h might change at any time, so if a file thatincludesmozilla/dom/BodyUtil.h needs a full definition ofmozilla::dom::FormData, it should includesmozilla/dom/FormData.hitself. In fact, these “accidentally redundant” headers MUST be included.Relying on accidentally redundant includes makes any change to a headerfile extremely hard, in particular when considering that the set ofaccidentally redundant includes differs between platforms.But some cases in fact are non-accidentally redundant, and these can andtypically should not be repeated:

    • The includes of the header file do not need to be repeated in itscorresponding implementation file. Rationale: the implementation file andits corresponding header file are tightly coupled per se.

    Macros are a special case. Generally, the literal rule also applies here,i.e. if the macro definition references a symbol, the file containing themacro definition should include the header defining the symbol. E.g.NS_IMPL_CYCLE_COLLECTING_NATIVE_RELEASE defined innsISupportsImpl.hmakes use ofMOZ_ASSERT defined inmozilla/Assertions.h, sonsISupportsImpl.h includesmozilla/Assertions.h. However, thisrequires human judgment of what is intended, since technically only theinvocations of the macro reference a symbol (and that’s howinclude-what-you-use handles this). It might depend on thecontext or parameters which symbol is actually referenced, and sometimesthis is on purpose. In these cases, the user of the macro needs to includethe required header(s).

COM and pointers

  • UsensCOMPtr<>If you don’t know how to use it, start looking in the code forexamples. The general rule, is that the very act of typingNS_RELEASE should be a signal to you to question your code:“Should I be usingnsCOMPtr here?”. Generally the only valid useofNS_RELEASE is when you are storing refcounted pointers in along-lived datastructure.

  • Declare new XPCOM interfaces usingXPIDL, so theywill be scriptable.

  • UsensCOMPtr for strong references, andnsWeakPtr for weak references.

  • Don’t useQueryInterface directly. UseCallQueryInterface ordo_QueryInterface instead.

  • UseContract IDs,instead of CIDs withdo_CreateInstance/do_GetService.

  • Use pointers, instead of references for function out parameters, evenfor primitive types.

IDL

Use leading-lowercase, or “interCaps”

When defining a method or attribute in IDL, the first letter should belowercase, and each following word should be capitalized. For example:

longupdateStatusBar();

Use attributes wherever possible

Whenever you are retrieving or setting a single value, without anycontext, you should use attributes. Don’t use two methods when you coulduse an attribute. Using attributes logically connects the getting andsetting of a value, and makes scripted code look cleaner.

This example has too many methods:

interfacensIFoo:nsISupports{longgetLength();voidsetLength(inlonglength);longgetColor();};

The code below will generate the exact same C++ signature, but is morescript-friendly.

interfacensIFoo:nsISupports{attributelonglength;readonlyattributelongcolor;};

Use Java-style constants

When defining scriptable constants in IDL, the name should be alluppercase, with underscores between words:

constlongERROR_UNDEFINED_VARIABLE=1;

See also

For details on interface development, as well as more detailed styleguides, see theInterface developmentguide.

Error handling

Check for errors early and often

Every time you make a call into an XPCOM function, you should check foran error condition. You need to do this even if you know that call willnever fail. Why?

  • Someone may change the callee in the future to return a failurecondition.

  • The object in question may live on another thread, another process,or possibly even another machine. The proxy could have failed to makeyour call in the first place.

Also, when you make a new function which is failable (i.e. it willreturn ansresult or abool that may indicate an error), you shouldexplicitly mark the return value should always be checked. For example:

//forIDL.[must_use]nsISupportscreate();//forC++,addthisin*declaration*,donotadditagaininimplementation.[[nodiscard]]nsresultDoSomething();

There are some exceptions:

  • Predicates or getters, which returnbool ornsresult.

  • IPC method implementation (For example,boolRecvSomeMessage()).

  • Most callers will check the output parameter, see below.

nsresultSomeMap::GetValue(constnsString&key,nsString&value);

If most callers need to check the output value first, then adding[[nodiscard]] might be too verbose. In this case, change the return valueto void might be a reasonable choice.

There is also a static analysis attribute[[nodiscard]], which canbe added to class declarations, to ensure that those declarations arealways used when they are returned.

Use the NS_WARN_IF macro when errors are unexpected.

TheNS_WARN_IF macro can be used to issue a console warning, in debugbuilds if the condition fails. This should only be used when the failureis unexpected and cannot be caused by normal web content.

If you are writing code which wants to issue warnings when methods fail,please either useNS_WARNING directly, or use the newNS_WARN_IF macro.

if(NS_WARN_IF(somethingthatshouldbefalse)){returnNS_ERROR_INVALID_ARG;}if(NS_WARN_IF(NS_FAILED(rv))){returnrv;}

Previously, theNS_ENSURE_* macros were used for this purpose, butthose macros hide return statements, and should not be used in new code.(This coding style rule isn’t generally agreed, so use ofNS_ENSURE_*can be valid.)

Return from errors immediately

In most cases, your knee-jerk reaction should be to return from thecurrent function, when an error condition occurs. Don’t do this:

rv=foo->Call1();if(NS_SUCCEEDED(rv)){rv=foo->Call2();if(NS_SUCCEEDED(rv)){rv=foo->Call3();}}returnrv;

Instead, do this:

rv=foo->Call1();if(NS_FAILED(rv)){returnrv;}rv=foo->Call2();if(NS_FAILED(rv)){returnrv;}rv=foo->Call3();if(NS_FAILED(rv)){returnrv;}

Why? Error handling should not obfuscate the logic of the code. Theauthor’s intent, in the first example, was to make 3 calls insuccession. Wrapping the calls in nested if() statements, insteadobscured the most likely behavior of the code.

Consider a more complicated example to hide a bug:

boolval;rv=foo->GetBooleanValue(&val);if(NS_SUCCEEDED(rv)&&val){foo->Call1();}else{foo->Call2();}

The intent of the author, may have been, thatfoo->Call2() would onlyhappen when val had a false value. In fact,foo->Call2() will also becalled, whenfoo->GetBooleanValue(&val) fails. This may, or may not,have been the author’s intent. It is not clear from this code. Here isan updated version:

boolval;rv=foo->GetBooleanValue(&val);if(NS_FAILED(rv)){returnrv;}if(val){foo->Call1();}else{foo->Call2();}

In this example, the author’s intent is clear, and an error conditionavoids both calls tofoo->Call1() andfoo->Call2();

Possible exceptions: Sometimes it is not fatal if a call fails. Forinstance, if you are notifying a series of observers that an event hasfired, it might be trivial that one of these notifications failed:

for(size_ti=0;i<length;++i){// we don't care if any individual observer failsobservers[i]->Observe(foo,bar,baz);}

Another possibility, is you are not sure if a component exists or isinstalled, and you wish to continue normally, if the component is notfound.

nsCOMPtr<nsIMyService>service=do_CreateInstance(NS_MYSERVICE_CID,&rv);// if the service is installed, then we'll use it.if(NS_SUCCEEDED(rv)){// non-fatal if this fails too, ignore this error.service->DoSomething();// this is important, handle this error!rv=service->DoSomethingImportant();if(NS_FAILED(rv)){returnrv;}}// continue normally whether or not the service exists.

Strings

Note

This section overlaps with the more verbose advice given inString guide.These should eventually be merged. For now, please refer to that guide formore advice.

  • String arguments to functions should be declared as[const]nsA[C]String&.

  • Prefer using string literals. In particular, use empty string literals,i.e.u""_ns or""_ns, instead ofEmpty[C]String() orconstnsAuto[C]Stringempty;. UseEmpty[C]String() only if youspecifically need aconstns[C]String&, e.g. with the ternary operatoror when you need to return/bind to a reference or take the address of theempty string.

  • For 16-bit literal strings, useu"..."_ns or, if necessaryNS_LITERAL_STRING_FROM_CSTRING(...) instead ofnsAutoString()or other ways that would do a run-time conversion.SeeAvoid runtime conversion of string literals below.

  • To compare a string with a literal, use.EqualsLiteral("...").

  • Usestr.IsEmpty() instead ofstr.Length()==0.

  • Usestr.Truncate() instead ofstr.SetLength(0),str.Assign(""_ns) orstr.AssignLiteral("").

  • Don’t use functions fromctype.h (isdigit(),isalpha(),etc.) or fromstrings.h (strcasecmp(),strncasecmp()).These are locale-sensitive, which makes them inappropriate forprocessing protocol text. At the same time, they are too limited towork properly for processing natural-language text. Use thealternatives inmozilla/TextUtils.h and innsUnicharUtils.hin place ofctype.h. In place ofstrings.h, prefer thensStringComparator facilities for comparing strings or if youhave to work with zero-terminated strings, usensCRT.h forASCII-case-insensitive comparison.

Use theAuto form of strings for local values

When declaring a local, short-livednsString class, always usensAutoString ornsAutoCString. These pre-allocate a 64-bytebuffer on the stack, and avoid fragmenting the heap. Don’t do this:

nsresultfoo(){nsCStringbar;..}

instead:

nsresultfoo(){nsAutoCStringbar;..}

Be wary of leaking values from non-XPCOM functions that return char* or PRUnichar*

It is an easy trap to return an allocated string, from an internalhelper function, and then using that function inline in your code,without freeing the value. Consider this code:

staticchar*GetStringValue(){..returnresultString.ToNewCString();}..WarnUser(GetStringValue());

In the above example,WarnUser will get the string allocated fromresultString.ToNewCString() and throw away the pointer. Theresulting value is never freed. Instead, either use the string classes,to make sure your string is automatically freed when it goes out ofscope, or make sure that your string is freed.

Automatic cleanup:

staticvoidGetStringValue(nsAWritableCString&aResult){..aResult.Assign("resulting string");}..nsAutoCStringwarning;GetStringValue(warning);WarnUser(warning.get());

Free the string manually:

staticchar*GetStringValue(){..returnresultString.ToNewCString();}..char*warning=GetStringValue();WarnUser(warning);nsMemory::Free(warning);

Avoid runtime conversion of string literals

It is very common to need to assign the value of a literal string, suchas"SomeString", into a unicode buffer. Instead of usingnsString’sAssignLiteral andAppendLiteral, use a user-defined literal likeu”foo”_nsinstead. On most platforms, this will force the compiler to compile in araw unicode string, and assign it directly. In cases where the literal is definedvia a macro that is used in both 8-bit and 16-bit ways, you can useNS_LITERAL_STRING_FROM_CSTRING to do the conversion at compile time.

Incorrect:

nsAutoStringwarning;warning.AssignLiteral("danger will robinson!");...foo->SetStringValue(warning);...bar->SetUnicodeValue(warning.get());

Correct:

constexprautowarning=u"danger will robinson!"_ns;...// if you'll be using the 'warning' string, you can still use it as before:foo->SetStringValue(warning);...bar->SetUnicodeValue(warning.get());// alternatively, use the wide string directly:foo->SetStringValue(u"danger will robinson!"_ns);...// if a macro is the source of a 8-bit literal and you cannot change it, use// NS_LITERAL_STRING_FROM_CSTRING, but only if necessary.#define MY_MACRO_LITERAL "danger will robinson!"foo->SetStringValue(NS_LITERAL_STRING_FROM_CSTRING(MY_MACRO_LITERAL));// If you need to pass to a raw const char16_t *, there's no benefit to// go through our string classes at all, just do...bar->SetUnicodeValue(u"danger will robinson!");// .. or, again, if a macro is the source of a 8-bit literalbar->SetUnicodeValue(u""MY_MACRO_LITERAL);

Usage of PR_(MAX|MIN|ABS|ROUNDUP) macro calls

Use the standard-library functions (std::max), instead ofPR_(MAX|MIN|ABS|ROUNDUP).

Usemozilla::Abs instead ofPR_ABS. AllPR_ABS calls in C++ code havebeen replaced withmozilla::Abs calls, inbug847480. All newcode inFirefox/core/toolkit needs to use theNS_foo variantsinstead ofPR_foo, or#include"mozilla/MathAlgorithms.h" formozilla::Abs.

Use of SpiderMonkey rooting typedefs

The rooting typedefs injs/public/TypeDecls.h, such asHandleObject andRootedObject, are deprecated both in and outside of SpiderMonkey. They willeventually be removed and should not be used in new code.