isocpp / CppCoreGuidelines

The C++ Core Guidelines are a set of tried-and-true guidelines, rules, and best practices about coding in C++
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
Other
42.59k stars 5.43k forks source link

Suggestion for rule about function try blocks of constructors/destructors #1196

Open tcorbat opened 6 years ago

tcorbat commented 6 years ago

I have a suggestion for an additional rule regarding function try blocks for constructors/destructors. As [except.handling]/14 [1] states that if control flow reaches the end of a handler of a constructor’s/destructor’s function try block, the handled exception is rethrown. To me (and I think for others it might be too) this case is surprising and therefore I suggest to always state this behavior explicitly (at least in the case mentioned), i.e. insert a throw; or return statement at the end of all handlers of constructor’s/destructor’s function try block. I'm curious whether others consider this as an issue too.

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/n4741.pdf

Possible rule:

E.XX: Explicitly throw at end of constructor’s/destructor’s function try block

Reason:

If controlflow reaches the end of a constructor's/destructor’s handler of a function try block the handled exception will be implicitly rethrown. The behavior is different in any other catch handler. Therefore, the desired behavior should be stated explicitly in this case.

Example, bad:

struct Resource {
    Resource() noexcept(false) try
    {
        allocationThatMightThrow();
    }
    catch (const AllocationException & e)
    {
        doSomeCleanup(e);
    }
};

Example:

struct Resource {
    Resource() noexcept(false) try
    {
        allocationThatMightThrow();
    }
    catch (const AllocationException& e)
    {
        doSomeCleanup(e);
        throw;
    }
};

Enforcement:

Flag catch handlers of a constructor’s function try block without explicit throw statement. Flag catch handlers of a destructor’s function try block without explicit return or throw statement.

jwakely commented 6 years ago

The example isn't a good use of a function try block. I think "don't use function try blocks" is a better guideline that applies more often.

beinhaerter commented 6 years ago

I disagree with the proposal as written now because except.handle 13 says

If a return statement (9.6.3) appears in a handler of the function-try-block of a constructor, the program is ill-formed.

This contradicts your rule Explicitly return or throw at end of constructor’s/destructor’s function try block.

tcorbat commented 6 years ago

Oh dear, how could I miss [except.handle]/13. This makes the return part completely wrong indeed.

Suggesting to avoid function try blocks (at least for constructors/destructors) seems to be reasonable for me too.

jwakely commented 6 years ago

Suggesting to avoid function try blocks (at least for constructors/destructors)

Avoid them everywhere.

cubbimew commented 6 years ago

Avoid them everywhere.

I was going to say "what about converting and rethrowing exceptions", but then I never had to do that myself.. So I just ran Debian Code Search for \)\s*try\s*: (not perfect since it won't catch ones with line breaks) and it found 243 uses in 30 packages, Almost all of them were compiler/IDE/other C++ parser test suites or that regex latching on to something that isn't C++.

I only found three uses in the entire Debian:

  1. MongoDB where they convert an exception in a constructor to std::terminate (because they can't handle those exceptions)
  2. Something called performous where they used it to convert standard exceptions from member initializers to their own SongParserException.... until earlier this year when they removed the function-try-block.
  3. Something called nordugrid-arc which doesn't seem to realize the exception is always rethrown and attempts to create an invalid object by executing valid = false; in the catch clause.

+1 for "Avoid them everywhere".

arvidn commented 6 years ago

Although I recognise most functions shouldn't have function-try blocks, the suggestion to avoid it, is it because:

  1. the need for a function-try block indicates some design flaw that should be fixed instead.

or

  1. the syntax is unfamiliar to people, so the more verbose version should be used instead.

If (2), I suppose this would apply to putting try-catch on a for-, while- and if-block as well. I do use try-blocks as for-loop bodies sometimes, if they handle events that are allowed to fail independently, without preventing all subsequent events from being executed.

I have a few example of cases where function try-blocks are convenient. For instance (because they save an indentation level):

  1. A function is best-effort and all errors are just logged, not reported up the stack
  2. Some specific kinds of errors are recorded by some other mean without interrupting the program flow of the caller. example1, example2. Perhaps the most compelling case for this is boost.asio handlers. The majority of errors that happens inside them should not be propagated up into the asio executor, but handled some other way. For instance by having a handler allocator that catches exceptions and lets the underlying object handle the error, (example).
  3. A function whose only purpose is to handle the current exception (i.e. re-throw and implement the catch handlers). example
  4. The main() function of a utility where any error propagated to that level should print an error and return a non-zero status code. example

@cubbimew I suppose my code didn't show up in your code search.

ChrisGuzak commented 6 years ago

Windows uses function try on ABI boundaries with the main reason being concision. I like @arvidn points and think the main() example is illustrative (and I have written that myself many times). if guidelines say never I need to learn more about what I'm getting wrong.

gdr-at-ms commented 6 years ago

We will clarify the existing rules about function-try block for constructors/destructor.

jwakely commented 6 years ago

@arvidn

If (2), I suppose this would apply to putting try-catch on a for-, while- and if-block as well.

No, not at all. A try-catch block is a statement, so I've never met anybody who is surprised that you can use it as the body of a for or while loop. It's no different to a single statement without braces:

while (cond)
  ++i; /* a single statement

while (cond)
  { /* a block statement */; }

while (cond)
  try { } catch (...) { };

But a function try block is unique syntax, allowing try to appear outside a function body. You can't do that with anything else:

void func() while (cond) { }

void func(int i) ++i; { }

It's a completely false analogy to say that if people aren't familiar with function try blocks they aren't familiar with other perfectly normal uses of try-catch.

arvidn commented 6 years ago

It's a completely false analogy to say that if people aren't familiar with function try blocks they aren't familiar with other perfectly normal uses of them.

It is my personal experience that it's common to learn about these uses of try-catch blocks together, and colleagues being surprised of both. But obviously I can't claim this is the case universally.

BjarneStroustrup commented 6 years ago

My impression is that function try blocks are little known because many, probably most, textbooks don't mention them.