Stale TODO/Proto-rules

[cubbimew]

A number of entries in the To-do: Unclassified proto-rules are addressed in the main text:

• "Avoid implicit conversions"
  addressed by C.164: Avoid conversion operators and C.46: By default, declare single-argument constructors explicit
• "Always initialize variables, use initialization lists for member variables."
  addressed by ES.20: Always initialize an object and C.49: Prefer initialization to assignment in constructors
• "Anyone writing a public interface which takes or returns void* should have their toes set on fire"
  addressed by I.4: Make interfaces precisely and strongly typed
• "Use const-ness wherever possible: member functions, variables and (yippee) const_iterators"
  addressed by Con.1: By default, make objects immutable, Con.2: By default, make member functions const and the rest of the Con section, although const_iterators probably need a new Con rule.
• "Use auto"
  addressed by ES.11: Use auto to avoid redundant repetition of type names
• "(size) vs. {initializers} vs. {Extent{size}}"
  would be addressed by C.105: Give a constructor and Extent constructor when it's written (a stub rule is better than a line in the TODO list)
• "Never pass a pointer down the call stack"
  possibly addressed by F.43: Never (directly or indirectly) return a pointer or a reference to a local object? A little unclear given that lifetime safety is missing.
• "should virtual calls be banned from ctors/dtors in your guidelines?"
  addressed by C.82: Don't call virtual functions in constructors and destructors
• "Use RAII lock guards"
  addressed by CP.20: Use RAII, never plain lock()/unlock()
• "Join your threads ... could support library provide a RAII wrapper for std::thread"
  addressed by CP.25: Prefer gsl::raii_thread over std::thread unless you plan to detach()
• If two or more mutexes must be acquired at the same time, use std::lock
  addressed by CP.21: Use std::lock() to acquire multiple mutexes
• "When using a condition_variable, always protect the condition by a mutex"
  is one of the items already in issue CP guidelines for condition variables #554
• "should non-virtual interface be promoted"
  is the subject of issue rule proposal: C.141 Prefer to make virtual functions private. #768 with pr dropping NVI from proto-rules due to no consensus on #768 #777

I believe all these should be dropped and the rest converted to open github issues (if it's unclear whether a rule should exist) or stub rules (if it's clear the rule should exist, but the details are not ready).

[AndrewPardoe]

Sergey, we agree that this set of suggestions is up to your usual standard of quality. Could you please make these edits?

[hsutter]

I'd suggest these (small) ones for .8:

• flag virtual calls in ctors/dtors
• don't use bind, use lambdas instead (yes, you can move-capture -- there's no capture by && but there is [x = std::move(x)])
• use RAII lock guards, never call .lock/.unlock directly

I'd suggest these for either .8 or post-.8:

• avoid implicit conversions
• const => thread-safe
• avoid void* parameters and return values
• use const wherever possible
• use auto (note in C++17, we can remove the "almost" from "almost always auto"...)
• static vs dynamic polymorphism
• avoid private inheritance, prefer private membership
• don't use recursive mutexes -- refactor to avoid the need for a nested transaction, or if you can't then at least pass a unique_lock instead to show that you did lock
• never use atomic_compare_exchange_strong<UDT> -- NOTE that because you should use _strong when doing a single test (if) and _weak for a loop which retries anyway, this also argues that you should not use atomic_compare_exchange_***<UDT> except in a retry loop (do not use it with a single if test)
• individual shared_ptr and shared_future objects should be synchronized like any old object if they are shared

We aim to cover these via lifetime rules:

• leaks from temporaries
• pointer/iterator invalidation

I'd suggest abandoning the following:

• long-distance friendship
• physical design (this is changing with modules anyway)
• using directives in global scope (these are okay in .cpp files after the last #include)
• namespace granularity
• inline namespaces (unless we have guidance? I don't)
• never pass a pointer down the call stack (why not? we should recommend passing pointers rather than smart pointers)

[cubbimew]

There are currently 35 entries in the To-do section.

Dropping 5 out of 6 suggested to be dropped by Herb (using directives is already covered by SF.6), dropping the 2 entries covered by lifetime rules, and dropping 11 of the issues already covered by existing rules/issues as noted in the first comment, I believe we're actually down to these 14

• Namespaces
  already in as a stub rule SF.20
• Const member functions should be thread safe
• Don't overabstract
• falling through a function bottom
• static vs dynamic polymorphism
• lambdas vs classes
  this one is actually already stated in F.50 fairly explicitly
• prefer lambdas to std::bind
• LSP
• private inheritance vs/and membership
• avoid static class members variables (race conditions, almost-global variables)
  this one is actually already mentioned in CP.2's enforcement section
• don't use recursive mutexes
• don't use atomic_compare_exchange_strong<UDT>
• shared_future and shared_ptr objects should be synchronized if shared
• rules for arithmetic
  these are now covered ES.100 - ES.107

Of these (minus 4 that appear to be covered), the following 7 seem concrete enough to make rules:

• Const member functions should be thread safe
• falling through a function bottom
• prefer lambdas to std::bind
• don't use recursive mutexes
• don't use atomic_compare_exchange_strong<UDT>
• shared_future and shared_ptr objects should be synchronized if shared
• private inheritance vs/and membership

and the following 3 seem philosophical and non-enforceable

• Don't overabstract
• static vs dynamic polymorphism
• LSP

shall I go ahead with creating 10 github issues for these and dropping the To-do section entirely?