switch on enum should be exhaustive and warning-free
authorTim Hammerquist <penryu@apple.com>
Tue, 11 Jul 2017 21:06:20 +0000 (21:06 +0000)
committerTim Hammerquist <penryu@apple.com>
Tue, 11 Jul 2017 21:06:20 +0000 (21:06 +0000)
Summary:
Testing the value of type_code against the closed enum TypeCodes
provides statically verifiable completeness of testing. However, one
branch assigns to type_code by casting directly from a masked integer
value. This is currently handled by adding a default: case after
checking each TypeCodes instance. This patch introduces a bool variable
containing the "default" state value, allowing the switch to be
exhaustive, protect against future instances not being handled in the
switch, and preserves the original logic.

This addresses the warning:
warning: default label in switch which covers all enumeration values
[-Wcovered-switch-default]

As an issue of maintainability, the bitmask on line 524 handles the
current values of TypeCodes enum, but this will be invalid if the enum
is extended. This patch does not address this, and a more closed
conversion from cfinfoa -> TypeCodes would help protect against this.

Reviewers: spyffe, lhames, sas

Reviewed By: sas

Subscribers: sas, lldb-commits

Differential Revision: https://reviews.llvm.org/D35036

llvm-svn: 307712

lldb/source/Plugins/Language/ObjC/Cocoa.cpp

index 2ba005e..8f49975 100644 (file)
@@ -543,6 +543,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
       }
       
       uint64_t value = 0;
+      bool success = false;
       switch (type_code) {
         case TypeCodes::sint8:
         value = process_sp->ReadUnsignedIntegerFromMemory(data_location, 1, 0,
@@ -550,6 +551,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
         if (error.Fail())
           return false;
         NSNumber_FormatChar(valobj, stream, (char)value, options.GetLanguage());
+        success = true;
         break;
         case TypeCodes::sint16:
         value = process_sp->ReadUnsignedIntegerFromMemory(data_location, 2, 0,
@@ -558,6 +560,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
           return false;
         NSNumber_FormatShort(valobj, stream, (short)value,
                              options.GetLanguage());
+        success = true;
         break;
       case TypeCodes::sint32:
         value = process_sp->ReadUnsignedIntegerFromMemory(data_location, 4, 0,
@@ -565,6 +568,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
         if (error.Fail())
           return false;
         NSNumber_FormatInt(valobj, stream, (int)value, options.GetLanguage());
+        success = true;
         break;
       case TypeCodes::sint64:
         value = process_sp->ReadUnsignedIntegerFromMemory(data_location, 8, 0,
@@ -572,6 +576,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
         if (error.Fail())
           return false;
         NSNumber_FormatLong(valobj, stream, value, options.GetLanguage());
+        success = true;
         break;
       case TypeCodes::f32:
       {
@@ -582,6 +587,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
         float flt_value = 0.0f;
         memcpy(&flt_value, &flt_as_int, sizeof(flt_as_int));
         NSNumber_FormatFloat(valobj, stream, flt_value, options.GetLanguage());
+        success = true;
         break;
       }
       case TypeCodes::f64:
@@ -593,6 +599,7 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
         double dbl_value = 0.0;
         memcpy(&dbl_value, &dbl_as_lng, sizeof(dbl_as_lng));
         NSNumber_FormatDouble(valobj, stream, dbl_value, options.GetLanguage());
+        success = true;
         break;
       }
       case TypeCodes::sint128: // internally, this is the same
@@ -608,12 +615,11 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
           return false;
         llvm::APInt i128_value(128, words);
         NSNumber_FormatInt128(valobj, stream, i128_value, options.GetLanguage());
+        success = true;
         break;
       }
-      default:
-        return false;
       }
-      return true;
+      return success;
     }
   }