Hate to dig up an old commit...<br><br><div class="gmail_quote">On Thu, May 12, 2011 at 3:46 PM, Sean Hunt <span dir="ltr"><<a href="mailto:scshunt@csclub.uwaterloo.ca">scshunt@csclub.uwaterloo.ca</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div id=":50">Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=131260&r1=131259&r2=131260&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=131260&r1=131259&r2=131260&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu May 12 17:46:29 2011<br>
@@ -1327,11 +1327,12 @@<br>
 bool Sema::<span class="il">FindAllocationOverload</span>(SourceLocation StartLoc, SourceRange Range,<br>
                                   DeclarationName Name, Expr** Args,<br>
                                   unsigned NumArgs, DeclContext *Ctx,<br>
-                                  bool AllowMissing, FunctionDecl *&Operator) {<br>
+                                  bool AllowMissing, FunctionDecl *&Operator,<br>
+                                  bool Diagnose) {<br></div></blockquote><div><br></div><div>I'm now thinking this pattern (the Diagnose bool) is more than annoying, it's actively harmful. Why can't we use one of the scoped diagnostic suppression utilities in Sema instead? That has quite a few advantages:</div>
<div>1) It doesn't require threading parameters everywhere</div><div>2) It is more localized to the area of the code that needs to suppress diagnostics</div><div>3) It doesn't suffer from programmer errors such as:</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div id=":50">
   case OR_No_Viable_Function:<br>
-    Diag(StartLoc, diag::err_ovl_no_viable_function_in_call)<br>
-      << Name << Range;<br>
+    if (Diagnose)<br>
+      Diag(StartLoc, diag::err_ovl_no_viable_function_in_call)<br>
+        << Name << Range;<br>
     Candidates.NoteCandidates(*this, OCD_AllCandidates, Args, NumArgs);<br>
     return true;<br></div></blockquote></div><div><br></div><div>The notes aren't conditioned here, and so are emitted and attached to whatever diagnostic happened to precede it. I know you fixed a bunch of these, but I just fixed more in r132746.</div>
<div><br></div><div>I understand the disadvantage of the scoped object too -- it makes it easy to forget to short-circuit any expensive operations performed only to produce diagnostics which are then suppressed. However, we could make this easier, potentially providing a way to more directly attach a callback for generating the (expensive) notes to a particular diagnostic, or just making easy query APIs for Sema-level suppression state and paying attention to profiles.</div>
<div><br></div><div>Anyways, wanted to bring this up to see if there is a better pattern to follow, and as a reminder that you may want to audit some of the other places you're using this pattern currently for similar bugs.</div>