1================
2C++ Coding style
3================
4
5
6This document attempts to explain the basic styles and patterns used in
7the Mozilla codebase. New code should try to conform to these standards,
8so it is as easy to maintain as existing code. There are exceptions, but
9it's still important to know the rules!
10
11This article is particularly for those new to the Mozilla codebase, and
12in the process of getting their code reviewed. Before requesting a
13review, please read over this document, making sure that your code
14conforms to recommendations.
15
16.. container:: blockIndicator warning
17
18   The Firefox code base adopts parts of the `Google Coding style for C++
19   code <https://google.github.io/styleguide/cppguide.html>`__, but not all of its rules.
20   A few rules are followed across the code base, others are intended to be
21   followed in new or significantly revised code. We may extend this list in the
22   future, when we evaluate the Google Coding Style for C++ Code further and/or update
23   our coding practices. However, the plan is not to adopt all rules of the Google Coding
24   Style for C++ Code. Some rules are explicitly unlikely to be adopted at any time.
25
26   Followed across the code base:
27
28   - `Formatting <https://google.github.io/styleguide/cppguide.html#Formatting>`__,
29     except for subsections noted here otherwise
30   - `Implicit Conversions <https://google.github.io/styleguide/cppguide.html#Implicit_Conversions>`__,
31     which is enforced by a custom clang-plugin check, unless explicitly overridden using
32     ``MOZ_IMPLICIT``
33
34   Followed in new/significantly revised code:
35
36   - `Include guards <https://google.github.io/styleguide/cppguide.html#The__define_Guard>`__
37
38   Unlikely to be ever adopted:
39
40   - `Forward declarations <https://google.github.io/styleguide/cppguide.html#Forward_Declarations>`__
41   - `Formatting/Conditionals <https://google.github.io/styleguide/cppguide.html#Conditionals>`__
42     w.r.t. curly braces around inner statements, we require them in all cases where the
43     Google style allows to leave them out for single-line conditional statements
44
45   This list reflects the state of the Google Google Coding Style for C++ Code as of
46   2020-07-17. It may become invalid when the Google modifies its Coding Style.
47
48
49Formatting code
50---------------
51
52Formatting is done automatically via clang-format, and controlled via in-tree
53configuration files. See :ref:`Formatting C++ Code With clang-format`
54for more information.
55
56Unix-style linebreaks (``\n``), not Windows-style (``\r\n``). You can
57convert patches, with DOS newlines to Unix via the ``dos2unix`` utility,
58or your favorite text editor.
59
60Static analysis
61---------------
62
63Several of the rules in the Google C++ coding styles and the additions mentioned below
64can be checked via clang-tidy (some rules are from the upstream clang-tidy, some are
65provided via a mozilla-specific plugin). Some of these checks also allow fixes to
66be automatically applied.
67
68``mach static-analysis`` provides a convenient way to run these checks. For example,
69for the check called ``google-readability-braces-around-statements``, you can run:
70
71.. code-block:: shell
72
73   ./mach static-analysis check --checks="-*,google-readability-braces-around-statements" --fix <file>
74
75It may be necessary to reformat the files after automatically applying fixes, see
76:ref:`Formatting C++ Code With clang-format`.
77
78Additional rules
79----------------
80
81*The norms in this section should be followed for new code. For existing code,
82use the prevailing style in a file or module, ask the owner if you are
83in another team's codebase or it's not clear what style to use.*
84
85
86
87
88Control structures
89~~~~~~~~~~~~~~~~~~
90
91Always brace controlled statements, even a single-line consequent of
92``if else else``. This is redundant, typically, but it avoids dangling
93else bugs, so it's safer at scale than fine-tuning.
94
95Examples:
96
97.. code-block:: cpp
98
99   if (...) {
100   } else if (...) {
101   } else {
102   }
103
104   while (...) {
105   }
106
107   do {
108   } while (...);
109
110   for (...; ...; ...) {
111   }
112
113   switch (...) {
114     case 1: {
115       // When you need to declare a variable in a switch, put the block in braces.
116       int var;
117       break;
118     }
119     case 2:
120       ...
121       break;
122     default:
123       break;
124   }
125
126``else`` should only ever be followed by ``{`` or ``if``; i.e., other
127control keywords are not allowed and should be placed inside braces.
128
129.. note::
130
131   For this rule, clang-tidy provides the ``google-readability-braces-around-statements``
132   check with autofixes.
133
134
135C++ namespaces
136~~~~~~~~~~~~~~
137
138Mozilla project C++ declarations should be in the ``mozilla``
139namespace. Modules should avoid adding nested namespaces under
140``mozilla``, unless they are meant to contain names which have a high
141probability of colliding with other names in the code base. For example,
142``Point``, ``Path``, etc. Such symbols can be put under
143module-specific namespaces, under ``mozilla``, with short
144all-lowercase names. Other global namespaces besides ``mozilla`` are
145not allowed.
146
147No ``using`` directives are allowed in header files, except inside class
148definitions or functions. (We don't want to pollute the global scope of
149compilation units that use the header file.)
150
151.. note::
152
153   For parts of this rule, clang-tidy provides the ``google-global-names-in-headers``
154   check. It only detects ``using namespace`` directives in the global namespace.
155
156
157``using namespace ...;`` is only allowed in ``.cpp`` files after all
158``#include``\ s. Prefer to wrap code in ``namespace ... { ... };``
159instead, if possible. ``using namespace ...;``\ should always specify
160the fully qualified namespace. That is, to use ``Foo::Bar`` do not
161write ``using namespace Foo; using namespace Bar;``, write
162``using namespace Foo::Bar;``
163
164Use nested namespaces (ex: ``namespace mozilla::widget {``
165
166.. note::
167
168   clang-tidy provides the ``modernize-concat-nested-namespaces``
169   check with autofixes.
170
171
172Anonymous namespaces
173~~~~~~~~~~~~~~~~~~~~
174
175We prefer using ``static``, instead of anonymous C++ namespaces. This may
176change once there is better debugger support (especially on Windows) for
177placing breakpoints, etc. on code in anonymous namespaces. You may still
178use anonymous namespaces for things that can't be hidden with ``static``,
179such as types, or certain objects which need to be passed to template
180functions.
181
182
183C++ classes
184~~~~~~~~~~~~
185
186.. code-block:: cpp
187
188   namespace mozilla {
189
190   class MyClass : public A
191   {
192     ...
193   };
194
195   class MyClass
196     : public X
197     , public Y
198   {
199   public:
200     MyClass(int aVar, int aVar2)
201       : mVar(aVar)
202       , mVar2(aVar2)
203     {
204        ...
205     }
206
207     // Special member functions, like constructors, that have default bodies
208     // should use '= default' annotation instead.
209     MyClass() = default;
210
211     // Unless it's a copy or move constructor or you have a specific reason to allow
212     // implicit conversions, mark all single-argument constructors explicit.
213     explicit MyClass(OtherClass aArg)
214     {
215       ...
216     }
217
218     // This constructor can also take a single argument, so it also needs to be marked
219     // explicit.
220     explicit MyClass(OtherClass aArg, AnotherClass aArg2 = AnotherClass())
221     {
222       ...
223     }
224
225     int LargerFunction()
226     {
227       ...
228       ...
229     }
230
231   private:
232     int mVar;
233   };
234
235   } // namespace mozilla
236
237Define classes using the style given above.
238
239.. note::
240
241   For the rule on ``= default``, clang-tidy provides the ``modernize-use-default``
242   check with autofixes.
243
244   For the rule on explicit constructors and conversion operators, clang-tidy
245   provides the ``mozilla-implicit-constructor`` check.
246
247Existing classes in the global namespace are named with a short prefix
248(For example, ``ns``) as a pseudo-namespace.
249
250
251Methods and functions
252~~~~~~~~~~~~~~~~~~~~~
253
254
255C/C++
256^^^^^
257
258In C/C++, method names should use ``UpperCamelCase``.
259
260Getters that never fail, and never return null, are named ``Foo()``,
261while all other getters use ``GetFoo()``. Getters can return an object
262value, via a ``Foo** aResult`` outparam (typical for an XPCOM getter),
263or as an ``already_AddRefed<Foo>`` (typical for a WebIDL getter,
264possibly with an ``ErrorResult& rv`` parameter), or occasionally as a
265``Foo*`` (typical for an internal getter for an object with a known
266lifetime). See `the bug 223255 <https://bugzilla.mozilla.org/show_bug.cgi?id=223255>`_
267for more information.
268
269XPCOM getters always return primitive values via an outparam, while
270other getters normally use a return value.
271
272Method declarations must use, at most, one of the following keywords:
273``virtual``, ``override``, or ``final``. Use ``virtual`` to declare
274virtual methods, which do not override a base class method with the same
275signature. Use ``override`` to declare virtual methods which do
276override a base class method, with the same signature, but can be
277further overridden in derived classes. Use ``final`` to declare virtual
278methods which do override a base class method, with the same signature,
279but can NOT be further overridden in the derived classes. This should
280help the person reading the code fully understand what the declaration
281is doing, without needing to further examine base classes.
282
283.. note::
284
285   For the rule on ``virtual/override/final``, clang-tidy provides the
286   ``modernize-use-override`` check with autofixes.
287
288
289Operators
290~~~~~~~~~
291
292The unary keyword operator ``sizeof``, should have its operand parenthesized
293even if it is an expression; e.g. ``int8_t arr[64]; memset(arr, 42, sizeof(arr));``.
294
295
296Literals
297~~~~~~~~
298
299Use ``\uXXXX`` unicode escapes for non-ASCII characters. The character
300set for XUL, DTD, script, and properties files is UTF-8, which is not easily
301readable.
302
303
304Prefixes
305~~~~~~~~
306
307Follow these naming prefix conventions:
308
309
310Variable prefixes
311^^^^^^^^^^^^^^^^^
312
313-  k=constant (e.g. ``kNC_child``). Not all code uses this style; some
314   uses ``ALL_CAPS`` for constants.
315-  g=global (e.g. ``gPrefService``)
316-  a=argument (e.g. ``aCount``)
317-  C++ Specific Prefixes
318
319   -  s=static member (e.g. ``sPrefChecked``)
320   -  m=member (e.g. ``mLength``)
321   -  e=enum variants (e.g. ``enum Foo { eBar, eBaz }``). Enum classes
322      should use ``CamelCase`` instead (e.g.
323      ``enum class Foo { Bar, Baz }``).
324
325
326Global functions/macros/etc
327^^^^^^^^^^^^^^^^^^^^^^^^^^^
328
329-  Macros begin with ``MOZ_``, and are all caps (e.g.
330   ``MOZ_WOW_GOODNESS``). Note that older code uses the ``NS_`` prefix;
331   while these aren't being changed, you should only use ``MOZ_`` for
332   new macros. The only exception is if you're creating a new macro,
333   which is part of a set of related macros still using the old ``NS_``
334   prefix. Then you should be consistent with the existing macros.
335
336
337Error Variables
338^^^^^^^^^^^^^^^
339
340-  Local variables that are assigned ``nsresult`` result codes should be named ``rv``
341   (i.e., e.g., not ``res``, not ``result``, not ``foo``). `rv` should not be
342   used for bool or other result types.
343-  Local variables that are assigned ``bool`` result codes should be named `ok`.
344
345
346C/C++ practices
347---------------
348
349-  **Have you checked for compiler warnings?** Warnings often point to
350   real bugs. `Many of them <https://searchfox.org/mozilla-central/source/build/moz.configure/warnings.configure>`__
351   are enabled by default in the build system.
352-  In C++ code, use ``nullptr`` for pointers. In C code, using ``NULL``
353   or ``0`` is allowed.
354
355.. note::
356
357   For the C++ rule, clang-tidy provides the ``modernize-use-nullptr`` check
358   with autofixes.
359
360-  Don't use ``PRBool`` and ``PRPackedBool`` in C++, use ``bool``
361   instead.
362-  For checking if a ``std`` container has no items, don't use
363   ``size()``, instead use ``empty()``.
364-  When testing a pointer, use ``(!myPtr)`` or ``(myPtr)``;
365   don't use ``myPtr != nullptr`` or ``myPtr == nullptr``.
366-  Do not compare ``x == true`` or ``x == false``. Use ``(x)`` or
367   ``(!x)`` instead. ``if (x == true)`` may have semantics different from
368   ``if (x)``!
369
370.. note::
371
372   clang-tidy provides the ``readability-simplify-boolean-expr`` check
373   with autofixes that checks for these and some other boolean expressions
374   that can be simplified.
375
376-  In general, initialize variables with ``nsFoo aFoo = bFoo,`` and not
377   ``nsFoo aFoo(bFoo)``.
378
379   -  For constructors, initialize member variables with : ``nsFoo
380      aFoo(bFoo)`` syntax.
381
382-  To avoid warnings created by variables used only in debug builds, use
383   the
384   `DebugOnly<T> <https://developer.mozilla.org/docs/Mozilla/Debugging/DebugOnly%3CT%3E>`__
385   helper when declaring them.
386-  You should `use the static preference
387   API <https://developer.mozilla.org/docs/Mozilla/Preferences/Using_preferences_from_application_code>`__ for
388   working with preferences.
389-  One-argument constructors, that are not copy or move constructors,
390   should generally be marked explicit. Exceptions should be annotated
391   with ``MOZ_IMPLICIT``.
392-  Use ``char32_t`` as the return type or argument type of a method that
393   returns or takes as argument a single Unicode scalar value. (Don't
394   use UTF-32 strings, though.)
395-  Prefer unsigned types for semantically-non-negative integer values.
396-  When operating on integers that could overflow, use ``CheckedInt``.
397-  Avoid the usage of ``typedef``, instead, please use ``using`` instead.
398
399.. note::
400
401   For parts of this rule, clang-tidy provides the ``modernize-use-using``
402   check with autofixes.
403
404
405Header files
406------------
407
408Since the Firefox code base is huge and uses a monolithic build, it is
409of utmost importance for keeping build times reasonable to limit the
410number of included files in each translation unit to the required minimum.
411Exported header files need particular attention in this regard, since their
412included files propagate, and many of them are directly or indirectly
413included in a large number of translation units.
414
415-  Include guards are named per the Google coding style (i.e. upper snake
416   case with a single trailing underscore). They should not include a
417   leading ``MOZ_`` or ``MOZILLA_``. For example, ``dom/media/foo.h``
418   would use the guard ``DOM_MEDIA_FOO_H_``.
419-  Forward-declare classes in your header files, instead of including
420   them, whenever possible. For example, if you have an interface with a
421   ``void DoSomething(nsIContent* aContent)`` function, forward-declare
422   with ``class nsIContent;`` instead of ``#include "nsIContent.h"``.
423   If a "forwarding header" is provided for a type, include that instead of
424   putting the literal forward declaration(s) in your header file. E.g. for
425   some JavaScript types, there is ``js/TypeDecls.h``, for the string types
426   there is ``StringFwd.h``. One reason for this is that this allows
427   changing a type to a type alias by only changing the forwarding header.
428   The following uses of a type can be done with a forward declaration only:
429
430   -  Parameter or return type in a function declaration
431   -  Member/local variable pointer or reference type
432   -  Use as a template argument (not in all cases) in a member/local variable type
433   -  Defining a type alias
434
435   The following uses of a type require a full definition:
436
437   -  Base class
438   -  Member/local variable type
439   -  Use with delete or new
440   -  Use as a template argument (not in all cases)
441   -  Any uses of non-scoped enum types
442   -  Enum values of a scoped enum type
443
444   Use as a template argument is somewhat tricky. It depends on how the
445   template uses the type. E.g. ``mozilla::Maybe<T>`` and ``AutoTArray<T>``
446   always require a full definition of ``T`` because the size of the
447   template instance depends on the size of ``T``. ``RefPtr<T>`` and
448   ``UniquePtr<T>`` don't require a full definition (because their
449   pointer member always has the same size), but their destructor
450   requires a full definition. If you encounter a template that cannot
451   be instantiated with a forward declaration only, but it seems
452   it should be possible, please file a bug (if it doesn't exist yet).
453
454   Therefore, also consider the following guidelines to allow using forward
455   declarations as widely as possible.
456-  Inline function bodies in header files often pull in a lot of additional
457   dependencies. Be mindful when adding or extending inline function bodies,
458   and consider moving the function body to the cpp file or to a separate
459   header file that is not included everywhere. Bug 1677553 intends to provide
460   a more specific guideline on this.
461-  Consider the use of the `Pimpl idiom <https://en.cppreference.com/w/cpp/language/pimpl>`__,
462   i.e. hide the actual implementation in a separate ``Impl`` class that is
463   defined in the implementation file and only expose a ``class Impl;`` forward
464   declaration and ``UniquePtr<Impl>`` member in the header file.
465-  Do not use non-scoped enum types. These cannot be forward-declared. Use
466   scoped enum types instead, and forward declare them when possible.
467-  Avoid nested types that need to be referenced from outside the class.
468   These cannot be forward declared. Place them in a namespace instead, maybe
469   in an extra inner namespace, and forward declare them where possible.
470-  Avoid mixing declarations with different sets of dependencies in a single
471   header file. This is generally advisable, but even more so when some of these
472   declarations are used by a subset of the translation units that include the
473   combined header file only. Consider such a badly mixed header file like:
474
475   .. code-block:: cpp
476
477      /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
478      /* vim: set ts=8 sts=2 et sw=2 tw=80: */
479      /* This Source Code Form is subject to the terms of the Mozilla Public
480      * License, v. 2.0. If a copy of the MPL was not distributed with this file,
481      * You can obtain one at http://mozilla.org/MPL/2.0/. */
482
483      #ifndef BAD_MIXED_FILE_H_
484      #define BAD_MIXED_FILE_H_
485
486      // Only this include is needed for the function declaration below.
487      #include "nsCOMPtr.h"
488
489      // These includes are only needed for the class definition.
490      #include "nsIFile.h"
491      #include "mozilla/ComplexBaseClass.h"
492
493      namespace mozilla {
494
495      class WrappedFile : public nsIFile, ComplexBaseClass {
496      // ... class definition left out for clarity
497      };
498
499      // Assuming that most translation units that include this file only call
500      // the function, but don't need the class definition, this should be in a
501      // header file on its own in order to avoid pulling in the other
502      // dependencies everywhere.
503      nsCOMPtr<nsIFile> CreateDefaultWrappedFile(nsCOMPtr<nsIFile>&& aFileToWrap);
504
505      } // namespace mozilla
506
507      #endif // BAD_MIXED_FILE_H_
508
509
510An example header file based on these rules (with some extra comments):
511
512.. code-block:: cpp
513
514   /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
515   /* vim: set ts=8 sts=2 et sw=2 tw=80: */
516   /* This Source Code Form is subject to the terms of the Mozilla Public
517   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
518   * You can obtain one at http://mozilla.org/MPL/2.0/. */
519
520   #ifndef DOM_BASE_FOO_H_
521   #define DOM_BASE_FOO_H_
522
523   // Include guards should come at the very beginning and always use exactly
524   // the style above. Otherwise, compiler optimizations that avoid rescanning
525   // repeatedly included headers might not hit and cause excessive compile
526   // times.
527
528   #include <cstdint>
529   #include "nsCOMPtr.h"  // This is needed because we have a nsCOMPtr<T> data member.
530
531   class nsIFile;  // Used as a template argument only.
532   enum class nsresult : uint32_t; // Used as a parameter type only.
533   template <class T>
534   class RefPtr;   // Used as a return type only.
535
536   namespace mozilla::dom {
537
538   class Document; // Used as a template argument only.
539
540   // Scoped enum, not as a nested type, so it can be
541   // forward-declared elsewhere.
542   enum class FooKind { Small, Big };
543
544   class Foo {
545   public:
546     // Do not put the implementation in the header file, it would
547     // require including nsIFile.h
548     Foo(nsCOMPtr<nsIFile> aFile, FooKind aFooKind);
549
550     RefPtr<Document> CreateDocument();
551
552     void SetResult(nsresult aResult);
553
554     // Even though we will default this destructor, do this in the
555     // implementation file since we would otherwise need to include
556     // nsIFile.h in the header.
557     ~Foo();
558
559   private:
560     nsCOMPtr<nsIFile> mFile;
561   };
562
563   } // namespace mozilla::dom
564
565   #endif // DOM_BASE_FOO_H_
566
567
568Corresponding implementation file:
569
570.. code-block:: cpp
571
572   /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
573   /* vim: set ts=8 sts=2 et sw=2 tw=80: */
574   /* This Source Code Form is subject to the terms of the Mozilla Public
575   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
576   * You can obtain one at http://mozilla.org/MPL/2.0/. */
577
578   #include "mozilla/dom/Foo.h"  // corresponding header
579
580   #include "mozilla/Assertions.h"  // Needed for MOZ_ASSERT.
581   #include "mozilla/dom/Document.h" // Needed because we construct a Document.
582   #include "nsError.h"  // Needed because we use NS_OK aka nsresult::NS_OK.
583   #include "nsIFile.h"  // This is needed because our destructor indirectly calls delete nsIFile in a template instance.
584
585   namespace mozilla::dom {
586
587   // Do not put the implementation in the header file, it would
588   // require including nsIFile.h
589   Foo::Foo(nsCOMPtr<nsIFile> aFile, FooKind aFooKind)
590    : mFile{std::move(aFile)} {
591   }
592
593   RefPtr<Document> Foo::CreateDocument() {
594     return MakeRefPtr<Document>();
595   }
596
597   void Foo::SetResult(nsresult aResult) {
598      MOZ_ASSERT(aResult != NS_OK);
599
600      // do something with aResult
601   }
602
603   // Even though we default this destructor, do this in the
604   // implementation file since we would otherwise need to include
605   // nsIFile.h in the header.
606   Foo::~Foo() = default;
607
608   } // namespace mozilla::dom
609
610
611Include directives
612------------------
613
614- Ordering:
615
616  - In an implementation file (cpp file), the very first include directive
617    should include the corresponding header file, followed by a blank line.
618  - Any conditional includes (depending on some ``#ifdef`` or similar) follow
619    after non-conditional includes. Don't mix them in.
620  - Don't place comments between non-conditional includes.
621
622  Bug 1679522 addresses automating the ordering via clang-format, which
623  is going to enforce some stricter rules. Expect the includes to be reordered.
624  If you include third-party headers that are not self-contained, and therefore
625  need to be included in a particular order, enclose those (and only those)
626  between ``// clang-format off`` and ``// clang-format on``. This should not be
627  done for Mozilla headers, which should rather be made self-contained if they
628  are not.
629
630- Brackets vs. quotes: C/C++ standard library headers are included using
631  brackets (e.g. ``#include <cstdint>``), all other include directives use
632  (double) quotes (e.g. ``#include "mozilla/dom/Document.h``).
633- Exported headers should always be included from their exported path, not
634  from their source path in the tree, even if available locally. E.g. always
635  do ``#include "mozilla/Vector.h"``, not ``#include "Vector.h"``, even
636  from within `mfbt`.
637- Generally, you should include exactly those headers that are needed, not
638  more and not less. Unfortunately this is not easy to see. Maybe C++20
639  modules will bring improvements to this, but it will take a long time
640  to be adopted.
641- The basic rule is that if you literally use a symbol in your file that
642  is declared in a header A.h, include that header. In particular in header
643  files, check if a forward declaration or including a forwarding header is
644  sufficient, see section :ref:`Header files`.
645
646  There are cases where this basic rule is not sufficient. Some cases where
647  you need to include additional headers are:
648
649  - You reference a member of a type that is not literally mentioned in your
650    code, but, e.g. is the return type of a function you are calling.
651
652  There are also cases where the basic rule leads to redundant includes. Note
653  that "redundant" here does not refer to "accidentally redundant" headers,
654  e.g. at the time of writing ``mozilla/dom/BodyUtil.h`` includes
655  ``mozilla/dom/FormData.h``, but it doesn't need to (it only needs a forward
656  declaration), so including ``mozilla/dom/FormData.h`` is "accidentally
657  redundant" when including ``mozilla/dom/BodyUtil.h``. The includes of
658  ``mozilla/dom/BodyUtil.h`` might change at any time, so if a file that
659  includes ``mozilla/dom/BodyUtil.h`` needs a full definition of
660  ``mozilla::dom::FormData``, it should includes ``mozilla/dom/FormData.h``
661  itself. In fact, these "accidentally redundant" headers MUST be included.
662  Relying on accidentally redundant includes makes any change to a header
663  file extremely hard, in particular when considering that the set of
664  accidentally redundant includes differs between platforms.
665  But some cases in fact are non-accidentally redundant, and these can and
666  typically should not be repeated:
667
668  - The includes of the header file do not need to be repeated in its
669    corresponding implementation file. Rationale: the implementation file and
670    its corresponding header file are tightly coupled per se.
671
672  Macros are a special case. Generally, the literal rule also applies here,
673  i.e. if the macro definition references a symbol, the file containing the
674  macro definition should include the header defining the symbol. E.g.
675  ``NS_IMPL_CYCLE_COLLECTING_NATIVE_RELEASE`` defined in ``nsISupportsImpl.h``
676  makes use of ``MOZ_ASSERT`` defined in ``mozilla/Assertions.h``, so
677  ``nsISupportsImpl.h`` includes ``mozilla/Assertions.h``. However, this
678  requires human judgment of what is intended, since technically only the
679  invocations of the macro reference a symbol (and that's how
680  include-what-you-use handles this). It might depend on the
681  context or parameters which symbol is actually referenced, and sometimes
682  this is on purpose. In these cases, the user of the macro needs to include
683  the required header(s).
684
685
686
687COM and pointers
688----------------
689
690-  Use ``nsCOMPtr<>``
691   If you don't know how to use it, start looking in the code for
692   examples. The general rule, is that the very act of typing
693   ``NS_RELEASE`` should be a signal to you to question your code:
694   "Should I be using ``nsCOMPtr`` here?". Generally the only valid use
695   of ``NS_RELEASE`` is when you are storing refcounted pointers in a
696   long-lived datastructure.
697-  Declare new XPCOM interfaces using `XPIDL <https://developer.mozilla.org/docs/Mozilla/Tech/XPIDL>`__, so they
698   will be scriptable.
699-  Use `nsCOMPtr <https://developer.mozilla.org/docs/Mozilla/Tech/XPCOM/Reference/Glue_classes/nsCOMPtr>`__ for strong references, and
700   `nsWeakPtr <https://developer.mozilla.org/docs/Mozilla/Tech/XPCOM/Weak_reference>`__ for weak references.
701-  Don't use ``QueryInterface`` directly. Use ``CallQueryInterface`` or
702   ``do_QueryInterface`` instead.
703-  Use `Contract
704   IDs <news://news.mozilla.org/3994AE3E.D96EF810@netscape.com>`__,
705   instead of CIDs with ``do_CreateInstance``/``do_GetService``.
706-  Use pointers, instead of references for function out parameters, even
707   for primitive types.
708
709
710IDL
711---
712
713
714Use leading-lowercase, or "interCaps"
715~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
716
717When defining a method or attribute in IDL, the first letter should be
718lowercase, and each following word should be capitalized. For example:
719
720.. code-block:: cpp
721
722   long updateStatusBar();
723
724
725Use attributes wherever possible
726~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
727
728Whenever you are retrieving or setting a single value, without any
729context, you should use attributes. Don't use two methods when you could
730use an attribute. Using attributes logically connects the getting and
731setting of a value, and makes scripted code look cleaner.
732
733This example has too many methods:
734
735.. code-block:: cpp
736
737   interface nsIFoo : nsISupports
738   {
739       long getLength();
740       void setLength(in long length);
741       long getColor();
742   };
743
744The code below will generate the exact same C++ signature, but is more
745script-friendly.
746
747.. code-block:: cpp
748
749   interface nsIFoo : nsISupports
750   {
751       attribute long length;
752       readonly attribute long color;
753   };
754
755
756Use Java-style constants
757~~~~~~~~~~~~~~~~~~~~~~~~
758
759When defining scriptable constants in IDL, the name should be all
760uppercase, with underscores between words:
761
762.. code-block:: cpp
763
764   const long ERROR_UNDEFINED_VARIABLE = 1;
765
766
767See also
768~~~~~~~~
769
770For details on interface development, as well as more detailed style
771guides, see the `Interface development
772guide <https://developer.mozilla.org/docs/Mozilla/Developer_guide/Interface_development_guide>`__.
773
774
775Error handling
776--------------
777
778
779Check for errors early and often
780~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
781
782Every time you make a call into an XPCOM function, you should check for
783an error condition. You need to do this even if you know that call will
784never fail. Why?
785
786-  Someone may change the callee in the future to return a failure
787   condition.
788-  The object in question may live on another thread, another process,
789   or possibly even another machine. The proxy could have failed to make
790   your call in the first place.
791
792Also, when you make a new function which is failable (i.e. it will
793return a ``nsresult`` or a ``bool`` that may indicate an error), you should
794explicitly mark the return value should always be checked. For example:
795
796::
797
798   // for IDL.
799   [must_use] nsISupports
800   create();
801
802   // for C++, add this in *declaration*, do not add it again in implementation.
803   [[nodiscard]] nsresult
804   DoSomething();
805
806There are some exceptions:
807
808-  Predicates or getters, which return ``bool`` or ``nsresult``.
809-  IPC method implementation (For example, ``bool RecvSomeMessage()``).
810-  Most callers will check the output parameter, see below.
811
812.. code-block:: cpp
813
814   nsresult
815   SomeMap::GetValue(const nsString& key, nsString& value);
816
817If most callers need to check the output value first, then adding
818``[[nodiscard]]`` might be too verbose. In this case, change the return value
819to void might be a reasonable choice.
820
821There is also a static analysis attribute ``[[nodiscard]]``, which can
822be added to class declarations, to ensure that those declarations are
823always used when they are returned.
824
825
826Use the NS_WARN_IF macro when errors are unexpected.
827~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
828
829The ``NS_WARN_IF`` macro can be used to issue a console warning, in debug
830builds if the condition fails. This should only be used when the failure
831is unexpected and cannot be caused by normal web content.
832
833If you are writing code which wants to issue warnings when methods fail,
834please either use ``NS_WARNING`` directly, or use the new ``NS_WARN_IF`` macro.
835
836.. code-block:: cpp
837
838   if (NS_WARN_IF(somethingthatshouldbefalse)) {
839     return NS_ERROR_INVALID_ARG;
840   }
841
842   if (NS_WARN_IF(NS_FAILED(rv))) {
843     return rv;
844   }
845
846Previously, the ``NS_ENSURE_*`` macros were used for this purpose, but
847those macros hide return statements, and should not be used in new code.
848(This coding style rule isn't generally agreed, so use of ``NS_ENSURE_*``
849can be valid.)
850
851
852Return from errors immediately
853~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
854
855In most cases, your knee-jerk reaction should be to return from the
856current function, when an error condition occurs. Don't do this:
857
858.. code-block:: cpp
859
860   rv = foo->Call1();
861   if (NS_SUCCEEDED(rv)) {
862     rv = foo->Call2();
863     if (NS_SUCCEEDED(rv)) {
864       rv = foo->Call3();
865     }
866   }
867   return rv;
868
869Instead, do this:
870
871.. code-block:: cpp
872
873   rv = foo->Call1();
874   if (NS_FAILED(rv)) {
875     return rv;
876   }
877
878   rv = foo->Call2();
879   if (NS_FAILED(rv)) {
880     return rv;
881   }
882
883   rv = foo->Call3();
884   if (NS_FAILED(rv)) {
885     return rv;
886   }
887
888Why? Error handling should not obfuscate the logic of the code. The
889author's intent, in the first example, was to make 3 calls in
890succession. Wrapping the calls in nested if() statements, instead
891obscured the most likely behavior of the code.
892
893Consider a more complicated example to hide a bug:
894
895.. code-block:: cpp
896
897   bool val;
898   rv = foo->GetBooleanValue(&val);
899   if (NS_SUCCEEDED(rv) && val) {
900     foo->Call1();
901   } else {
902     foo->Call2();
903   }
904
905The intent of the author, may have been, that ``foo->Call2()`` would only
906happen when val had a false value. In fact, ``foo->Call2()`` will also be
907called, when ``foo->GetBooleanValue(&val)`` fails. This may, or may not,
908have been the author's intent. It is not clear from this code. Here is
909an updated version:
910
911.. code-block:: cpp
912
913   bool val;
914   rv = foo->GetBooleanValue(&val);
915   if (NS_FAILED(rv)) {
916     return rv;
917   }
918   if (val) {
919     foo->Call1();
920   } else {
921     foo->Call2();
922   }
923
924In this example, the author's intent is clear, and an error condition
925avoids both calls to ``foo->Call1()`` and ``foo->Call2();``
926
927*Possible exceptions:* Sometimes it is not fatal if a call fails. For
928instance, if you are notifying a series of observers that an event has
929fired, it might be trivial that one of these notifications failed:
930
931.. code-block:: cpp
932
933   for (size_t i = 0; i < length; ++i) {
934     // we don't care if any individual observer fails
935     observers[i]->Observe(foo, bar, baz);
936   }
937
938Another possibility, is you are not sure if a component exists or is
939installed, and you wish to continue normally, if the component is not
940found.
941
942.. code-block:: cpp
943
944   nsCOMPtr<nsIMyService> service = do_CreateInstance(NS_MYSERVICE_CID, &rv);
945   // if the service is installed, then we'll use it.
946   if (NS_SUCCEEDED(rv)) {
947     // non-fatal if this fails too, ignore this error.
948     service->DoSomething();
949
950     // this is important, handle this error!
951     rv = service->DoSomethingImportant();
952     if (NS_FAILED(rv)) {
953       return rv;
954     }
955   }
956
957   // continue normally whether or not the service exists.
958
959
960Strings
961-------
962
963.. note::
964
965   This section overlaps with the more verbose advice given in
966   `Internal strings <https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings>`__.
967   These should eventually be merged. For now, please refer to that guide for
968   more advice.
969
970-  String arguments to functions should be declared as ``[const] nsA[C]String&``.
971-  Prefer using string literals. In particular, use empty string literals,
972   i.e. ``u""_ns`` or ``""_ns``, instead of ``Empty[C]String()`` or
973   ``const nsAuto[C]String empty;``. Use ``Empty[C]String()`` only if you
974   specifically need a ``const ns[C]String&``, e.g. with the ternary operator
975   or when you need to return/bind to a reference or take the address of the
976   empty string.
977-  For 16-bit literal strings, use ``u"..."_ns`` or, if necessary
978   ``NS_LITERAL_STRING_FROM_CSTRING(...)`` instead of ``nsAutoString()``
979   or other ways that would do a run-time conversion.
980   See :ref:`Avoid runtime conversion of string literals <Avoid runtime conversion of string literals>` below.
981-  To compare a string with a literal, use ``.EqualsLiteral("...")``.
982-  Use ``str.IsEmpty()`` instead of ``str.Length() == 0``.
983-  Use ``str.Truncate()`` instead of ``str.SetLength(0)``,
984   ``str.Assign(""_ns)`` or ``str.AssignLiteral("")``.
985-  Don't use functions from ``ctype.h`` (``isdigit()``, ``isalpha()``,
986   etc.) or from ``strings.h`` (``strcasecmp()``, ``strncasecmp()``).
987   These are locale-sensitive, which makes them inappropriate for
988   processing protocol text. At the same time, they are too limited to
989   work properly for processing natural-language text. Use the
990   alternatives in ``mozilla/TextUtils.h`` and in ``nsUnicharUtils.h``
991   in place of ``ctype.h``. In place of ``strings.h``, prefer the
992   ``nsStringComparator`` facilities for comparing strings or if you
993   have to work with zero-terminated strings, use ``nsCRT.h`` for
994   ASCII-case-insensitive comparison.
995
996
997Use the ``Auto`` form of strings for local values
998~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
999
1000When declaring a local, short-lived ``nsString`` class, always use
1001``nsAutoString`` or ``nsAutoCString``. These pre-allocate a 64-byte
1002buffer on the stack, and avoid fragmenting the heap. Don't do this:
1003
1004.. code-block:: cpp
1005
1006   nsresult
1007   foo()
1008   {
1009     nsCString bar;
1010     ..
1011   }
1012
1013instead:
1014
1015.. code-block:: cpp
1016
1017   nsresult
1018   foo()
1019   {
1020     nsAutoCString bar;
1021     ..
1022   }
1023
1024
1025Be wary of leaking values from non-XPCOM functions that return char\* or PRUnichar\*
1026~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1027
1028It is an easy trap to return an allocated string, from an internal
1029helper function, and then using that function inline in your code,
1030without freeing the value. Consider this code:
1031
1032.. code-block:: cpp
1033
1034   static char*
1035   GetStringValue()
1036   {
1037     ..
1038     return resultString.ToNewCString();
1039   }
1040
1041     ..
1042     WarnUser(GetStringValue());
1043
1044In the above example, ``WarnUser`` will get the string allocated from
1045``resultString.ToNewCString()`` and throw away the pointer. The
1046resulting value is never freed. Instead, either use the string classes,
1047to make sure your string is automatically freed when it goes out of
1048scope, or make sure that your string is freed.
1049
1050Automatic cleanup:
1051
1052.. code-block:: cpp
1053
1054   static void
1055   GetStringValue(nsAWritableCString& aResult)
1056   {
1057     ..
1058     aResult.Assign("resulting string");
1059   }
1060
1061     ..
1062     nsAutoCString warning;
1063     GetStringValue(warning);
1064     WarnUser(warning.get());
1065
1066Free the string manually:
1067
1068.. code-block:: cpp
1069
1070   static char*
1071   GetStringValue()
1072   {
1073     ..
1074     return resultString.ToNewCString();
1075   }
1076
1077     ..
1078     char* warning = GetStringValue();
1079     WarnUser(warning);
1080     nsMemory::Free(warning);
1081
1082.. _Avoid runtime conversion of string literals:
1083
1084Avoid runtime conversion of string literals
1085~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1086
1087It is very common to need to assign the value of a literal string, such
1088as ``"Some String"``, into a unicode buffer. Instead of using ``nsString``'s
1089``AssignLiteral`` and ``AppendLiteral``, use a user-defined literal like `u"foo"_ns`
1090instead. On most platforms, this will force the compiler to compile in a
1091raw unicode string, and assign it directly. In cases where the literal is defined
1092via a macro that is used in both 8-bit and 16-bit ways, you can use
1093`NS_LITERAL_STRING_FROM_CSTRING` to do the conversion at compile time.
1094
1095Incorrect:
1096
1097.. code-block:: cpp
1098
1099   nsAutoString warning;
1100   warning.AssignLiteral("danger will robinson!");
1101   ...
1102   foo->SetStringValue(warning);
1103   ...
1104   bar->SetUnicodeValue(warning.get());
1105
1106Correct:
1107
1108.. code-block:: cpp
1109
1110   constexpr auto warning = u"danger will robinson!"_ns;
1111   ...
1112   // if you'll be using the 'warning' string, you can still use it as before:
1113   foo->SetStringValue(warning);
1114   ...
1115   bar->SetUnicodeValue(warning.get());
1116
1117   // alternatively, use the wide string directly:
1118   foo->SetStringValue(u"danger will robinson!"_ns);
1119   ...
1120
1121   // if a macro is the source of a 8-bit literal and you cannot change it, use
1122   // NS_LITERAL_STRING_FROM_CSTRING, but only if necessary.
1123   #define MY_MACRO_LITERAL "danger will robinson!"
1124   foo->SetStringValue(NS_LITERAL_STRING_FROM_CSTRING(MY_MACRO_LITERAL));
1125
1126   // If you need to pass to a raw const char16_t *, there's no benefit to
1127   // go through our string classes at all, just do...
1128   bar->SetUnicodeValue(u"danger will robinson!");
1129
1130   // .. or, again, if a macro is the source of a 8-bit literal
1131   bar->SetUnicodeValue(u"" MY_MACRO_LITERAL);
1132
1133
1134Usage of PR_(MAX|MIN|ABS|ROUNDUP) macro calls
1135---------------------------------------------
1136
1137Use the standard-library functions (``std::max``), instead of
1138``PR_(MAX|MIN|ABS|ROUNDUP)``.
1139
1140Use ``mozilla::Abs`` instead of ``PR_ABS``. All ``PR_ABS`` calls in C++ code have
1141been replaced with ``mozilla::Abs`` calls, in `bug
1142847480 <https://bugzilla.mozilla.org/show_bug.cgi?id=847480>`__. All new
1143code in ``Firefox/core/toolkit`` needs to ``#include "nsAlgorithm.h"`` and
1144use the ``NS_foo`` variants instead of ``PR_foo``, or
1145``#include "mozilla/MathAlgorithms.h"`` for ``mozilla::Abs``.
1146