I'm mostly happy with this patch. Accepting it, so that you can land it without
yet another round of review, but please consider addressing the comments below.
Thank you for doing this!
================
Comment at: lib/Driver/SanitizerArgs.cpp:147
@@ +146,3 @@
+ Add &= ~TrapRemove;
+ if ((Add & ~TrappingSupportedWithGroups) != 0) {
+ SanitizerSet S;
----------------
if (SanitizerMask InvalidValues = Add & ~TrappingSupportedWithGroups) {
SanitizerSet S;
S.Mask = InvalidValues;
//....
}
================
Comment at: lib/Driver/SanitizerArgs.cpp:165
@@ +164,3 @@
+ Arg->claim();
+ TrapRemove |= Undefined | UndefinedGroup;
+ }
----------------
TrapRemove |= expandSanitizerGroups(UndefinedGroup);
================
Comment at: lib/Driver/SanitizerArgs.cpp:239
@@ +238,3 @@
+ SanitizerMask KindsToDiagnose = Add & NotSupported & ~DiagnosedKinds;
+ if (KindsToDiagnose & Vptr) {
+ D.Diag(diag::err_drv_argument_not_allowed_with)
----------------
I don't think this is right. Suppose `-fsanitize=vptr` is not supported on
target foo (by corresponding toolchain). Then
clang -target foo -fsanitize=vptr a.cc
would produce confusing
"-fsanitize=vptr" is not allowed with "-fsanitize-trap=undefined"
I think you would need separate clause for that
if (SanitizerMask KindsToDiagnose = Add & TrappingKinds & NotAllowedWithTrap
& ~DiagnosedKinds) {
std::string Desc = describeSanitizeArg(*I, KindsToDiagnose);
D.Diag(diag::err_drv_argument_not_allowed_with) << Desc <<
lastTrapArgumentForMask(KindsToDiagnose);
DiagnosedKinds |= KindsToDiagnose;
}
http://reviews.llvm.org/D10464
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
