[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