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