Removing C-style casts in favor of explict C++ style casts

[shafik]

Summary

C-style casts hide intent and when code changes they can also hide logic errors.

This change converts most of the C-style cast into static_cast and the icky ones are converted to reinterpret_cast and we can deal with fixing those icky cases in another change.

One case was moved to use std::numeric_limits<unsigned>::max() since it was simply using the fact that -1 converted to unsigned results in the maximum unsigned value.

[labath]

This is fine, though, since you're already touching these lines, I'd consider converting them from LLDB_LOGF to LLDB_LOG at the same time (getting rid of most of the static_casts...).

However, I don't think we should be changing (void) to static_cast<void>.

There is also ample precedent in llvm for using -1 (or sometimes ~0) to mean UINTxx_MAX, so I'm not sure we should change that too. Though it is somewhat confusing, in a way, it is actually safer, as -1 will always convert to the right MAX value. (e.g., in foo = -1, the value will remain fff...ff even if we change the type of foo from uint32_t to uint64_t, but that won't happen with UINT32_MAX)...

lldb/source/Expression/IRExecutionUnit.cpp

306–307

I think this is overkill. (void) is used throughout llvm as a way to explicitly ignore a value (usually done to supress some kind of a warning).

[JDevlieghere]

In D72190#1805861, @labath wrote:

This is fine, though, since you're already touching these lines, I'd consider converting them from LLDB_LOGF to LLDB_LOG at the same time (getting rid of most of the static_casts...).

+1 :-)

[teemperor]

lldb/include/lldb/Expression/IRExecutionUnit.h

330

Do we even need this cast? Neither Clang nor GCC complain about unsigned X = -1 for me.

lldb/include/lldb/Breakpoint/BreakpointOptions.h

protected:
   enum class OptionNames {
     ConditionText = 0,
     IgnoreCount,
     EnabledState,
     OneShotState,
     AutoContinue,
     LastOptionName
   };
-  static const char *g_option_names[(size_t)OptionNames::LastOptionName];
+  static const char
+      *g_option_names[static_cast<size_t>(OptionNames::LastOptionName)];
 
   static const char *GetKey(OptionNames enum_value) {
-    return g_option_names[(size_t)enum_value];
+    return g_option_names[static_cast<size_t>(enum_value)];
   }
 
   static bool BreakpointOptionsCallbackFunction(
       void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
       lldb::user_id_t break_loc_id);
 
   void SetThreadSpec(std::unique_ptr<ThreadSpec> &thread_spec_up);
 

lldb/include/lldb/Breakpoint/BreakpointResolver.h

public:
   /// is used to implement the LLVM classof checks.  This should not be used
   /// for any other purpose, as the values may change as LLDB evolves.
   unsigned getResolverID() const { return SubclassID; }
 
   enum ResolverTy GetResolverTy() {
     if (SubclassID > ResolverTy::LastKnownResolverType)
       return ResolverTy::UnknownResolver;
     else
-      return (enum ResolverTy)SubclassID;
+      return static_cast<enum ResolverTy>(SubclassID);
   }
 
   const char *GetResolverName() { return ResolverTyToName(GetResolverTy()); }
 
   static const char *ResolverTyToName(enum ResolverTy);
 
   static ResolverTy NameToResolverTy(llvm::StringRef name);
 

lldb/include/lldb/Core/SearchFilter.h

public:
   };
 
   static const char *g_ty_to_name[LastKnownFilterType + 2];
 
   enum FilterTy GetFilterTy() {
     if (SubclassID > FilterTy::LastKnownFilterType)
       return FilterTy::UnknownFilter;
     else
-      return (enum FilterTy)SubclassID;
+      return static_cast<enum FilterTy>(SubclassID);
   }
 
   const char *GetFilterName() { return FilterTyToName(GetFilterTy()); }
 
   static const char *FilterTyToName(enum FilterTy);
 
   static FilterTy NameToFilterTy(llvm::StringRef name);
 

lldb/include/lldb/Core/Value.h

bool IsValid() const {
       return (length > 0 && length < kMaxByteSize &&
               byte_order != lldb::eByteOrderInvalid);
     }
     // Casts a vector, if valid, to an unsigned int of matching or largest
     // supported size. Truncates to the beginning of the vector if required.
     // Returns a default constructed Scalar if the Vector data is internally
     // inconsistent.
     llvm::APInt rhs = llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128,
-                                  ((type128 *)bytes)->x);
+                                  (reinterpret_cast<type128 *>(bytes))->x);
     Scalar GetAsScalar() const {
       Scalar scalar;
       if (IsValid()) {
         if (length == 1)
-          scalar = *(const uint8_t *)bytes;
+          scalar = *reinterpret_cast<const uint8_t *>(bytes);
         else if (length == 2)
-          scalar = *(const uint16_t *)bytes;
+          scalar = *reinterpret_cast<const uint16_t *>(bytes);
         else if (length == 4)
-          scalar = *(const uint32_t *)bytes;
+          scalar = *reinterpret_cast<const uint32_t *>(bytes);
         else if (length == 8)
-          scalar = *(const uint64_t *)bytes;
+          scalar = *reinterpret_cast<const uint64_t *>(bytes);
         else if (length >= 16)
           scalar = rhs;
       }
       return scalar;
     }
   };
 
   Value();

lldb/include/lldb/DataFormatters/TypeSynthetic.h

private:
 
     DISALLOW_COPY_AND_ASSIGN(FrontEnd);
   };
 
   SyntheticChildrenFrontEnd::AutoPointer
   GetFrontEnd(ValueObject &backend) override {
     auto synth_ptr = SyntheticChildrenFrontEnd::AutoPointer(
         new FrontEnd(m_python_class, backend));
-    if (synth_ptr && ((FrontEnd *)synth_ptr.get())->IsValid())
+    if (synth_ptr && (static_cast<FrontEnd *>(synth_ptr.get()))->IsValid())
       return synth_ptr;
     return nullptr;
   }
 
 private:
   DISALLOW_COPY_AND_ASSIGN(ScriptedSyntheticChildren);
 };
 } // namespace lldb_private
 
 #endif // lldb_TypeSynthetic_h_

lldb/include/lldb/Expression/IRExecutionUnit.h

 //===-- IRExecutionUnit.h ---------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_IRExecutionUnit_h_
 #define liblldb_IRExecutionUnit_h_
 
 #include <atomic>
+#include <limits>
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/Module.h"
 
 #include "lldb/Expression/IRMemoryMap.h"
std::unique_ptr<SectionMemoryManager> m_default_mm_up; ///< The memory
                                                            /// in actually
                                                            /// creating space.
                                                            /// All calls are
                                                            /// passed through to
                                                            /// it.
     IRExecutionUnit &m_parent; ///< The execution unit this is a proxy for.
   };
 
-  static const unsigned eSectionIDInvalid = (unsigned)-1;
+  static const unsigned eSectionIDInvalid =
+      std::numeric_limits<unsigned>::max();
 
   enum class AllocationKind { Stub, Code, Data, Global, Bytes };
 
   static lldb::SectionType
   GetSectionTypeFromSectionName(const llvm::StringRef &name,
                                 AllocationKind alloc_kind);
 
   /// Encapsulates a single allocation request made by the JIT.

lldb/include/lldb/Symbol/Symbol.h

public:
   void SetReExportedSymbolName(ConstString name);
 
   bool SetReExportedSymbolSharedLibrary(const FileSpec &fspec);
 
   Symbol *ResolveReExportedSymbol(Target &target) const;
 
   uint32_t GetSiblingIndex() const;
 
-  lldb::SymbolType GetType() const { return (lldb::SymbolType)m_type; }
+  lldb::SymbolType GetType() const {
+    return static_cast<lldb::SymbolType>(m_type);
+  }
 
-  void SetType(lldb::SymbolType type) { m_type = (lldb::SymbolType)type; }
+  void SetType(lldb::SymbolType type) {
+    m_type = static_cast<lldb::SymbolType>(type);
+  }
 
   const char *GetTypeAsString() const;
 
   uint32_t GetFlags() const { return m_flags; }
 
   void SetFlags(uint32_t flags) { m_flags = flags; }
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level,

lldb/source/Expression/IRExecutionUnit.cpp

if (func_local_addr == LLDB_INVALID_ADDRESS) {
     ret.SetErrorStringWithFormat("Couldn't find function %s for disassembly",
                                  m_name.AsCString());
     return ret;
   }
 
   LLDB_LOGF(log,
             "Found function, has local address 0x%" PRIx64
             " and remote address 0x%" PRIx64,
-            (uint64_t)func_local_addr, (uint64_t)func_remote_addr);
+            static_cast<uint64_t>(func_local_addr),
+            static_cast<uint64_t>(func_remote_addr));
 
   std::pair<lldb::addr_t, lldb::addr_t> func_range;
 
   func_range = GetRemoteRangeForLocal(func_local_addr);
 
   if (func_range.first == 0 && func_range.second == 0) {
     ret.SetErrorToGenericError();
     ret.SetErrorStringWithFormat("Couldn't find code range for function %s",
void IRExecutionUnit::GetRunnableInfo(Status &error, lldb::addr_t &func_addr,
   class ObjectDumper : public llvm::ObjectCache {
   public:
     void notifyObjectCompiled(const llvm::Module *module,
                               llvm::MemoryBufferRef object) override {
       int fd = 0;
       llvm::SmallVector<char, 256> result_path;
       std::string object_name_model =
           "jit-object-" + module->getModuleIdentifier() + "-%%%.o";
-      (void)llvm::sys::fs::createUniqueFile(object_name_model, fd, result_path);
+      static_cast<void>(
+          llvm::sys::fs::createUniqueFile(object_name_model, fd, result_path));
       llvm::raw_fd_ostream fds(fd, true);
       fds.write(object.getBufferStart(), object.getBufferSize());
     }
 
     std::unique_ptr<llvm::MemoryBuffer>
     getObject(const llvm::Module *module) override {
       // Return nothing - we're just abusing the object-cache mechanism to dump
       // objects.
for (AllocationRecord &record : m_records) {
                                      lldb::eByteOrderBig, 8);
           my_extractor.PutToLog(log, 0, my_buffer.GetByteSize(),
                                 record.m_process_address, 16,
                                 DataExtractor::TypeUInt8);
         }
       } else {
         record.dump(log);
 
-        DataExtractor my_extractor((const void *)record.m_host_address,
-                                   record.m_size, lldb::eByteOrderBig, 8);
+        DataExtractor my_extractor(
+            reinterpret_cast<const void *>(record.m_host_address),
         my_extractor.PutToLog(log, 0, record.m_size, record.m_host_address, 16,
                               DataExtractor::TypeUInt8);
       }
     }
   }
 
   func_addr = m_function_load_addr;
   func_end = m_function_end_load_addr;
uint8_t *IRExecutionUnit::MemoryManager::allocateCodeSection(
     uintptr_t Size, unsigned Alignment, unsigned SectionID,
     llvm::StringRef SectionName) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   uint8_t *return_value = m_default_mm_up->allocateCodeSection(
       Size, Alignment, SectionID, SectionName);
 
   m_parent.m_records.push_back(AllocationRecord(
-      (uintptr_t)return_value,
+      reinterpret_cast<uintptr_t>(return_value),
       lldb::ePermissionsReadable | lldb::ePermissionsExecutable,
       GetSectionTypeFromSectionName(SectionName, AllocationKind::Code), Size,
       Alignment, SectionID, SectionName.str().c_str()));
 
   LLDB_LOGF(log,
             "IRExecutionUnit::allocateCodeSection(Size=0x%" PRIx64
             ", Alignment=%u, SectionID=%u) = %p",
-            (uint64_t)Size, Alignment, SectionID, (void *)return_value);
+            static_cast<uint64_t>(Size), Alignment, SectionID,
+            static_cast<void *>(return_value));
 
   if (m_parent.m_reported_allocations) {
     Status err;
     lldb::ProcessSP process_sp =
         m_parent.GetBestExecutionContextScope()->CalculateProcess();
 
     m_parent.CommitOneAllocation(process_sp, err, m_parent.m_records.back());
   }
 
   return return_value;
 }
 
 uint8_t *IRExecutionUnit::MemoryManager::allocateDataSection(
     uintptr_t Size, unsigned Alignment, unsigned SectionID,
     llvm::StringRef SectionName, bool IsReadOnly) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   uint8_t *return_value = m_default_mm_up->allocateDataSection(
       Size, Alignment, SectionID, SectionName, IsReadOnly);
 
   uint32_t permissions = lldb::ePermissionsReadable;
   if (!IsReadOnly)
     permissions |= lldb::ePermissionsWritable;
   m_parent.m_records.push_back(AllocationRecord(
-      (uintptr_t)return_value, permissions,
+      reinterpret_cast<uintptr_t>(return_value), permissions,
       GetSectionTypeFromSectionName(SectionName, AllocationKind::Data), Size,
       Alignment, SectionID, SectionName.str().c_str()));
   LLDB_LOGF(log,
             "IRExecutionUnit::allocateDataSection(Size=0x%" PRIx64
             ", Alignment=%u, SectionID=%u) = %p",
-            (uint64_t)Size, Alignment, SectionID, (void *)return_value);
+            static_cast<uint64_t>(Size), Alignment, SectionID,
+            static_cast<void *>(return_value));
 
   if (m_parent.m_reported_allocations) {
     Status err;
     lldb::ProcessSP process_sp =
         m_parent.GetBestExecutionContextScope()->CalculateProcess();
 
     m_parent.CommitOneAllocation(process_sp, err, m_parent.m_records.back());
   }
if (ret == LLDB_INVALID_ADDRESS) {
     LLDB_LOGF(log, "IRExecutionUnit::getSymbolAddress(Name=\"%s\") = %" PRIx64,
               Name.c_str(), ret);
     return ret;
   }
 }
 
 void *IRExecutionUnit::MemoryManager::getPointerToNamedFunction(
     const std::string &Name, bool AbortOnFailure) {
-  return (void *)getSymbolAddress(Name);
+  return reinterpret_cast<void *>(getSymbolAddress(Name));
 }
 
 lldb::addr_t
 IRExecutionUnit::GetRemoteAddressForLocal(lldb::addr_t local_address) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   for (AllocationRecord &record : m_records) {
     if (local_address >= record.m_host_address &&
         local_address < record.m_host_address + record.m_size) {
       if (record.m_process_address == LLDB_INVALID_ADDRESS)
         return LLDB_INVALID_ADDRESS;
 
       lldb::addr_t ret =
           record.m_process_address + (local_address - record.m_host_address);
 
       LLDB_LOGF(log,
                 "IRExecutionUnit::GetRemoteAddressForLocal() found 0x%" PRIx64
                 " in [0x%" PRIx64 "..0x%" PRIx64 "], and returned 0x%" PRIx64
                 " from [0x%" PRIx64 "..0x%" PRIx64 "].",
-                local_address, (uint64_t)record.m_host_address,
-                (uint64_t)record.m_host_address + (uint64_t)record.m_size, ret,
-                record.m_process_address,
+                local_address, static_cast<uint64_t>(record.m_host_address),
+                static_cast<uint64_t>(record.m_host_address) +
+                    static_cast<uint64_t>(record.m_size),
+                ret, record.m_process_address,
                 record.m_process_address + record.m_size);
 
       return ret;
     }
   }
 
   return LLDB_INVALID_ADDRESS;
 }
void IRExecutionUnit::ReportAllocations(llvm::ExecutionEngine &engine) {
 
   for (AllocationRecord &record : m_records) {
     if (record.m_process_address == LLDB_INVALID_ADDRESS)
       continue;
 
     if (record.m_section_id == eSectionIDInvalid)
       continue;
 
-    engine.mapSectionAddress((void *)record.m_host_address,
+    engine.mapSectionAddress(reinterpret_cast<void *>(record.m_host_address),
                              record.m_process_address);
   }
 
   // Trigger re-application of relocations.
   engine.finalizeObject();
 }
 
 bool IRExecutionUnit::WriteData(lldb::ProcessSP &process_sp) {
   bool wrote_something = false;
   for (AllocationRecord &record : m_records) {
     if (record.m_process_address != LLDB_INVALID_ADDRESS) {
       lldb_private::Status err;
-      WriteMemory(record.m_process_address, (uint8_t *)record.m_host_address,
+      WriteMemory(record.m_process_address,
+                  reinterpret_cast<uint8_t *>(record.m_host_address),
                   record.m_size, err);
       if (err.Success())
         wrote_something = true;
     }
   }
   return wrote_something;
 }
 
 void IRExecutionUnit::AllocationRecord::dump(Log *log) {
   if (!log)
     return;
 
   LLDB_LOGF(log,
             "[0x%llx+0x%llx]->0x%llx (alignment %d, section ID %d, name %s)",
-            (unsigned long long)m_host_address, (unsigned long long)m_size,
-            (unsigned long long)m_process_address, (unsigned)m_alignment,
-            (unsigned)m_section_id, m_name.c_str());
+            static_cast<unsigned long long>(m_host_address),
+            static_cast<unsigned long long>(m_size),
+            static_cast<unsigned long long>(m_process_address),
+            static_cast<unsigned>(m_alignment),
+            static_cast<unsigned>(m_section_id), m_name.c_str());
 }
 
 lldb::ByteOrder IRExecutionUnit::GetByteOrder() const {
   ExecutionContext exe_ctx(GetBestExecutionContextScope());
   return exe_ctx.GetByteOrder();
 }
 
 uint32_t IRExecutionUnit::GetAddressByteSize() const {

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

public:
 
     void Clear();
 
     bool IsValid() {
       if (!m_parsed)
         Parse();
       if (m_parse_error)
         return false;
-      return (bool)m_full;
+      return static_cast<bool>(m_full);
     }
 
     ConstString GetFullName() const { return m_full; }
 
     std::string GetScopeQualifiedName();
 
     llvm::StringRef GetBasename();