[Lldb-commits] [lldb] r263333 - Let's not convert from UINT32_MAX to the std::numeric_limits version.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Tue Mar 15 14:37:01 PDT 2016


It doesn't look like there is any advantage to the numeric_limits if you already know the type you are getting the max or min of.  The template would be nice for doing more generic programming, but we're never doing anything fancier than "What is the max of a uint32_t".  It doesn't look like there is any advantage to the numeric_limits if you already know the type you are getting the max or min of.

In general we tend not to use raw UINT32_MAX and the like, but semantically significant equivalents like LLDB_INVALID_THREAD_ID, etc.  Some of the uses of UINT32_MAX here should have been LLDB_INVALID_FILE_INDEX32, for instance, though not all.  At some point we should go clean up uses of *_MAX and use more meaningful defines where appropriate.  But I can't see that there is any value to replacing the current _MAX usage with the much more verbose numeric_limits invocations.

Jim
 

> On Mar 15, 2016, at 2:20 PM, Zachary Turner <zturner at google.com> wrote:
> 
> Is the stdint version better somehow?  I thought C++ numeric_limits were actually preferred over the C macros.
> 
> On Mon, Mar 14, 2016 at 7:59 AM Adrian McCarthy via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> If we're favoring the <stdint.h> macros over the <limits> functions, then perhaps update the #includes?
> 
> On Fri, Mar 11, 2016 at 7:33 PM, Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> Author: jingham
> Date: Fri Mar 11 21:33:36 2016
> New Revision: 263333
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=263333&view=rev
> Log:
> Let's not convert from UINT32_MAX to the std::numeric_limits version.
> 
> Modified:
>     lldb/trunk/source/Core/DataEncoder.cpp
>     lldb/trunk/source/Core/Disassembler.cpp
>     lldb/trunk/source/Core/FileSpecList.cpp
>     lldb/trunk/source/Core/SearchFilter.cpp
> 
> Modified: lldb/trunk/source/Core/DataEncoder.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/DataEncoder.cpp?rev=263333&r1=263332&r2=263333&view=diff
> ==============================================================================
> --- lldb/trunk/source/Core/DataEncoder.cpp (original)
> +++ lldb/trunk/source/Core/DataEncoder.cpp Fri Mar 11 21:33:36 2016
> @@ -233,7 +233,7 @@ DataEncoder::PutU8 (uint32_t offset, uin
>          m_start[offset] = value;
>          return offset + 1;
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  uint32_t
> @@ -248,7 +248,7 @@ DataEncoder::PutU16 (uint32_t offset, ui
> 
>          return offset + sizeof (value);
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  uint32_t
> @@ -263,7 +263,7 @@ DataEncoder::PutU32 (uint32_t offset, ui
> 
>          return offset + sizeof (value);
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  uint32_t
> @@ -278,7 +278,7 @@ DataEncoder::PutU64 (uint32_t offset, ui
> 
>          return offset + sizeof (value);
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  //----------------------------------------------------------------------
> @@ -304,7 +304,7 @@ DataEncoder::PutMaxU64 (uint32_t offset,
>          assert(!"GetMax64 unhandled case!");
>          break;
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  uint32_t
> @@ -318,7 +318,7 @@ DataEncoder::PutData (uint32_t offset, c
>          memcpy (m_start + offset, src, src_len);
>          return offset + src_len;
>      }
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  uint32_t
> @@ -332,5 +332,5 @@ DataEncoder::PutCString (uint32_t offset
>  {
>      if (cstr != nullptr)
>          return PutData (offset, cstr, strlen(cstr) + 1);
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
> Modified: lldb/trunk/source/Core/Disassembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Disassembler.cpp?rev=263333&r1=263332&r2=263333&view=diff
> ==============================================================================
> --- lldb/trunk/source/Core/Disassembler.cpp (original)
> +++ lldb/trunk/source/Core/Disassembler.cpp Fri Mar 11 21:33:36 2016
> @@ -1036,7 +1036,7 @@ InstructionList::GetIndexOfNextBranchIns
>  {
>      size_t num_instructions = m_instructions.size();
> 
> -    uint32_t next_branch = std::numeric_limits<uint32_t>::max();
> +    uint32_t next_branch = UINT32_MAX;
>      size_t i;
>      for (i = start; i < num_instructions; i++)
>      {
> @@ -1053,7 +1053,7 @@ InstructionList::GetIndexOfNextBranchIns
>      if (target.GetArchitecture().GetTriple().getArch() == llvm::Triple::hexagon)
>      {
>          // If we didn't find a branch, find the last packet start.
> -        if (next_branch == std::numeric_limits<uint32_t>::max())
> +        if (next_branch == UINT32_MAX)
>          {
>              i = num_instructions - 1;
>          }
> @@ -1086,7 +1086,7 @@ InstructionList::GetIndexOfNextBranchIns
>              }
>          }
> 
> -        if (next_branch == std::numeric_limits<uint32_t>::max())
> +        if (next_branch == UINT32_MAX)
>          {
>              // We couldn't find the previous packet, so return start
>              next_branch = start;
> @@ -1099,7 +1099,7 @@ uint32_t
>  InstructionList::GetIndexOfInstructionAtAddress (const Address &address)
>  {
>      size_t num_instructions = m_instructions.size();
> -    uint32_t index = std::numeric_limits<uint32_t>::max();
> +    uint32_t index = UINT32_MAX;
>      for (size_t i = 0; i < num_instructions; i++)
>      {
>          if (m_instructions[i]->GetAddress() == address)
> @@ -1152,7 +1152,7 @@ Disassembler::ParseInstructions (const E
>                                  m_arch.GetByteOrder(),
>                                  m_arch.GetAddressByteSize());
>              const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
> -            return DecodeInstructions(range.GetBaseAddress(), data, 0, std::numeric_limits<uint32_t>::max(), false,
> +            return DecodeInstructions(range.GetBaseAddress(), data, 0, UINT32_MAX, false,
>                                        data_from_file);
>          }
>          else if (error_strm_ptr)
> 
> Modified: lldb/trunk/source/Core/FileSpecList.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FileSpecList.cpp?rev=263333&r1=263332&r2=263333&view=diff
> ==============================================================================
> --- lldb/trunk/source/Core/FileSpecList.cpp (original)
> +++ lldb/trunk/source/Core/FileSpecList.cpp Fri Mar 11 21:33:36 2016
> @@ -125,7 +125,7 @@ FileSpecList::FindFileIndex (size_t star
>      }
> 
>      // We didn't find the file, return an invalid index
> -    return std::numeric_limits<uint32_t>::max();
> +    return UINT32_MAX;
>  }
> 
>  //------------------------------------------------------------------
> 
> Modified: lldb/trunk/source/Core/SearchFilter.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/SearchFilter.cpp?rev=263333&r1=263332&r2=263333&view=diff
> ==============================================================================
> --- lldb/trunk/source/Core/SearchFilter.cpp (original)
> +++ lldb/trunk/source/Core/SearchFilter.cpp Fri Mar 11 21:33:36 2016
> @@ -266,7 +266,10 @@ SearchFilter::DoFunctionIteration (Funct
>  bool
>  SearchFilterForUnconstrainedSearches::ModulePasses (const FileSpec &module_spec)
>  {
> -    return (!m_target_sp->ModuleIsExcludedForUnconstrainedSearches(module_spec));
> +    if (m_target_sp->ModuleIsExcludedForUnconstrainedSearches (module_spec))
> +        return false;
> +    else
> +        return true;
>  }
> 
>  bool
> @@ -445,7 +448,7 @@ SearchFilterByModuleList::ModulePasses (
>          return true;
> 
>      if (module_sp &&
> -        m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) != std::numeric_limits<uint32_t>::max())
> +        m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) != UINT32_MAX)
>          return true;
>      else
>          return false;
> @@ -457,7 +460,7 @@ SearchFilterByModuleList::ModulePasses (
>      if (m_module_spec_list.GetSize() == 0)
>          return true;
> 
> -    if (m_module_spec_list.FindFileIndex(0, spec, true) != std::numeric_limits<uint32_t>::max())
> +    if (m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX)
>          return true;
>      else
>          return false;
> @@ -506,7 +509,7 @@ SearchFilterByModuleList::Search (Search
>      for (size_t i = 0; i < num_modules; i++)
>      {
>          Module* module = target_modules.GetModulePointerAtIndexUnlocked(i);
> -        if (m_module_spec_list.FindFileIndex(0, module->GetFileSpec(), false) != std::numeric_limits<uint32_t>::max())
> +        if (m_module_spec_list.FindFileIndex(0, module->GetFileSpec(), false) != UINT32_MAX)
>          {
>              SymbolContext matchingContext(m_target_sp, module->shared_from_this());
>              Searcher::CallbackReturn shouldContinue;
> @@ -613,13 +616,13 @@ SearchFilterByModuleListAndCU::AddressPa
>  bool
>  SearchFilterByModuleListAndCU::CompUnitPasses (FileSpec &fileSpec)
>  {
> -    return m_cu_spec_list.FindFileIndex(0, fileSpec, false) != std::numeric_limits<uint32_t>::max();
> +    return m_cu_spec_list.FindFileIndex(0, fileSpec, false) != UINT32_MAX;
>  }
> 
>  bool
>  SearchFilterByModuleListAndCU::CompUnitPasses (CompileUnit &compUnit)
>  {
> -    bool in_cu_list = m_cu_spec_list.FindFileIndex(0, compUnit, false) != std::numeric_limits<uint32_t>::max();
> +    bool in_cu_list = m_cu_spec_list.FindFileIndex(0, compUnit, false) != UINT32_MAX;
>      if (in_cu_list)
>      {
>          ModuleSP module_sp(compUnit.GetModule());
> @@ -662,7 +665,7 @@ SearchFilterByModuleListAndCU::Search (S
>      {
>          lldb::ModuleSP module_sp = target_images.GetModuleAtIndexUnlocked(i);
>          if (no_modules_in_filter ||
> -            m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) != std::numeric_limits<uint32_t>::max())
> +            m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) != UINT32_MAX)
>          {
>              SymbolContext matchingContext(m_target_sp, module_sp);
>              Searcher::CallbackReturn shouldContinue;
> @@ -682,8 +685,7 @@ SearchFilterByModuleListAndCU::Search (S
>                      matchingContext.comp_unit = cu_sp.get();
>                      if (matchingContext.comp_unit)
>                      {
> -                        if (m_cu_spec_list.FindFileIndex(0, *matchingContext.comp_unit, false) !=
> -                            std::numeric_limits<uint32_t>::max())
> +                        if (m_cu_spec_list.FindFileIndex(0, *matchingContext.comp_unit, false) != UINT32_MAX)
>                          {
>                              shouldContinue = DoCUIteration(module_sp, matchingContext, searcher);
>                              if (shouldContinue == Searcher::eCallbackReturnStop)
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits



More information about the lldb-commits mailing list