summaryrefslogtreecommitdiffstats
path: root/docs/code-quality/coding-style/coding_style_cpp.rst
blob: cb4764cea588cf8615e0733c750230495636048c (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966
967
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013
1014
1015
1016
1017
1018
1019
1020
1021
1022
1023
1024
1025
1026
1027
1028
1029
1030
1031
1032
1033
1034
1035
1036
1037
1038
1039
1040
1041
1042
1043
1044
1045
1046
1047
1048
1049
1050
1051
1052
1053
1054
1055
1056
1057
1058
1059
1060
1061
1062
1063
1064
1065
1066
1067
1068
1069
1070
1071
1072
1073
1074
1075
1076
1077
1078
1079
1080
1081
1082
1083
1084
1085
1086
1087
1088
1089
1090
1091
1092
1093
1094
1095
1096
1097
1098
1099
1100
1101
1102
1103
1104
1105
1106
1107
1108
1109
1110
1111
1112
1113
1114
1115
1116
1117
1118
1119
1120
1121
1122
1123
1124
1125
1126
1127
1128
1129
1130
1131
1132
1133
1134
1135
1136
1137
1138
1139
1140
1141
1142
1143
1144
1145
1146
1147
1148
1149
1150
================
C++ Coding style
================


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

This article is particularly for those new to the Mozilla codebase, and
in the process of getting their code reviewed. Before requesting a
review, please read over this document, making sure that your code
conforms to recommendations.

.. container:: blockIndicator warning

   The Firefox code base adopts parts of the `Google Coding style for C++
   code <https://google.github.io/styleguide/cppguide.html>`__, but not all of its rules.
   A few rules are followed across the code base, others are intended to be
   followed in new or significantly revised code. We may extend this list in the
   future, when we evaluate the Google Coding Style for C++ Code further and/or update
   our coding practices. However, the plan is not to adopt all rules of the Google Coding
   Style for C++ Code. Some rules are explicitly unlikely to be adopted at any time.

   Followed across the code base:

   - `Formatting <https://google.github.io/styleguide/cppguide.html#Formatting>`__,
     except for subsections noted here otherwise
   - `Implicit Conversions <https://google.github.io/styleguide/cppguide.html#Implicit_Conversions>`__,
     which is enforced by a custom clang-plugin check, unless explicitly overridden using
     ``MOZ_IMPLICIT``

   Followed in new/significantly revised code:

   - `Include guards <https://google.github.io/styleguide/cppguide.html#The__define_Guard>`__

   Unlikely to be ever adopted:

   - `Forward declarations <https://google.github.io/styleguide/cppguide.html#Forward_Declarations>`__
   - `Formatting/Conditionals <https://google.github.io/styleguide/cppguide.html#Conditionals>`__
     w.r.t. curly braces around inner statements, we require them in all cases where the
     Google style allows to leave them out for single-line conditional statements

   This list reflects the state of the Google Google Coding Style for C++ Code as of
   2020-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-tree
configuration files. See :ref:`Formatting C++ Code With clang-format`
for more information.

Unix-style linebreaks (``\n``), not Windows-style (``\r\n``). You can
convert patches, with DOS newlines to Unix via the ``dos2unix`` utility,
or your favorite text editor.

Static analysis
---------------

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

``mach static-analysis`` provides a convenient way to run these checks. For example,
for the check called ``google-readability-braces-around-statements``, you can run:

.. code-block:: shell

   ./mach static-analysis check --checks="-*,google-readability-braces-around-statements" --fix <file>

It may be necessary to reformat the files after automatically applying fixes, see
:ref:`Formatting 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 are
in another team's codebase or it's not clear what style to use.*




Control structures
~~~~~~~~~~~~~~~~~~

Always brace controlled statements, even a single-line consequent of
``if else else``. This is redundant, typically, but it avoids dangling
else bugs, so it's safer at scale than fine-tuning.

Examples:

.. code-block:: cpp

   if (...) {
   } else if (...) {
   } else {
   }

   while (...) {
   }

   do {
   } while (...);

   for (...; ...; ...) {
   }

   switch (...) {
     case 1: {
       // When you need to declare a variable in a switch, put the block in braces.
       int var;
       break;
     }
     case 2:
       ...
       break;
     default:
       break;
   }

``else`` should only ever be followed by ``{`` or ``if``; i.e., other
control keywords are not allowed and should be placed inside braces.

.. note::

   For this rule, clang-tidy provides the ``google-readability-braces-around-statements``
   check with autofixes.


C++ namespaces
~~~~~~~~~~~~~~

Mozilla project C++ declarations should be in the ``mozilla``
namespace. Modules should avoid adding nested namespaces under
``mozilla``, unless they are meant to contain names which have a high
probability of colliding with other names in the code base. For example,
``Point``, ``Path``, etc. Such symbols can be put under
module-specific namespaces, under ``mozilla``, with short
all-lowercase names. Other global namespaces besides ``mozilla`` are
not allowed.

No ``using`` directives are allowed in header files, except inside class
definitions or functions. (We don't want to pollute the global scope of
compilation units that use the header file.)

.. note::

   For parts of this rule, clang-tidy provides the ``google-global-names-in-headers``
   check. It only detects ``using namespace`` directives in the global namespace.


``using namespace ...;`` is only allowed in ``.cpp`` files after all
``#include``\ s. Prefer to wrap code in ``namespace ... { ... };``
instead, if possible. ``using namespace ...;``\ should always specify
the fully qualified namespace. That is, to use ``Foo::Bar`` do not
write ``using namespace Foo; using namespace Bar;``, write
``using namespace Foo::Bar;``

Use nested namespaces (ex: ``namespace mozilla::widget {``

.. note::

   clang-tidy provides the ``modernize-concat-nested-namespaces``
   check with autofixes.


Anonymous namespaces
~~~~~~~~~~~~~~~~~~~~

We prefer using ``static``, instead of anonymous C++ namespaces. This may
change once there is better debugger support (especially on Windows) for
placing breakpoints, etc. on code in anonymous namespaces. You may still
use anonymous namespaces for things that can't be hidden with ``static``,
such as types, or certain objects which need to be passed to template
functions.


C++ classes
~~~~~~~~~~~~

.. code-block:: cpp

   namespace mozilla {

   class MyClass : public A
   {
     ...
   };

   class MyClass
     : public X
     , public Y
   {
   public:
     MyClass(int aVar, int aVar2)
       : 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.
     explicit MyClass(OtherClass aArg)
     {
       ...
     }

     // This constructor can also take a single argument, so it also needs to be marked
     // explicit.
     explicit MyClass(OtherClass aArg, AnotherClass aArg2 = AnotherClass())
     {
       ...
     }

     int LargerFunction()
     {
       ...
       ...
     }

   private:
     int mVar;
   };

   } // namespace mozilla

Define classes using the style given above.

.. note::

   For the rule on ``= default``, clang-tidy provides the ``modernize-use-default``
   check with autofixes.

   For the rule on explicit constructors and conversion operators, clang-tidy
   provides the ``mozilla-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 use ``UpperCamelCase``.

Getters that never fail, and never return null, are named ``Foo()``,
while all other getters use ``GetFoo()``. Getters can return an object
value, via a ``Foo** aResult`` outparam (typical for an XPCOM getter),
or as an ``already_AddRefed<Foo>`` (typical for a WebIDL getter,
possibly with an ``ErrorResult& rv`` parameter), or occasionally as a
``Foo*`` (typical for an internal getter for an object with a known
lifetime). See `the bug 223255 <https://bugzilla.mozilla.org/show_bug.cgi?id=223255>`_
for more information.

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

Method declarations must use, at most, one of the following keywords:
``virtual``, ``override``, or ``final``. Use ``virtual`` to declare
virtual methods, which do not override a base class method with the same
signature. Use ``override`` to declare virtual methods which do
override a base class method, with the same signature, but can be
further overridden in derived classes. Use ``final`` to declare virtual
methods which do override a base class method, with the same signature,
but can NOT be further overridden in the derived classes. This should
help the person reading the code fully understand what the declaration
is doing, without needing to further examine base classes.

.. note::

   For the rule on ``virtual/override/final``, clang-tidy provides the
   ``modernize-use-override`` check with autofixes.


Operators
~~~~~~~~~

The unary keyword operator ``sizeof``, should have its operand parenthesized
even if it is an expression; e.g. ``int8_t arr[64]; memset(arr, 42, sizeof(arr));``.


Literals
~~~~~~~~

Use ``\uXXXX`` unicode escapes for non-ASCII characters. The character
set for XUL, script, and properties files is UTF-8, which is not easily
readable.


Prefixes
~~~~~~~~

Follow these naming prefix conventions:


Variable prefixes
^^^^^^^^^^^^^^^^^

-  k=constant (e.g. ``kNC_child``). Not all code uses this style; some
   uses ``ALL_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. ``enum Foo { eBar, eBaz }``). Enum classes
      should use ``CamelCase`` instead (e.g.
      ``enum class Foo { Bar, Baz }``).


Global functions/macros/etc
^^^^^^^^^^^^^^^^^^^^^^^^^^^

-  Macros begin with ``MOZ_``, and are all caps (e.g.
   ``MOZ_WOW_GOODNESS``). Note that older code uses the ``NS_`` prefix;
   while these aren't being changed, you should only use ``MOZ_`` for
   new macros. The only exception is if you're creating a new macro,
   which is part of a set of related macros still using the old ``NS_``
   prefix. Then you should be consistent with the existing macros.


Error Variables
^^^^^^^^^^^^^^^

-  Local variables that are assigned ``nsresult`` result codes should be named ``rv``
   (i.e., e.g., not ``res``, not ``result``, not ``foo``). `rv` should not be
   used for bool or other result types.
-  Local variables that are assigned ``bool`` result codes should be named `ok`.


C/C++ practices
---------------

-  **Have you checked for compiler warnings?** Warnings often point to
   real bugs. `Many of them <https://searchfox.org/mozilla-central/source/build/moz.configure/warnings.configure>`__
   are enabled by default in the build system.
-  In C++ code, use ``nullptr`` for pointers. In C code, using ``NULL``
   or ``0`` is allowed.

.. note::

   For the C++ rule, clang-tidy provides the ``modernize-use-nullptr`` check
   with autofixes.

-  Don't use ``PRBool`` and ``PRPackedBool`` in C++, use ``bool``
   instead.
-  For checking if a ``std`` container has no items, don't use
   ``size()``, instead use ``empty()``.
-  When testing a pointer, use ``(!myPtr)`` or ``(myPtr)``;
   don't use ``myPtr != nullptr`` or ``myPtr == nullptr``.
-  Do not compare ``x == true`` or ``x == false``. Use ``(x)`` or
   ``(!x)`` instead. ``if (x == true)`` may have semantics different from
   ``if (x)``!

.. note::

   clang-tidy provides the ``readability-simplify-boolean-expr`` check
   with autofixes that checks for these and some other boolean expressions
   that can be simplified.

-  In general, initialize variables with ``nsFoo aFoo = bFoo,`` and not
   ``nsFoo aFoo(bFoo)``.

   -  For constructors, initialize member variables with : ``nsFoo
      aFoo(bFoo)`` syntax.

-  To avoid warnings created by variables used only in debug builds, use
   the
   `DebugOnly<T> <https://developer.mozilla.org/docs/Mozilla/Debugging/DebugOnly%3CT%3E>`__
   helper when declaring them.
-  You should `use the static preference
   API <https://firefox-source-docs.mozilla.org/modules/libpref/index.html>`__ for
   working with preferences.
-  One-argument constructors, that are not copy or move constructors,
   should generally be marked explicit. Exceptions should be annotated
   with ``MOZ_IMPLICIT``.
-  Use ``char32_t`` as the return type or argument type of a method that
   returns or takes as argument a single Unicode scalar value. (Don't
   use UTF-32 strings, though.)
-  Prefer unsigned types for semantically-non-negative integer values.
-  When operating on integers that could overflow, use ``CheckedInt``.
-  Avoid the usage of ``typedef``, instead, please use ``using`` instead.

.. note::

   For parts of this rule, clang-tidy provides the ``modernize-use-using``
   check with autofixes.


Header files
------------

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

-  Include guards are named per the Google coding style (i.e. upper snake
   case with a single trailing underscore). They should not include a
   leading ``MOZ_`` or ``MOZILLA_``. For example, ``dom/media/foo.h``
   would use the guard ``DOM_MEDIA_FOO_H_``.
-  Forward-declare classes in your header files, instead of including
   them, whenever possible. For example, if you have an interface with a
   ``void DoSomething(nsIContent* aContent)`` function, forward-declare
   with ``class nsIContent;`` instead of ``#include "nsIContent.h"``.
   If a "forwarding header" is provided for a type, include that instead of
   putting the literal forward declaration(s) in your header file. E.g. for
   some JavaScript types, there is ``js/TypeDecls.h``, for the string types
   there is ``StringFwd.h``. One reason for this is that this allows
   changing 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 the
   template uses the type. E.g. ``mozilla::Maybe<T>`` and ``AutoTArray<T>``
   always require a full definition of ``T`` because the size of the
   template instance depends on the size of ``T``. ``RefPtr<T>`` and
   ``UniquePtr<T>`` don't require a full definition (because their
   pointer member always has the same size), but their destructor
   requires a full definition. If you encounter a template that cannot
   be instantiated with a forward declaration only, but it seems
   it should be possible, please file a bug (if it doesn't exist yet).

   Therefore, also consider the following guidelines to allow using forward
   declarations as widely as possible.
-  Inline function bodies in header files often pull in a lot of additional
   dependencies. Be mindful when adding or extending inline function bodies,
   and consider moving the function body to the cpp file or to a separate
   header file that is not included everywhere. Bug 1677553 intends to provide
   a more specific guideline on this.
-  Consider the use of the `Pimpl idiom <https://en.cppreference.com/w/cpp/language/pimpl>`__,
   i.e. hide the actual implementation in a separate ``Impl`` class that is
   defined in the implementation file and only expose a ``class Impl;`` forward
   declaration and ``UniquePtr<Impl>`` member in the header file.
-  Do not use non-scoped enum types. These cannot be forward-declared. Use
   scoped 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, maybe
   in an extra inner namespace, and forward declare them where possible.
-  Avoid mixing declarations with different sets of dependencies in a single
   header file. This is generally advisable, but even more so when some of these
   declarations are used by a subset of the translation units that include the
   combined header file only. Consider such a badly mixed header file like:

   .. code-block:: cpp

      /* -*- 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"

      namespace mozilla {

      class WrappedFile : public nsIFile, 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):

.. code-block:: cpp

   /* -*- 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.

   class nsIFile;  // Used as a template argument only.
   enum class nsresult : uint32_t; // Used as a parameter type only.
   template <class T>
   class RefPtr;   // Used as a return type only.

   namespace mozilla::dom {

   class Document; // Used as a template argument only.

   // Scoped enum, not as a nested type, so it can be
   // forward-declared elsewhere.
   enum class FooKind { Small, Big };

   class Foo {
   public:
     // Do not put the implementation in the header file, it would
     // require including nsIFile.h
     Foo(nsCOMPtr<nsIFile> aFile, FooKind aFooKind);

     RefPtr<Document> CreateDocument();

     void SetResult(nsresult aResult);

     // 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:

.. code-block:: cpp

   /* -*- 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.

   namespace mozilla::dom {

   // Do not put the implementation in the header file, it would
   // require including nsIFile.h
   Foo::Foo(nsCOMPtr<nsIFile> aFile, FooKind aFooKind)
    : mFile{std::move(aFile)} {
   }

   RefPtr<Document> Foo::CreateDocument() {
     return MakeRefPtr<Document>();
   }

   void Foo::SetResult(nsresult aResult) {
      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 directive
    should include the corresponding header file, followed by a blank line.
  - Any conditional includes (depending on some ``#ifdef`` or similar) follow
    after 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, which
  is going to enforce some stricter rules. Expect the includes to be reordered.
  If you include third-party headers that are not self-contained, and therefore
  need to be included in a particular order, enclose those (and only those)
  between ``// clang-format off`` and ``// clang-format on``. This should not be
  done for Mozilla headers, which should rather be made self-contained if they
  are not.

- Brackets vs. quotes: C/C++ standard library headers are included using
  brackets (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, not
  from their source path in the tree, even if available locally. E.g. always
  do ``#include "mozilla/Vector.h"``, not ``#include "Vector.h"``, even
  from within `mfbt`.
- Generally, you should include exactly those headers that are needed, not
  more and not less. Unfortunately this is not easy to see. Maybe C++20
  modules will bring improvements to this, but it will take a long time
  to be adopted.
- The basic rule is that if you literally use a symbol in your file that
  is declared in a header A.h, include that header. In particular in header
  files, check if a forward declaration or including a forwarding header is
  sufficient, see section :ref:`Header files`.

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

  - You reference a member of a type that is not literally mentioned in your
    code, 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. Note
  that "redundant" here does not refer to "accidentally redundant" headers,
  e.g. at the time of writing ``mozilla/dom/BodyUtil.h`` includes
  ``mozilla/dom/FormData.h``, but it doesn't need to (it only needs a forward
  declaration), so including ``mozilla/dom/FormData.h`` is "accidentally
  redundant" when including ``mozilla/dom/BodyUtil.h``. The includes of
  ``mozilla/dom/BodyUtil.h`` might change at any time, so if a file that
  includes ``mozilla/dom/BodyUtil.h`` needs a full definition of
  ``mozilla::dom::FormData``, it should includes ``mozilla/dom/FormData.h``
  itself. In fact, these "accidentally redundant" headers MUST be included.
  Relying on accidentally redundant includes makes any change to a header
  file extremely hard, in particular when considering that the set of
  accidentally redundant includes differs between platforms.
  But some cases in fact are non-accidentally redundant, and these can and
  typically should not be repeated:

  - The includes of the header file do not need to be repeated in its
    corresponding implementation file. Rationale: the implementation file and
    its 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 the
  macro definition should include the header defining the symbol. E.g.
  ``NS_IMPL_CYCLE_COLLECTING_NATIVE_RELEASE`` defined in ``nsISupportsImpl.h``
  makes use of ``MOZ_ASSERT`` defined in ``mozilla/Assertions.h``, so
  ``nsISupportsImpl.h`` includes ``mozilla/Assertions.h``. However, this
  requires human judgment of what is intended, since technically only the
  invocations of the macro reference a symbol (and that's how
  include-what-you-use handles this). It might depend on the
  context or parameters which symbol is actually referenced, and sometimes
  this is on purpose. In these cases, the user of the macro needs to include
  the required header(s).



COM and pointers
----------------

-  Use ``nsCOMPtr<>``
   If you don't know how to use it, start looking in the code for
   examples. The general rule, is that the very act of typing
   ``NS_RELEASE`` should be a signal to you to question your code:
   "Should I be using ``nsCOMPtr`` here?". Generally the only valid use
   of ``NS_RELEASE`` is when you are storing refcounted pointers in a
   long-lived datastructure.
-  Declare new XPCOM interfaces using :doc:`XPIDL </xpcom/xpidl>`, so they
   will be scriptable.
-  Use :doc:`nsCOMPtr </xpcom/refptr>` for strong references, and
   ``nsWeakPtr`` for weak references.
-  Don't use ``QueryInterface`` directly. Use ``CallQueryInterface`` or
   ``do_QueryInterface`` instead.
-  Use :ref:`Contract IDs <contract_ids>`,
   instead of CIDs with ``do_CreateInstance``/``do_GetService``.
-  Use pointers, instead of references for function out parameters, even
   for primitive types.


IDL
---


Use leading-lowercase, or "interCaps"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

.. code-block:: cpp

   long updateStatusBar();


Use attributes wherever possible
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

This example has too many methods:

.. code-block:: cpp

   interface nsIFoo : nsISupports
   {
       long getLength();
       void setLength(in long length);
       long getColor();
   };

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

.. code-block:: cpp

   interface nsIFoo : nsISupports
   {
       attribute long length;
       readonly attribute long color;
   };


Use Java-style constants
~~~~~~~~~~~~~~~~~~~~~~~~

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

.. code-block:: cpp

   const long ERROR_UNDEFINED_VARIABLE = 1;


See also
~~~~~~~~

For details on interface development, as well as more detailed style
guides, see the `Interface development
guide <https://developer.mozilla.org/docs/Mozilla/Developer_guide/Interface_development_guide>`__.


Error handling
--------------


Check for errors early and often
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

-  Someone may change the callee in the future to return a failure
   condition.
-  The object in question may live on another thread, another process,
   or possibly even another machine. The proxy could have failed to make
   your call in the first place.

Also, when you make a new function which is failable (i.e. it will
return a ``nsresult`` or a ``bool`` that may indicate an error), you should
explicitly mark the return value should always be checked. For example:

::

   // for IDL.
   [must_use] nsISupports
   create();

   // for C++, add this in *declaration*, do not add it again in implementation.
   [[nodiscard]] nsresult
   DoSomething();

There are some exceptions:

-  Predicates or getters, which return ``bool`` or ``nsresult``.
-  IPC method implementation (For example, ``bool RecvSomeMessage()``).
-  Most callers will check the output parameter, see below.

.. code-block:: cpp

   nsresult
   SomeMap::GetValue(const nsString& 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 value
to void might be a reasonable choice.

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


Use the NS_WARN_IF macro when errors are unexpected.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The ``NS_WARN_IF`` macro can be used to issue a console warning, in debug
builds if the condition fails. This should only be used when the failure
is unexpected and cannot be caused by normal web content.

If you are writing code which wants to issue warnings when methods fail,
please either use ``NS_WARNING`` directly, or use the new ``NS_WARN_IF`` macro.

.. code-block:: cpp

   if (NS_WARN_IF(somethingthatshouldbefalse)) {
     return NS_ERROR_INVALID_ARG;
   }

   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }

Previously, the ``NS_ENSURE_*`` macros were used for this purpose, but
those macros hide return statements, and should not be used in new code.
(This coding style rule isn't generally agreed, so use of ``NS_ENSURE_*``
can be valid.)


Return from errors immediately
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

.. code-block:: cpp

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

Instead, do this:

.. code-block:: cpp

   rv = foo->Call1();
   if (NS_FAILED(rv)) {
     return rv;
   }

   rv = foo->Call2();
   if (NS_FAILED(rv)) {
     return rv;
   }

   rv = foo->Call3();
   if (NS_FAILED(rv)) {
     return rv;
   }

Why? Error handling should not obfuscate the logic of the code. The
author's intent, in the first example, was to make 3 calls in
succession. Wrapping the calls in nested if() statements, instead
obscured the most likely behavior of the code.

Consider a more complicated example to hide a bug:

.. code-block:: cpp

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

The intent of the author, may have been, that ``foo->Call2()`` would only
happen when val had a false value. In fact, ``foo->Call2()`` will also be
called, when ``foo->GetBooleanValue(&val)`` fails. This may, or may not,
have been the author's intent. It is not clear from this code. Here is
an updated version:

.. code-block:: cpp

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

In this example, the author's intent is clear, and an error condition
avoids both calls to ``foo->Call1()`` and ``foo->Call2();``

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

.. code-block:: cpp

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

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

.. code-block:: cpp

   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)) {
       return rv;
     }
   }

   // continue normally whether or not the service exists.


Strings
-------

.. note::

   This section overlaps with the more verbose advice given in
   :doc:`String guide </xpcom/stringguide>`.
   These should eventually be merged. For now, please refer to that guide for
   more 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 of ``Empty[C]String()`` or
   ``const nsAuto[C]String empty;``. Use ``Empty[C]String()`` only if you
   specifically need a ``const ns[C]String&``, e.g. with the ternary operator
   or when you need to return/bind to a reference or take the address of the
   empty string.
-  For 16-bit literal strings, use ``u"..."_ns`` or, if necessary
   ``NS_LITERAL_STRING_FROM_CSTRING(...)`` instead of ``nsAutoString()``
   or other ways that would do a run-time conversion.
   See :ref:`Avoid runtime conversion of string literals <Avoid runtime conversion of string literals>` below.
-  To compare a string with a literal, use ``.EqualsLiteral("...")``.
-  Use ``str.IsEmpty()`` instead of ``str.Length() == 0``.
-  Use ``str.Truncate()`` instead of ``str.SetLength(0)``,
   ``str.Assign(""_ns)`` or ``str.AssignLiteral("")``.
-  Don't use functions from ``ctype.h`` (``isdigit()``, ``isalpha()``,
   etc.) or from ``strings.h`` (``strcasecmp()``, ``strncasecmp()``).
   These are locale-sensitive, which makes them inappropriate for
   processing protocol text. At the same time, they are too limited to
   work properly for processing natural-language text. Use the
   alternatives in ``mozilla/TextUtils.h`` and in ``nsUnicharUtils.h``
   in place of ``ctype.h``. In place of ``strings.h``, prefer the
   ``nsStringComparator`` facilities for comparing strings or if you
   have to work with zero-terminated strings, use ``nsCRT.h`` for
   ASCII-case-insensitive comparison.


Use the ``Auto`` form of strings for local values
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When declaring a local, short-lived ``nsString`` class, always use
``nsAutoString`` or ``nsAutoCString``. These pre-allocate a 64-byte
buffer on the stack, and avoid fragmenting the heap. Don't do this:

.. code-block:: cpp

   nsresult
   foo()
   {
     nsCString bar;
     ..
   }

instead:

.. code-block:: cpp

   nsresult
   foo()
   {
     nsAutoCString bar;
     ..
   }


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 internal
helper function, and then using that function inline in your code,
without freeing the value. Consider this code:

.. code-block:: cpp

   static char*
   GetStringValue()
   {
     ..
     return resultString.ToNewCString();
   }

     ..
     WarnUser(GetStringValue());

In the above example, ``WarnUser`` will get the string allocated from
``resultString.ToNewCString()`` and throw away the pointer. The
resulting value is never freed. Instead, either use the string classes,
to make sure your string is automatically freed when it goes out of
scope, or make sure that your string is freed.

Automatic cleanup:

.. code-block:: cpp

   static void
   GetStringValue(nsAWritableCString& aResult)
   {
     ..
     aResult.Assign("resulting string");
   }

     ..
     nsAutoCString warning;
     GetStringValue(warning);
     WarnUser(warning.get());

Free the string manually:

.. code-block:: cpp

   static char*
   GetStringValue()
   {
     ..
     return resultString.ToNewCString();
   }

     ..
     char* warning = GetStringValue();
     WarnUser(warning);
     nsMemory::Free(warning);

.. _Avoid runtime conversion of string literals:

Avoid runtime conversion of string literals
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It is very common to need to assign the value of a literal string, such
as ``"Some String"``, into a unicode buffer. Instead of using ``nsString``'s
``AssignLiteral`` and ``AppendLiteral``, use a user-defined literal like `u"foo"_ns`
instead. On most platforms, this will force the compiler to compile in a
raw unicode string, and assign it directly. In cases where the literal is defined
via a macro that is used in both 8-bit and 16-bit ways, you can use
`NS_LITERAL_STRING_FROM_CSTRING` to do the conversion at compile time.

Incorrect:

.. code-block:: cpp

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

Correct:

.. code-block:: cpp

   constexpr auto warning = 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 literal
   bar->SetUnicodeValue(u"" MY_MACRO_LITERAL);


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

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

Use ``mozilla::Abs`` instead of ``PR_ABS``. All ``PR_ABS`` calls in C++ code have
been replaced with ``mozilla::Abs`` calls, in `bug
847480 <https://bugzilla.mozilla.org/show_bug.cgi?id=847480>`__. All new
code in ``Firefox/core/toolkit`` needs to ``#include "nsAlgorithm.h"`` and
use the ``NS_foo`` variants instead of ``PR_foo``, or
``#include "mozilla/MathAlgorithms.h"`` for ``mozilla::Abs``.

Use of SpiderMonkey rooting typedefs
------------------------------------
The rooting typedefs in ``js/public/TypeDecls.h``, such as ``HandleObject`` and
``RootedObject``, are deprecated both in and outside of SpiderMonkey. They will
eventually be removed and should not be used in new code.