Nice. Thanks for the pointer. Does the attached patch look good?
On Tue, Dec 4, 2012 at 2:00 AM, Richard Smith <[email protected]> wrote: > On Fri, Nov 30, 2012 at 3:01 AM, Chandler Carruth <[email protected]>wrote: > >> >> >> ================ >> Comment at: >> llvm/tools/clang/include/clang/Basic/DiagnosticDriverKinds.td:104-107 >> @@ -103,2 +103,6 @@ >> "AddressSanitizer on Android requires '-pie'">; >> +def err_drv_tsan_requires_pie : Error< >> + "ThreadSanitizer requires '-pie'">; >> +def err_drv_msan_requires_pie : Error< >> + "MemorySanitizer requires '-pie'">; >> def err_drv_unknown_objc_runtime : Error< >> ---------------- >> Evgeniy Stepanov wrote: >> > Dmitri Gribenko wrote: >> > > Chandler Carruth wrote: >> > > > You don't need two diagnostics: >> > > > >> > > > def err_drv_sanitizer_requires_pie : Error< >> > > > "%select{thread|memory}0 sanitizer requires the '-pie' option">; >> > > I think it is better to use two diagnostics since %select would >> require magic numbers in the source. >> > I also prefer two diagnosticts. >> > >> > Btw, %select{a|b}0 evaluates to "b" if true, and "a" if false. I find >> this _really_ weird. >> > >> 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. >> >> I also dislike the duplicated text intensely. >> >> A good solution would be something like this: >> >> def err_drv_sanitizer_requires_pie : Error< >> "%select{thread|memory}0 sanitizer requires the '-pie' option">, >> SelectNames<0, { SDK_Thread, SDK_Memory }>; >> >> 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. >> >> 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. > > > We already have diag::err_drv_argument_only_allowed_with for indicating > that one argument requires another. >
1.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
