<div dir="ltr">On Thu, May 9, 2013 at 1:54 AM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank" class="cremed">ruiu@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ruiu<br>
Date: Wed May 8 18:54:10 2013<br>
New Revision: 181492<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181492&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=181492&view=rev</a><br>
Log:<br>
[lld] Add comments to InputFiles::searchLibraries() arguments.<br></blockquote><div style><br></div><div style>I'm not sure this is really the best approach, and if it is it would be better to follow the conventions for comments on arguments in the rest of LLVM (unless LLD has *another* divergent convention here??).</div>
<div style><br></div><div style>Specifically, could we instead use the (very common in LLVM and Clang) pattern of:</div><div style><br></div><div style>foo(/*Arg1=*/ true, /*Arg2=*/ false, ...);</div><div style><br></div>
<div style><br></div><div style>Maybe even better would be to provide interfaces that don't require so many booleans. For example, here it seems like a bitmask enum would be nicely self-documenting?</div><div style><br>
</div><div style>_inputFiles.searchLibraries(undefName, SLK_SharedLibs | SLK_Archives | ...);</div><div style><br></div><div style>Just something to consider.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
lld/trunk/lib/Core/Resolver.cpp<br>
<br>
Modified: lld/trunk/lib/Core/Resolver.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=181492&r1=181491&r2=181492&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=181492&r1=181491&r2=181492&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/lib/Core/Resolver.cpp (original)<br>
+++ lld/trunk/lib/Core/Resolver.cpp Wed May 8 18:54:10 2013<br>
@@ -200,7 +200,11 @@ void Resolver::resolveUndefines() {<br>
StringRef undefName = undefAtom->name();<br>
// load for previous undefine may also have loaded this undefine<br>
if (!_symbolTable.isDefined(undefName)) {<br>
- _inputFiles.searchLibraries(undefName, true, true, false, *this);<br>
+ _inputFiles.searchLibraries(undefName,<br>
+ true, // searchSharedLibs<br>
+ true, // searchArchives<br>
+ false, // dataSymbolOnly<br>
+ *this);<br>
}<br>
}<br>
// search libraries for overrides of common symbols<br>
@@ -213,10 +217,13 @@ void Resolver::resolveUndefines() {<br>
const Atom *curAtom = _symbolTable.findByName(tentDefName);<br>
assert(curAtom != nullptr);<br>
if (const DefinedAtom* curDefAtom = dyn_cast<DefinedAtom>(curAtom)) {<br>
- if (curDefAtom->merge() == DefinedAtom::mergeAsTentative ) {<br>
+ if (curDefAtom->merge() == DefinedAtom::mergeAsTentative) {<br>
// Still tentative definition, so look for override.<br>
- _inputFiles.searchLibraries(tentDefName, searchSharedLibs,<br>
- searchArchives, true, *this);<br>
+ _inputFiles.searchLibraries(tentDefName,<br>
+ searchSharedLibs,<br>
+ searchArchives,<br>
+ true, // dataSymbolOnly<br>
+ *this);<br>
}<br>
}<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>