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