[PATCH] Unify sanitizer kind representation between the driver and the rest of the compiler.

[Peter Collingbourne]

================
Comment at: include/clang/Basic/Sanitizers.h:28
@@ +27,3 @@
+// bit positions.
+enum : uint64_t {
+#define SANITIZER(NAME, ID) SO_##ID,
----------------
samsonov wrote:
> Leave this named as SanitizerOrdinal, so that SO_ prefix makes sense.
Done.

================
Comment at: include/clang/Basic/Sanitizers.h:49
@@ -31,2 +48,3 @@
 
   /// \brief Check if a certain sanitizer is enabled.
+  bool has(SanitizerMask K) const;
----------------
samsonov wrote:
> Fix comments to reflect that we're checking/setting sets of sanitizers now. However, I think we might still use two different entities for SanitizerKind and SanitizerMask....
> Fix comments to reflect that we're checking/setting sets of sanitizers now.

These functions are for single sanitizers only. I've added assertion checks to enforce that.

> However, I think we might still use two different entities for SanitizerKind and SanitizerMask....

I don't think we need to do that. In my opinion it just makes things more confusing.

================
Comment at: include/clang/Basic/Sanitizers.h:62
@@ -43,1 +61,3 @@
+  /// \brief Bitmask of enabled sanitizers.
+  SanitizerMask Mask;
 };
----------------
samsonov wrote:
> Any reason to make this public?
It makes the code simpler, as we don't need accessor functions in every case where we need to do some bit operation on the mask.

================
Comment at: lib/CodeGen/CGExpr.cpp:2246
@@ -2245,3 +2245,3 @@
 
-static CheckRecoverableKind getRecoverableKind(SanitizerKind Kind) {
+static CheckRecoverableKind getRecoverableKind(SanitizerMask Kind) {
   switch (Kind) {
----------------
samsonov wrote:
> That's the problem I see with using `SanitizerMask` instead of `SanitizerKind` class enum - sometimes (like in this case) we want to distinguish between a *single* sanitizer kind and a set of sanitizers. Here, for example, what should we return if Kind contains both Vptr and Return? Add a method that would tell you if SanitizerMask contains a single kind, and assert in this function?
> Add a method that would tell you if SanitizerMask contains a single kind, and assert in this function?

Yes. It makes more sense to add dynamic checks to the rare function that accepts a single sanitizer, than to use two different enums and  try to make sure that the right one is being used everywhere.

================
Comment at: lib/CodeGen/CGExpr.cpp:2304
@@ -2303,3 +2303,3 @@
     llvm::Value *&Cond =
         CGM.getCodeGenOpts().SanitizeRecover.has(Checked[i].second)
             ? RecoverableCond
----------------
samsonov wrote:
> Same here - what if SanitizeRecover is defined for some bits in Checked[i].second, and not defined for another?
Added an assertion check to SanitizerSet::has.

http://reviews.llvm.org/D9618

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/