Add failure paths to a few JSONNumber members

[omjavaid]

Summary

This patch updates GetAsUnsigned(), GetAsSigned(), and GetAsDouble() JSONNumber functions to add failure check.

The previous code was generating compiler warnings for not being able to provide a return path for all possibilities.

[tberghammer]

These GetAs{...} functions should never fail in their current implementation as we have only 3 different data type and all of them are handled. The new function signatures you proposed are making the API significantly harder to use with no benefit. (Also they will produce clang warnings because of the default in a switch covering all cases.)

To silent the warning I would suggest to do this:

double
JSONNumber::GetAsDouble() const
{
    switch (m_data_type)
    {
        case DataType::Unsigned:
          return (double)m_data.m_unsigned;
        case DataType::Signed:
          return (double)m_data.m_signed;
        case DataType::Double:
          return m_data.m_double;
    }
    assert(false && "Unhandled data type");
    return 0;
}

In this code:

• Gcc won't emit warnings because all code path have a return value.
• Clang will emit a warning if somebody adds a new data type without handling them in these GetAs{...} functions (this is why I don't want a default case).
• We have an additional runtime check for the fail case with the assert if the warnings are ignored.

[labath]

btw, if you use llvm_unreachable("text"), then you don't need the return after that.

[omjavaid]

Correction made upstream by @tberghammer.

[tberghammer]

I don't know what correction you are referring to as I haven't made any change in JSON.{h,cpp} since you created this patch but I am happy with the current situation (no warning on clang)

[omjavaid]

Sorry @tberghammer there was corruption in my setup showing changes which i just made internally.

I have updated this diff as per suggestions.

LGTM?

[tberghammer]

LGTM

include/lldb/Utility/JSON.h

148

Nice catch!

include/lldb/Utility/JSON.h

public:
         operator = (const JSONNumber& s) = delete;
 
         void
         Write(Stream& s) override;
 
         uint64_t
         GetAsUnsigned() const;
 
-        uint64_t
+        int64_t
         GetAsSigned() const;
 
         double
         GetAsDouble() const;
 
         static bool classof(const JSONValue *V)
         {
             return V->GetKind() == JSONValue::Kind::Number;

source/Utility/JSON.cpp

 //===--------------------- JSON.cpp -----------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/JSON.h"
 
 #include <limits.h>
 #include "lldb/Core/StreamString.h"
 #include "lldb/Host/StringConvert.h"
+#include "llvm/Support/ErrorHandling.h"
 
 using namespace lldb_private;
 
 std::string
 JSONString::json_string_quote_metachars (const std::string &s)
 {
     if (s.find('"') == std::string::npos)
         return s;
JSONNumber::GetAsUnsigned() const
     {
         case DataType::Unsigned:
             return m_data.m_unsigned;
         case DataType::Signed:
             return (uint64_t)m_data.m_signed;
         case DataType::Double:
             return (uint64_t)m_data.m_double;
     }
+    llvm_unreachable("Unhandled data type");
 }
 
-uint64_t
+int64_t
 JSONNumber::GetAsSigned() const
 {
     switch (m_data_type)
     {
         case DataType::Unsigned:
             return (int64_t)m_data.m_unsigned;
         case DataType::Signed:
             return m_data.m_signed;
         case DataType::Double:
             return (int64_t)m_data.m_double;
     }
+    llvm_unreachable("Unhandled data type");
 }
 
 double
 JSONNumber::GetAsDouble() const
 {
     switch (m_data_type)
     {
         case DataType::Unsigned:
             return (double)m_data.m_unsigned;
         case DataType::Signed:
             return (double)m_data.m_signed;
         case DataType::Double:
             return m_data.m_double;
     }
+    llvm_unreachable("Unhandled data type");
 }
 
 void
 JSONNumber::Write (Stream& s)
 {
     switch (m_data_type)
     {
         case DataType::Unsigned: