[clang-tidy] Create alias cert-err51-cpp for check bugprone-exception-escape

[vbvictor]

This CERT rule https://wiki.sei.cmu.edu/confluence/display/cplusplus/ERR51-CPP.+Handle+all+exceptions should be already fully covered by clang-tidy https://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-escape.html check

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Baranov Victor (vbvictor)

This CERT rule https://wiki.sei.cmu.edu/confluence/plugins/servlet/mobile?contentId=88046367#content/view/88046367 should be already fully covered by clang-tidy https://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-escape.html check

[vbvictor]

Once we create this alias, it could be added to CERT list on their Confluence.
Maybe we would need to ask Aaron for it.

[vbvictor]

I think we need to twist settings of bugprone-exception-escape.
Currently, it unconditionally diagnoses the following violations:

• Destructors
• Move constructors
• Move assignment operators
• The main() functions
• swap() functions
• iter_swap() functions
• iter_move() functions
• Functions marked with throw() or noexcept
• Other functions given as option

But as for CERT guidelines, I determined that only a portion of violations should be diagnosed:

ERR51-CPP should diagnose:

• Destructors
• The main() functions

ERR55-CPP should diagnose:

• Functions marked with throw() or noexcept

@AaronBallman, could you please confirm I made the correct mapping for CERT rules?
If so, I'd proceed to add new aliases which we could maybe place on CERT website.

[AaronBallman]

I think we need to twist settings of bugprone-exception-escape. Currently, it unconditionally diagnoses the following violations:

* Destructors

* Move constructors

* Move assignment operators

* The main() functions

* swap() functions

* iter_swap() functions

* iter_move() functions

* Functions marked with throw() or noexcept

* Other functions given as option

But as for CERT guidelines, I determined that only a portion of violations should be diagnosed:

ERR51-CPP should diagnose:

* Destructors

* The main() functions

ERR55-CPP should diagnose:

* Functions marked with throw() or noexcept

@AaronBallman, could you please confirm I made the correct mapping for CERT rules? If so, I'd proceed to add new aliases which we could maybe place on CERT website.

Yes, but with a caveat to be careful with ERR55-CPP to make sure we don't diagnose code like this:

int func(int i) noexcept {
  try {
    something_which_can_throw_an_exception();
  } catch (...) {
    return 12;
  }
  return 100;
}

but still diagnoses something like this:

void foo() { throw 12; }
int func(int i) noexcept {
  try {
    foo();
  } catch (std::exception E) { // Doesn't catch the thrown 'int'
    return 12;
  }
  return 100;
}

So ERR55-CPP might be harder to handle from a tidy check.

Once we create this alias, it could be added to CERT list on their Confluence.
Maybe we would need to ask Aaron for it.

I won't be able to add it myself but I know who to poke to get it added and am happy to help coordinate.

[vbvictor]

but still diagnoses something like this:

Yes, the check already take account of different exception types, so it can find violations this:

llvm-project/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

Lines 568 to 580 in f973d77

	void bad_catch_nested_try() noexcept {
	// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'bad_catch_nested_try' which should not throw exceptions
	try {
	throw 1;
	} catch(int &) {
	try {
	throw 1.1;
	} catch(int &) {
	}
	} catch(double &) {
	}
	}
	// CHECK-MESSAGES: :[[@LINE-6]]:7: note: frame #0: unhandled exception of type 'double' may be thrown in function 'bad_catch_nested_try' here

The check was improved in 21th release by printing "stack trace" of potentially escaped exception so it became more user-friendly and now we can be proud of it