* Add CP.52 and CP.53 guidelines
* Address some feedback on CP.52
* Fix PR break caused by code line longer than 100 characters in CP.53
* Fix spelling error and one line of trailing whitespace
* Tweaks to CP.53 based on feedback
* Clarify "concrete type" and make type category descriptions consistent (closes#1780)
* markdown fixup
Co-authored-by: Jonathan Wakely <cxx@kayari.org>
* Elaborated that we can't generally add comparison operators to C structs
Co-authored-by: Sergey Zubkov <cubbi@cubbi.com>
Co-authored-by: Jonathan Wakely <cxx@kayari.org>
* Minor comment fix
* Minor language fix
* Minor language fix
* Minor clarification
* Minor language fix
* Minor language fix
* Remove whitespace for clarity
* Make whitespace consistent (again, it wasn't the "bad" here, right?)
* Minor language fix
* Change weird formatting for emphasis
* Fix (what I believe is) a slightly distracting typo
* Minor language fix
* Suggest to highlight even security vulnerabilities as a possible outcome of concurrency errors
* Minor language fix
* Suggest to add new section for a seemingly unrelated note
* Minor language fix
* Minor language fix (not sure what this intended to say in the first place?)
* Minor language fix
* Add minor remark
* Minor language improvement
* Minor language fix
* Point out a bad example
* Minor fixes
* Minor language fix
* Add missing period (the lack of which doesn't look bad here, but does a bit in the rendered html)
* Minor language fix
* Minor language fix
* Minor typo fix (but is this what was meant?)
* Minor language improvement (?)
* Minor language fix
* Minor language fix (?)
* Add missing closing quotation mark
* Minor language fix
* Minor language fix
* Remove extra dot (or was it supposed to be three dots instead?)
* Minor language fix
* Minor language fix
* Minor language fix
* Minor language fix
* Minor language improvement
* Minor formatting improvements
* Minor improvements
* Minor language fix
* More fixing
* Add missing parentheses (did I get it right?)
* Minor language fix (?)
* Minor language fix
* Minor language fix
* Fix language (but is this what was meant in the first place?)
* Update heading to match content (in particular, the example)
* Remove superfluous whitespace
* Update also the "bad" example to use int*
* Add '<=>' to comparison operators related rules (C.87, C.167)
C++20's three-way comparison operator should respect the same rules as other
comparison operators.
* Fix a minor typo
Co-authored-by: Jonathan Wakely <github@kayari.org>
Co-authored-by: Jonathan Wakely <github@kayari.org>
This allows the example to remain simple well not misleading a beginner
such a comparison is safe. Including an epsilon comparison or something
similar would overly complicate this example.
Co-authored-by: Martin O'Shea <martin.oshea@native-instruments.com>
* Update C.83 with swap for resource mgmt.
The swap is useful to implement assignments idiomatically (e.g. copy-swap idiom).
With the current enforcement, to non-virtual classes, very simple classes (e.g. `trivially_copyable`, a struct encapsulating an stl container) are suggested to have a swap. This suggestion can be argued wrong since for very simple classes copy-swap idiom is not efficient (the creation of a third object on assignment is not needed, which copy-swap does).
* Rework C.83 according to PR comments.
The various guidelines now have consistent Enforcements.
All C-style casts are now consistently banned, including to `(void)`.
Cast to `(void`)` should be `std::ignore =` instead.
All functional-style casts are now consistently banned, instead of
`T(x)` use `T{x}`.
In English, the word "may" is overloaded and ambiguous. This commit
changes it to "might" wherever possible, otherwise more specific
meanings like "can," "could," or especially "must" ("may not" -> "must
not" when that is intended).
The examples in SF.12 are likely to encourage readers to always use the `""` form of `'#include` when including headers from the same project ([discussion](https://github.com/isocpp/CppCoreGuidelines/pull/1596#issuecomment-673266275)). However, in larger projects this may not always be appropriate; `<>` should be used for includes located via a header search path.
This proposed solution adds an example of the later, i.e. where `<>` is used to include a header from the same project.
- Add a cross-reference to C.139 and note that it doesn't matter whether
a function is declared with override or final if the whole class is
already final.
- Fix C.139 to make it clearer that it's about `final` on classes.
explicitly note that "file" refers to as it exists in the location of it being authored/modified.
this is to address any confusion about #including "bar.h" from foo.h based on whether bar.h is relative to foo.h at the time and location foo.h is being modified (e.g. under mylib/foo.h) versus at the time/location from which foo.h is installed to the system and included by the user (e.g. from /usr/include).
Part of SL.str.1 references replacing “const string*” with “string_view”, but the example code above shows “const string&” instead.
This changes the comment to the reference rather than pointer.
As a general rule, `operator()` should be const-qualified.
The exceptions are few and far between.
I was actually looking for a Guideline on that subject;
I grepped for `operator()` and found that not only is there
no such Guideline yet, the doc actually contained these
places that (needlessly) violated the general rule.
* Use '<thing> template' i.s.o. 'template <thing>'
The word 'template' is often wrongly used as an adjective to the 'thing'
it becomes when instantiated.
* Expand succinct formulation for readability
Co-authored-by: Kristoffel Pirard <kristoffel.pirard@vanhool.com>
The _bad_ example wasn't annotated as such and a perfectly fine function
name was marked bad.
Annotate the example as bad and remove the misleading function name
annotation.
For some reason the trailing `e = 3` seemed more of a red flag
than anything else about this line. Let's imply that the programmer
is trying to make some constants for hexadecimal translation.
The current guidance on SF.12 can be over-applied and devolves into "always use <>" because all compilers support adding include directories to the <> search. In this case, even the current directory may be added and so it is always possible to use <> for every header. Applying the guidance then devolves into an undesirable state where <> is always used and include"" is never used.
Instead, the proposed guidance leverages and encourages the distinction between <> and "" to create an easy-to-understand rule that the original guidance hints at and that most developers already follow and understand: "" is for local headers and <> is for library and external headers.
* Actually detect negative sizes by following ES.106
And don't use senseless one letter names
* fix grammar
Co-Authored-By: Johel Ernesto Guerrero Peña <johelegp@gmail.com>
Co-authored-by: Johel Ernesto Guerrero Peña <johelegp@gmail.com>
* add SF.12
* add incscope to isocpp.dic to get the CI build to pass
* expand INCLUDES, update dictionary for the ci build to pass
* pr feedback
* in the same directory
* update based on feedback
* 3rd try
* PR feedback
* update lable
* Update CppCoreGuidelines.md
* Update CppCoreGuidelines.md
Changed the two anchors back (anchors need to stay stable and don't
display visibly anyway)
Switched "non-public" back to hyphernated (to use hyphenation
consistently)
* Casting away const isn't undefined behavior, but modifying a constant is
You can cast away `const` as much as you like, as long as you never write to variable.
* rewording for clarification
When a reference in the Bibliography is available online, it would seem to be nice to have a link. In this example, the original article was published in C/C++ User's Journal, with the content later ported to the DDJ web site. The DDJ site is no longer reliable, but a good copy exists in archive.org, and I've linked to it here.
* make the sample in Sd-factory compileable (closes#1488)
- make the sample in Sd-factory compileable
- fixed wrong capitalization: create/Create -> create
- `make_shared` cannot access protected constructors, so made them public. To still have access protection introduced a protected `class Token` in each class. That token can only be created by the class itself (and derived classes) and needs to be passed to the constructor.
- changed order: `public` first, then `protected`
- same sample for C.50 and Sd-factory
- removed spurious "see Item 49.1" as it is unclear what this means
* line length
* tabs -> spaces
* spelling
* input from cubbimew
- added back in Item 49.1
- added link for items as suggested ("in [SuttAlex05](#SuttAlex05)")
* changed link to Item 49.1 to link to C.82
This PR affirms that all virtual functions, *including destructors*,
should be declared exactly one of `virtual`, `override`, or `final`, and
takesa pass through the document to make the examples and guidance
consistent with that.
Of course a virtual destructor is a virtual function: It behaves
polymorphically, and it has a vtable entry that can be overwritten ==
overridden in a derived class exactly the same as any other derived
virtual override. See also [class.virtual]/7: "Even though destructors
are not inherited, a destructor in a derived class overrides a base
class destructor declared virtual; see [class.dtor] and [class.free]."
However, the following exception text currently appears in C.128:
> If a base class destructor is declared `virtual`, one should avoid
declaring derived class destructors `virtual` or `override`. Some code
base and tools might insist on `override` for destructors, but that is
not the recommendation of these guidelines.
... but this exception is (a) not well-founded, and (b) inconsistent
with the Guidelines' practice in other examples and with the rationale a
few lines earlier for C.128 itself.
Re (a):
- The exception is overly broad: The rationale given for this exception
is entirely against marking destructors `override` (not `virtual`). So
clearly the exception to write neither keyword is too broad: At most,
the exception should be to write `virtual` rather than `override`.
- Explicit `virtual` is primarily for class users, not class authors:
The arguments given in #721 favoring this exception are from the
viewpoint of the implementation of the function (even then, the
arguments are debatable and debated). But `virtual`, `override`, and
`final` are primarily for the far larger audience of *class users and
call sites* of the function, for whom of course we should document each
declared function that is polymorphic, *especially* the destructor --
this tells calling code whether the function is safe to call through a
(smart or built-in) pointer or reference to base, which will nearly
always be the case for such types. We should not make the reader of the
code go way to look in the base classes to figure out whether a function
declared in this class is virtual or not -- the reason this Item exists
is primarily to avoid that implicit virtual antipattern via convention
and automated enforcement. For class users, all virtual functions
including destructors are equally polymorphic.
Re (b): The Guidelines already don't follow this. For instance, two
Items later (in C.130) we have this example that correctly uses
`override`:
~~~
virtual ~D() override;
~~~
... though per C.128 it should not also specify `virtual` (also fixed in
this PR).
Finally, the exception also contradicts the rationale given earlier in
the same Item.
- typo "a" -> "as"
- added "???" to mark incomplete sentence
- typo "than" -> "that"
- "scanf using s" -> "scanf using %s" (same as for printf)
- added missing comma
* ES section, different stuff
- ES.26: same capitalization for all function names in example
- ES.34: fix wrong formatting (first line of example was formatted as text)
- ES.46: corrected value in comment (new value read out in debugger)
- ES.46: Capitalize Enforcement bullet points (as in other ES rules)
- ES.65: fix formatting of code after list (compare https://meta.stackexchange.com/a/34325/172717)
* review-feedback from jwakely
and:
- ES.46/ES.47: added period at end of sentence
Alexey Dmitriev
Herb Sutter
bgloyer
David Machaj
ash
Sergey Zubkov
Gustav Jansson
Alessandro Gentilini
Marco Gartmann
Juha Reunanen
peno2
5chmidti
Bill Rose
Kie
nullptr-cpp
Martin
JuliusBrueggemann
syntroniks
shaneasd
Florian Behrens
Niels Dekker
Fernando J. Iglesias García
David B
rjhcnf
John McFarlane
LE GARREC Vincent
cadenzasong
Gabriel Dos Reis
apenn-msft
Hugh Macdonald
al-mission-2016
Arthur O'Dwyer
Manlio Morini
Anthony Cassidy
xtofl
rico-chet
Hyuk Myeong
Moritz 'Morty' Strübe
Dale Visser
Yaro Shkvorets
Akash Mozumdar
Sylwester Fraczek
Adnn
Chris Guzak
roroberto
kuzkry
Kim Viggedal
Mark Nelson
Baptiste Thémine
Bernd Ritter
Bjarne Stroustrup
Zachary Henkel
johansin
Taewon Park
Tom Lankhorst
Elviss Strazdins
Mike B
Simon Daester
Jonathan Wakely
beinhaerter
Werner Henze
Tyler Young