F.15 is an omnibus item that covers the guidance we want to pursue. There is overlap/redundancy with related material in F.20-25, and F.40. These need to be integrated.
The obvious options are "keep F.15 and remove others that are redundant" or "distribute F.15 material into the others." But I think a better option is: Keep F.15 and have just one item each on how to pass "in", "inout", and "out" parameters. I'll go with that if there's no objection.

There is already at least one implementation at https://github.com/Microsoft/GSL and it might be good (essential?) that people understand what they are getting when they adopt of of these implementations. If we don't really expect multiple implementations, why not just include the implementation in this repository so that changes to the specification and the implementation can happen simultaneously?

Example, bad: Consider a famous security bug

 void f() // problematic
 {
  char buffer[MAX];
  // ...
  memset(buffer,0,MAX);
 }

There was no postcondition stating that the buffer should be cleared and the optimizer eliminated the apparently redundant memset() call:

 void f() // better
 {
  char buffer[MAX];
  // ...
  memset(buffer,0,MAX);
  Ensures(buffer[0]==0);
 }

This implies that the second version averts the security bug. But this isn't true: the compiler is free to note that buffer[0]==0 should always be true at the point, replace the argument with a constant true, then make the same elimination of the memset call. And in fact, both GCC and Clang do so. The only correct way to avoid the security bug is to use a special-purpose function like memset_s (or maybe volatile).

Hello, my name is Chris Ohk, administrator of Facebook C++ Korea community (http://www.facebook.com/groups/cppkorea).
This is an awesome document that I'd love to read. C++ Korea users, too. However, they have a hard time reading because it is not Korean, but English. So, as a suggestion, I want to translate from English into Korean. Can you ask for permission? That would make it really easy for Korean people to read a nicely formatted version. Thanks!

Having the contents of the guidelines in a single Markdown file seems an odd decision. Why not split each section into a separate Markdown file? By doing so, you'd make the merging of pull requests much easier.

Also, has the Foundation considered trying to use GitHub Pages for presenting the material?

Although I do understand reasoning behind using template alias with the 'owner' (no semantic code changes and might be used by external tools), my concern is that it actually limits the availability to write a generic code using it. What I mean by that, is that it's not possible to distinguish it from not owning, raw, pointer.

Example:

auto f(auto object) {
     static_assert(is_owned(object));
}

{
T* ptr = nullptr;
f(ptr); // is_owned=false
}

{
owner<T*> ptr = nulltpr;
f(ptr); // is_owned=true? // the same for smart pointers
}

Question:
Is there any way to implement is_owned? since it's just an alias for raw pointer it seems not possible? Following question (if the previous answer is yes - it's not possible), any thoughts of making detection of is_owned possible in the future?

Node* find(Node* t, const string& s)    // find s in a binary tree of Nodes
{
    if (t->name==s) return t;
    if (auto p = find(t->left,s)) return p;
    if (auto p = find(t->right,s)) return p;
    return nullptr;
}

There's no termination in this recursion besides t->name == s, so nullptr can never be returned. An initial if(!t) return nullptr; is required.

void increment2(array_view<int> p)
{
    for (int x : p) ++x;
}

I assume that this should be:

void increment2(array_view<int> p)
{
    for (int& x : p) ++x;
}

because otherwise we're incrementing and immediately throw away the result without any side effects, which would contradict the earlier example.

Before the cutover to this repo, the "type" and "bounds" safety profiles (sets of rules) were maintained in separate files. These need to be merged into the main doc. Creating this issue to track that todo item.

The documentation gives the following reason for zstring's existence:

C-style strings are ubiquitous. They are defined by convention: zero-terminated arrays of characters. Functions are inconsistent in their use of nullptr and we must be more explicit.

Is this correct? It seems to me that the utility of zstring is in differentiating between pointers-to-char and strings, and doesn't really have anything to do with nullptr. Sure, not_null can be used with zstring, but that is just the combination of two different features.

The second (related) question I have about zstring is whether it is meant to be used just to annotate legacy "C-style" APIs, or whether it is meant to be used in new code. I ask, because in my opinion zstring should never be null. My rationale is that zstring is conceptually a string, not a pointer; pointers may be null, strings may not (if you disagree, ask yourself why std::string cannot be null). The only reason that some APIs allow zstring to be null is that the authors have been fooled by its pointer-esque appearance (char const*). If you need an optional string parameter, do what you do with every other type: use a pointer:

void f(zstring* s); // void f(char** s);

Of course, legacy code can and does allow zstring to be null, so if it is meant to be used in said legacy code, it must be possible to assign zstring a null value. However, if you agree with my analysis above, it seems a little unwise to encourage the use of this legacy-compatible zstring in new code. Thus, regarding this excerpt from the guidelines:

Use not_null for C-style strings that cannot be nullptr. ??? Do we need a name for not_null? or is its ugliness a feature?

I think the answer is yes, given that not_null<zstring> is (IMHO) the correct usage. In fact, in order to encourage correct use of the types, I would probably rename zstring to something like nullable_zstring, and then have zstring as an alias for not_null<nullable_zstring> (a bit of a double negative, but it does the job!)

Let me know if I'm horribly wide of the mark here.

inerface --> interface
efford --> effort
rage --> range

Edit: pull request submitted.

There are many links to undefined anchors. I sent fixes (PR #84) for some easy parts of them. To fix remaining I think some guidance (or action) from the author(s) are required.

The remaining (the result of script shown in PR #84, after the fixes are applied) are:

(empty)

Sd-noexcept

Rcon-empty
Rcon-init
Rcon-move
rcon-ptr
Rcon-stl
Rcon-val
Ref-conceptsTS
Rf-local-ref-ref
Rper-checking
Rr-smartptrconcepts
Rt-???
Rt-cast
Rt-specialization2
S-???
Sd-???
Sd-noexcept

I couldn't figure out fixes for them because:

• Some of them seems to be intentional placeholders:

  • "Rcon-*" (including "rcon-ptr")
  • "Rr-smartptrconcepts"
  • "Rt-???"
  • "S-???"
  • "Sd-???"

  I don't know whether it is OK or not to keep such placeholders as dead links. I think dead links are bad, and the link (not the text) should be removed, or the anchor should be defined somewhere with a note like "to be written" or something.

• "Ref-conceptsTS" is surely referring to ISO C++ Concepts TS. But the use of the link (anchor name) is at only one place among some more places referring the same TS. Also, there is "References" section. I think It is better to be fixed with a document wide policy about how to link to TS or the Standard.

• I couldn't be sure of the intended target of some links:

  • "Sd-noexcept" (including "#Sd-noexcept"):
    "Sd-never-fail" seems the most relevant discussion. But I think it is possible to be a placeholder to future discussion or an oversight after removal of old discussion.
  • "Rf-local-ref-ref":
    The link is set to the text "F54". But there is no rule with such number. There are similar anchor; "Rf-return-ref-ref" and "Rf-pass-ref-ref". But "Rf-local-ref-ref" is used beside them, so they seem distinct rules. If it is a removed rule, entire "See also F54." should be removed.
  • "Rper-checking":
    There is no such rule in "Rper-*" that is saying something like the link text: "move checking outside the critical part of the code".

• "Rt-specialization2", "Rt-cast"
  They seems to be oversight of removal of corresponding rules. In that case, the link (with text) should be just removed from the summary. But I think it is possible that the rule is just not written yet.

• [The link with empty anchor ("example of such overloading")](https://github.com/isocpp/CppCoreGuidelines/blame/245f82acd58eecf6b92cc51393fe32e114dc7b90/CppCoreGuidelines.md#L2222) seems like an old placeholder when the example was not there. In that case, I think the link (with text) should be removed.

The solution to the first example is:

int area(int height, int width)
{
    auto res = height*width;
    Ensures(res>0);
    return res;
}

When the multiplication overflows, undefined behavior is invoked. This makes the Ensures(res>0) depend on undefined behavior.

I'm not sure what the correct solution should be like. Possibly something like clang's and gcc's __builtin_sadd_overflow or one of the techniques described in the book Hacker's Delight. The former would be clang/gcc-specific, the latter quite involved.

Neat paper and I've just started reading it. Page 4, section "Cyclone: Regions" says "where a pointer can be associated with a region to ensure the pointer outlives the region it points into." Shouldn't this be "never outlives"?

Scott Meyers in Effective STL Item 3 "Use const whenever possible" in the subsection "Avoid Duplication in const and Non-const Member Functions" mentions it is safe (and even recommended practice) to use const_cast to help avoid duplication in const and non-const overloads.

This is also discussed in this StackOverflow question: https://stackoverflow.com/questions/123758

There is a following example in P1:

auto p = find(v,val);               // better

I can't find any mention of that even with c++17 context (although I would love to have that). Should that be replaced with

auto p = find(v.begin(), v.end(), val);

or that not a typo and intended?

Beman Dawes pointed out that there is potential for confusion between the string_view in the GSL and the existing string_view in the TS. The main issue is that the former allows mutation whereas the latter does not.

While we plan to discuss aligning these types at the upcoming ISO C++ meeting in October, we should consider a different name such as string_ref.

int m3 = min(-1,v);             // run-time evaluation
constexpr int m4 = min(-1,2);   // error: connot evaluate at compile-time

I take it that second line should read

constexpr int m4 = min(-1,v);   // error: connot evaluate at compile-time

Markdown has a number of dialects to it, and it seems that the .md files in this archive are built around the specific extensions made by GitHub. That's fine, but there should be a statement somewhere that these files use GitHub's Markdown dialect.

I mention this because I tried to format it with Pandoc, and the headers didn't come out well at all. I'd prefer that some formatting work be done to make it work, but I recognize that supporting different Markdown formatters is probably not on the top of your priority list.

With Microsoft's compiler the annotation extension __declspec(dllexport) can be used and with the GCC compiler (and many other compilers) the annotation extension __attribute__ ((visibility ("default")) (if the compiler option -fvisibility is set to hidden) it is possible to explicitly mark which symbols should be exported from a shared library. Such marked symbols could be checked to see if they are primitive and C like. Note that C++ and its vague linkage entities have weird interactions with the -fvisibility option though. Note also that generalized attribute syntax can be used for GCC's visibility's annotation syntax at least I think.

Used the public before -> Use the public before
srandomly -> randomly
botton up -> buttom up

What about exceptions? This is sometimes an enigma for the correctly use in C++ (like in constructors / destructors with using inheritance). In this topic I see much code with memory leaks or runtime errors.

I believe we need a guideline for accepting function object parameters in functions that model some form of control flow, and particularly in functions modeling loops of some kind.

Let's start with the example currently in F.24:

template <class F, class... Args>
inline auto invoke(F&& f, Args&&... args) {
    return forward<F>(f)(forward<Args>(args)...);
}

Now let's modify it slightly to bring the issue out:

template <class F, class... Args>
inline void invoke_n(int n, F&& f, Args&&... args) {
    for (int i = 0; i < n; ++i) {
        forward<F>(f)(forward<Args>(args)...);
    }
}

I come across situations like this all the time where I am trying to model some form of loop as a higher order function.

The question is: how should functions like invoke_n accept the function object parameter (f in this case)?

We can rule out a few options right away. invoke_n can't accept f as any form of l-value reference or as any form of const reference. A non-const l-value reference would prevent temporary objects from being passed as the function argument, which is particularly bad for lambdas. Any const reference would prevent mutable function objects from being passed, which is also undesirable.

That leaves us with two options: to accept f by forwarding reference, as in the example above, or to accept f by value. Both of these have possible issues:

By forwarding reference

template <class F, class... Args>
inline void invoke_n(int n, F&& f, Args&&... args) {
    for (int i = 0; i < n; ++i) {
        forward<F>(f)(forward<Args>(args)...);
    }
}

This has a number of advantages:

1. It accepts any form of function object exactly as we would expect it to.
2. It feels right conceptually: all we're trying to do is just "inject" some code into a different context. This does that as purely as possible.
3. It always forwards its forwarding reference as recommended in the guidelines, so it's a consistent rule to follow.

It also has a possible very odd disadvantage: it forwards its function object (and indeed, its parameter pack) multiple times. This feels really dirty, since it means that we will be (possibly) using a single object as an r-value over and over again. This is potentially problematic since we can ref qualify functions and the writer of a function object class may not expect an r-value qualified member function to be called repeatedly. This is likely not an issue for lambdas, but could be for other function objects. Indeed, one could see extending this issue to simply ask about forwarding any kind of object multiple times (one could envision a function like make_n that constructs n copies of some type and returns them in a vector, for example; indeed, this very example forwards its parameter pack multiple times, which could get hairy).

There's another option, of course:

template <class F, class... Args>
inline void invoke_n(int n, F&& f, Args&&... args) {
    for (int i = 0; i < n; ++i) {
        f(forward<Args>(args)...);
    }
}

This avoids the problem of forwarding f multiple times (though it still forwards its parameter pack multiple times). On the other hand, it treats a possible r-value reference like an l-value and it creates a corner case for an otherwise consistent rule: here we are not forwarding a forwarding reference. This also feels gross.

By value

template <class F, class... Args>
inline void invoke_n(int n, F f, Args&&... args) {
    for (int i = 0; i < n; ++i) {
        f(forward<Args>(args)...);
    }
}

This avoids all the reference issues above, but has two major downsides:

1. It relies heavily on the optimizer's inlining ability to elide copies. While perhaps a reasonable expectation, it doesn't feel great to copy a function object just to model a loop.
2. It won't work with any function object that wants to mutate itself and allow the caller to read those results. That's not an issue with lambdas, but it would be with a good deal of plausible hand-rolled function objects.

In short, I'm stumped as to how to handle this case. I guess my knee jerk reaction is to simply forward every forwarding reference every time, but I can see that causing problems. Some guidance on this issue would be greatly appreciated, and it may even need to be extended to guidance for more than just function objects; forwarding the parameter pack in this example is equally problematic. It's the function object that I find most interesting, however.

Item P.6 is misleading: it encourages use of vector and array_view templates in the interface functions, but completely ignores ABI and compatibility issues.

extern void f4(vector<int>&);       // separately compiled, possibly dynamically loaded
extern void f4(array_view<int>);    // separately compiled, possibly dynamically loaded

I think it has to at least briefly mention that these templates may not be implemented in the same way, and most likely than not "possibly dynamically loaded" wouldn't work as expected.

The CP.1 rule needs context, namely that it applies most sharply to code that is intended to be shared (such as libraries) and less urgently to things like standalone applications.

This rule also needs an example, and needs alternatives for fixing that example.

A prototype may be found here: https://github.com/paulmckrcu/CppCoreGuidelines at commit 4462a0462d7f8ee4cc04a321190dbbdfd35db2f0 .

The (in)famous factorial example in rule F.4 has a small bug.

Instead of:

Expects(0<=x && x<max_exp);

it should be

Expects(0<=n && n<max_exp);

to check the input parameter.

Thank you for writing these guidelines. Helping people write robust code is great.

I am against the rule ES.20 -- Always initialize variable

I sometimes write code that has some bug in it. By mistake of course. Then, if this bug is detected by a tool I use there is not much harm done. But preventing that it is detected sounds bad to me.

Initializing variables does ensure that there are not uninitialized variables. However it is well defined that it is a bug to use uninitialized variables. Therefore static analysis tools and dynamic analysis tools look for this bug. No combination of tools is perfect as far as I know but imho most of these bugs are found using such tools.

Initializing variables prevents that tools can discover subtle bugs. For instance:

int i, j;
if (a) i = j = 1;
else  j = 2;  // <- forgot to assign i in this else
return i+j;

If i and j are initialized. then it is not well defined if there is a bug or not. and then tools can't warn about that code.

Subtle bugs are very common. So preventing that subtle bugs are found is a big mistake.

For information I am a open source static analysis tool developer.

The guidelines provide conflicting advice for implementing a clone() method.

C.67 states: "Don't let this tempt you into returning an owning raw pointer; this is a minor drawback compared to the major robustness benefit delivered by the owning smart pointer."

C.130 gives an example in which the clone() method does, indeed, return a raw pointer, and notes: "Note that because of language rules, the covariant return type cannot be a smart pointer."

I might suggest the following as an initial straw man for how to allow cloning of a derived class with a smart pointer. This has some drawbacks, perhaps some of which I am not aware, but I'm hoping that some variation of this technique might be suggested as an alternative:

class base {
public:
    inline std::unique_ptr<base> clone()
        { return do_clone(); }

private:
    virtual std::unique_ptr<base> do_clone() =0;
};


class derived : public base {
public:
    inline std::unique_ptr<derived> clone()
        { return std::make_unique<derived>(); }

private:
    virtual std::unique_ptr<base> do_clone();
};

///...

std::unique_ptr<base> derived::do_clone()
{
    return std::make_unique<derived>();
}

Unless I'm misreading things, the comments on the example in F.19:

int length(zstring p);  // it is the caller's job to make sure p!=nullptr

int length(not_null<Zstring> p);    // the implementor of length() must assume that p==nullptr is possible

...are swapped. At least as I'd normally read things, a not_null<Zstring> allows the imlementor to assume that p!=nullptr, and a simple zstring parameter requires him to assume that p==nulltpr is possible (In fact, I'd assume that not_null's ctor would throw when/if passed a nullptr).

Currently Microsoft is providing one implementation that is, to the best of our knowledge, completely portable, ISO-standard C++. It is under MIT license. We plan to improve the documentation and eventually provide a proper specification as part of the guidelines.

make_final_act (or make_final) would be more consistent with other std construction helpers such as make_unique.

Additionally there's danger of finally accidentally being called without assigning into a temporary, thus inducing the destructor to activate immediately. The static analysis could catch this, or renaming makes this sort of mistake less likely to happen imo.

This example is used in ES.60--no naked new --and ES.61-- delete[] / delete .

The text in E.61 is a bit confusing. I propose changing the text to read

"Note: This example not only violates the no naked new rule, it has many other problems."

or

"Note: This example not only violates the no naked new rule as in the previous example, it has many other problems."

We should of course preserve the link if we preserve the text about naked new

The above quoted information is incorrect as I understand the standard. That case will always be an initializer list, even in C++17.

What's changed is direct-list-initialization: auto x{1}; will result in x being an int in C++17, but was an initializer_list previously.

carricature
consructor
evenry
protability
opinios
srandomly
.. etc

I find it increasingly hard to get compilers to digest im using a counter. And what if i want a meta engine that pops out near xeroxes of the program, or what have you and wish to change buffer sizes to protect goods inside?

In its enforcement,
C.146: Use dynamic_cast where class hierarchy navigation is unavoidable
states
"Enforcement: Flag all uses of static_cast for downcasts, including C-style casts that perform a static_cast."

What about static downcasts that are on the scale between almost-always-safe to guaranteed-to-be-safe?
I'm specifically talking about CRTP base templates. The enforcement of this rule is a false positive
there.

The Core Guidelines advises e.g. using ranged for instead of for, for instead of while, and specialized ranged algorithms instead of either.

It also advises to stagger template definitions, avoid templates unless necessary, etc.

These and others are part of what I call the Rule of Minimum Power:

"Use the least amount of language power that gets the task done."

Under this specific strategic guideline fall a number of the concrete checkable guidelines. Then programmers seeing e.g.:

file.cpp(475): Rule of Minimum Power: use ranged for instead of explicit for loop.

will immediately understand the high-level motivation.

The another reason is exception-safety:

// Not exception-safe: T2() may throw after T1() was constructed
// and before unique_ptr<T1> was constructed.
f(unique_ptr<T1>(new T1()), unique_ptr<T2>(new T2())); 

// Exception-safe: calls to functions are not interleaved.
f(make_unique<T1>(), make_unique<T2>()); 

Herb's article on the topic: http://herbsutter.com/gotw/_102/

This is an awesome document that I'd love to print out and read offline, but GitHub doesn't provide a nice way to print markdown docs (you basically get what's rendered on the page and there's a ton of wasted space & bad formatting). As a suggestion, when you reach certain milestones with the doc can you render the contents to a PDF (perhaps something like this would work? http://www.markdowntopdf.com/) and create a new release on GitHub with the PDFs? That would make it really easy for people to grab a nicely formatted version for printing. Thanks!

C.ctor: Constructors, assignments, and destructors

a default constructor: X()
a copy constructor: X(const X&)
a copy assignment: operator=(const X&)
a move constructor: X(X&&)
a a move assignment: operator=(X&&) <<<<<<<<<<<
a destructor: ~X()

What about dependency manager? This is a much more important part of the infrastructure of the language than string_view or array_view.

A branch of the form if(a || b) {…} is treated as if(a){ if(b) {} else{…}}

should be

A branch of the form if(a || b) {…} is treated as if(a){..}else{ if(b) {..} }

The example in F.41 reads:

int f( const string& input, /*output only*/ string& output_data ) { // BAD: output-only parameter documented in a comment
    // ...
    output_data = something();
    return status;
}

tuple<int,string> f( const string& input ) { // GOOD: self-documenting
    // ...
    return make_tuple(something(), status);
}

In fact the BAD version may as well be good and the GOOD one, bad. Consider:

for (auto&& item : many_items) {
    auto result = f(item);
    ...
}

This is ouchworthy because it creates and destroys a string compulsively with every iteration. In contrast, the version deemed bad fares quite a bit better:

string data;
for (auto&& item : many_items) {
    auto result = f(item, data);
    ...
}

In this case memory is (re)allocated only when the current data does not have enough capacity. Howard Hinnant, Eric Niebler are good to talk to for this kind of stuff.

Defining terms like STL.

The glossary at the beginning of the document defines "STL" as "Standard Library". Do we want to give the abbreviation this new meaning? Will there be danger of confusion with Stepanov's historic Standard Template Library?

Suggestion: make use of syntax-highlighting available in GitHub-flavored Markdown.

Example current view:

Example with syntax highlighting:

Note: many of the current code examples use indentation, rather than fenced code block syntax, to trigger monospace rendering. Those examples would need to be converted to fenced code blocks to get syntax highlighting.

Happy to open a PR if this is desired.

There is already a Secure Coding Standard which is activly maintained as a community project at
https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=637

It would be bad if there would be two contradicting standards/guidelines (which inevitably will occur if not everyone involved knows about both projects). There should be a clear statement and before that a decision wether those rules are

• repeated here
• referenced here
• only stated/edited there
• or both guidelines should be merged
• or security related stuff will be handled regardless of the existing "standard" here

Should maybe be transformed in a wiki page after initial discussion.

I appreciate your effort.

Please label it "No dangling resources" instead of "no dangling pointers".
I also would like to appreciate some more attention to exception handling for failing system calls,
like open, write, read, mmap.
This is because of many people still think, that exception handling is mostly intended for out-of-bounds-errors and people are fine with ignoring system call errors.
Eventually the notion

• resource wrapper (system call creating a resource)
• functional wrapper (system call not creating a resource, like write)

which wrap system calls into C++, throwing an exception in case of failure and at the same time exploit RAII (not only for memory).

double funct1_tau(double val, int flag1, int flag2)
// ...
return func1_tau(-val, flag2);  // handled by func1_tau: flag1 = -flag1;

Looks incorrect to me.

F.41 recommends returning a tuple and accepting a tie:

tie( iter, success ) = myset.insert( “Hello” );

Does this prevent RVO?