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 <ztur...@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@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@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@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to