================
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.
================
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<
----------------
Chandler Carruth wrote:
> 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.
The idea is to model an enumeration of selectors, not a bool... The value
accepted by the diagnostic engine is technically an 'unsigned'.
http://llvm-reviews.chandlerc.com/D146
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
