> On Mar 15, 2016, at 3:00 PM, Zachary Turner <ztur...@google.com> wrote: > > I agree that they are pretty much equivalent, I guess the main advantage I > see is that you don't have to think about what type something is when > assigning it an "invalid" value. For example, LLDB_INVALID_THREAD_ID could > just as easily be std::numeric_limits<lldb::tid_t>::min(), and then you don't > have to worry about whether it's signed, or unsigned, or 32 bits, or 64-bits. >
I'd be fine with this at the place where LLDB_INVALID_THREAD_ID was defined if that somehow made things more better. But std::numeric_limits<lldb::tid_t>::min() is not very expressive of the fact that you're testing for an invalid thread ID, so I wouldn't want to use that as the test in ordinary code. I bet many of the uses of UINT32_MAX are places where we should have invented (or did invent but didn't use) a more significant name. Cleaning that up will not be made easier by replacing UINT32_MAX with the limits version. Jim > > I guess put another way, if they're equivalent then we should prefer the C++ > ones if for no other reason than that it makes porting refactoring, porting, > and other similar tasks easier in the future (even if it seems unlikely and > the benefit is mostly theoretical). > > Anyway, doesn't really matter, I just thought it strange to see C++ code > ported to C, as usually it happens the other way around. > > On Tue, Mar 15, 2016 at 2:37 PM Jim Ingham <jing...@apple.com> wrote: > 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
