aaron.ballman added a comment.
In https://reviews.llvm.org/D39013#900046, @erichkeane wrote:
> I've convinced myself that a re-throw with a catch around it should not be
> diagnosed, otherwise there is no way to suppress the warning in this case.
> It ends up being a false-positive in many cases.
Wouldn't a catch-all handler suppress the warning in that case? e.g.,
void f() noexcept {
try {
throw; // rethrows
} catch (int) {
// We hope this catches it, and if it always does, then this is a fp
warning.
} catch (...) {
// However, without cross-TU analysis, we can't know that detail and it
seems plausible that
// the active exception object is something else, so the catch-all is not
unreasonable and adds
// extra safety. The diagnostic (even if a fp) requires the developer to
think about this case.
assert(false);
}
}
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:299
if (!ThrowType)
return false;
if (ThrowType->isReferenceType())
----------------
erichkeane wrote:
> The other potential fix here is to simply change this line here to "return
> true;". This would result the warning being suppressed on 'rethrow' if there
> is any catch statements.
>
> I wonder what the opinion of the reviewers is on this one.
I'm a little bit concerned about it, truth be told. The current behavior has
false positives, and this patch trades those for false negatives. The question
boils down to which is the more likely scenario. Rethrow from within a try
block that catches the exception being rethrown seems like a rather unlikely
scenario to me. Do you have examples of this being a false positive with
real-world code?
I do agree that the catch-all handler should silence the diagnostic, however.
https://reviews.llvm.org/D39013
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
