On Tue, Dec 4, 2012 at 3:04 AM, Evgeniy Stepanov <span dir="ltr"><<a href="mailto:eugenis@google.com" target="_blank">eugenis@google.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div dir="ltr">Nice. Thanks for the pointer.<div>Does the attached patch look good?</div></div></div></blockquote><div><br></div><div>Looks fine. Presumably a test needs updating too?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div><div class="h5"><div class="gmail_extra">
<div class="gmail_quote">On Tue, Dec 4, 2012 at 2:00 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><div>On Fri, Nov 30, 2012 at 3:01 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br></div></div><div class="gmail_quote">
<div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
<br>
================<br>
Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticDriverKinds.td:104-107<br>
@@ -103,2 +103,6 @@<br>
</div><div>   "AddressSanitizer on Android requires '-pie'">;<br>
</div><div>+def err_drv_tsan_requires_pie : Error<<br>
+  "ThreadSanitizer requires '-pie'">;<br>
+def err_drv_msan_requires_pie : Error<<br>
+  "MemorySanitizer requires '-pie'">;<br>
 def err_drv_unknown_objc_runtime : Error<<br>
----------------<br>
</div><div>Evgeniy Stepanov wrote:<br>
> Dmitri Gribenko wrote:<br>
> > Chandler Carruth wrote:<br>
> > > You don't need two diagnostics:<br>
> > ><br>
> > >   def err_drv_sanitizer_requires_pie : Error<<br>
> > >     "%select{thread|memory}0 sanitizer requires the '-pie' option">;<br>
> > I think it is better to use two diagnostics since %select would require magic numbers in the source.<br>
> I also prefer two diagnosticts.<br>
><br>
> Btw, %select{a|b}0 evaluates to "b" if true, and "a" if false. I find this _really_ weird.<br>
><br>
</div>I don't like that pattern any more than you do, but Clang's diagnostics use it *extensively*, adding a comment next to the magic number indicating which is which.<br>
<br>
I also dislike the duplicated text intensely.<br>
<br>
A good solution would be something like this:<br>
<br>
  def err_drv_sanitizer_requires_pie : Error<<br>
    "%select{thread|memory}0 sanitizer requires the '-pie' option">,<br>
    SelectNames<0, { SDK_Thread, SDK_Memory }>;<br>
<br>
Where this automatically teaches the diagnostic engine that select #0 should accept a specific enum rather than an unsigned, and defines that enum in the diag namespace with the given enumerators.<br>
<br>
But this would be a huge (good) change and shouldn't be in this patch. I think I mildly prefer consistency with other diagnostics until we get something like this.</blockquote><div><br></div></div></div><div>We already have diag::err_drv_argument_only_allowed_with for indicating that one argument requires another.</div>


</div>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br>