[PATCH] D39013: [CFG] Relax Wexceptions warning on rethrow
Erich Keane via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 17 13:29:03 PDT 2017
erichkeane added a comment.
> Wouldn't a catch-all handler suppress the warning in that case? e.g.,
Yep, you're right of course. I'll revert to my first patch and update this. I'd surmised that false-positives were worse than false-negatives, but since there is a trivial workaround, I'm ok with that. Thanks for the quick review!
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:299
if (!ThrowType)
return false;
if (ThrowType->isReferenceType())
----------------
aaron.ballman wrote:
> 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.
I don't have any examples, just the bug report.
https://reviews.llvm.org/D39013
More information about the cfe-commits
mailing list