Awesome, thanks David! This definitely helps make llvm's sets a bit less weird. <span></span> <br><br>On Wednesday, November 19, 2014, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 18, 2014 at 9:45 PM, David Blaikie <span dir="ltr"><<a href="javascript:_e(%7B%7D,'cvml','dblaikie@gmail.com');" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Nov 18, 2014 at 9:24 PM, Joe Groff <span dir="ltr"><<a href="javascript:_e(%7B%7D,'cvml','arcata@gmail.com');" target="_blank">arcata@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>This change makes it so that you can't use StringSet with SetVector:</div><div><br></div><div><p style="margin:0px;font-size:11px;font-family:Menlo">/Users/jgroff/src/s/llvm/include/llvm/ADT/SetVector.h:103:10: error: no viable conversion from 'std::pair<typename base::iterator, bool>' to 'bool'</p>
<p style="margin:0px;font-size:11px;font-family:Menlo"> bool result = set_.insert(X);</p>
<p style="margin:0px;font-size:11px;font-family:Menlo"> ^ ~~~~~~~~~~~~~~</p></div></div></blockquote></span><div><br>Are there any in-tree uses of this? I guess not.<br><br>But great - SmallSet has the same API oddity... will fix that to & then update SetVector so it's use is STL-compatible.<br></div></div></div></div></blockquote><div><br>Fixed in <span style="font-family:monospace">222334 - which involved changing SmallSet and SmallPtrSet's insert to return pair<iterator, bool> (well, in the case of SmallSet, since we don't have iterators, I just made it pair<NoneType, bool> for API compatibility, with a FIXME to add iterators at some point) - though this does involve... quite a bit of API cleanup/churn adding ".second" to many calls to insert. Sorry about that.</span><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span><font color="#888888"><div><br></div><div>-Joe</div></font></span><div><div><div><br></div>On Tue, Nov 18, 2014 at 6:56 PM, David Blaikie <span dir="ltr"><<a href="javascript:_e(%7B%7D,'cvml','dblaikie@gmail.com');" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: dblaikie<br>
Date: Tue Nov 18 20:56:00 2014<br>
New Revision: 222302<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222302&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222302&view=rev</a><br>
Log:<br>
Make StringSet::insert return pair<iterator, bool> like other self-associative containers<br>
<br>
StringSet is still a bit dodgy in that it exposes the raw iterator of<br>
the StringMap parent, which exposes the weird detail that StringSet<br>
actually has a 'value'... but anyway, this is useful for a handful of<br>
clients that want to reference the newly inserted/persistent string data<br>
in the StringSet/Map/Entry/thing.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/StringSet.h<br>
llvm/trunk/tools/opt/BreakpointPrinter.cpp<br>
llvm/trunk/utils/FileCheck/FileCheck.cpp<br>
llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/StringSet.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringSet.h?rev=222302&r1=222301&r2=222302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringSet.h?rev=222302&r1=222301&r2=222302&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/StringSet.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/StringSet.h Tue Nov 18 20:56:00 2014<br>
@@ -24,20 +24,9 @@ namespace llvm {<br>
typedef llvm::StringMap<char, AllocatorTy> base;<br>
public:<br>
<br>
- /// insert - Insert the specified key into the set. If the key already<br>
- /// exists in the set, return false and ignore the request, otherwise insert<br>
- /// it and return true.<br>
- bool insert(StringRef Key) {<br>
- // Get or create the map entry for the key; if it doesn't exist the value<br>
- // type will be default constructed which we use to detect insert.<br>
- //<br>
- // We use '+' as the sentinel value in the map.<br>
+ std::pair<typename base::iterator, bool> insert(StringRef Key) {<br>
assert(!Key.empty());<br>
- StringMapEntry<char> &Entry = this->GetOrCreateValue(Key);<br>
- if (Entry.getValue() == '+')<br>
- return false;<br>
- Entry.setValue('+');<br>
- return true;<br>
+ return base::insert(std::make_pair(Key, '\0'));<br>
}<br>
};<br>
}<br>
<br>
Modified: llvm/trunk/tools/opt/BreakpointPrinter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/BreakpointPrinter.cpp?rev=222302&r1=222301&r2=222302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/BreakpointPrinter.cpp?rev=222302&r1=222301&r2=222302&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/opt/BreakpointPrinter.cpp (original)<br>
+++ llvm/trunk/tools/opt/BreakpointPrinter.cpp Tue Nov 18 20:56:00 2014<br>
@@ -62,7 +62,7 @@ struct BreakpointPrinter : public Module<br>
continue;<br>
getContextName(SP.getContext().resolve(TypeIdentifierMap), Name);<br>
Name = Name + SP.getDisplayName().str();<br>
- if (!Name.empty() && Processed.insert(Name)) {<br>
+ if (!Name.empty() && Processed.insert(Name).second) {<br>
Out << Name << "\n";<br>
}<br>
}<br>
<br>
Modified: llvm/trunk/utils/FileCheck/FileCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/FileCheck/FileCheck.cpp?rev=222302&r1=222301&r2=222302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/FileCheck/FileCheck.cpp?rev=222302&r1=222301&r2=222302&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/FileCheck/FileCheck.cpp (original)<br>
+++ llvm/trunk/utils/FileCheck/FileCheck.cpp Tue Nov 18 20:56:00 2014<br>
@@ -1219,7 +1219,7 @@ static bool ValidateCheckPrefixes() {<br>
if (Prefix == "")<br>
return false;<br>
<br>
- if (!PrefixSet.insert(Prefix))<br>
+ if (!PrefixSet.insert(Prefix).second)<br>
return false;<br>
<br>
if (!ValidateCheckPrefix(Prefix))<br>
<br>
Modified: llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp?rev=222302&r1=222301&r2=222302&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp?rev=222302&r1=222301&r2=222302&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp (original)<br>
+++ llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp Tue Nov 18 20:56:00 2014<br>
@@ -454,7 +454,7 @@ static void FactorNodes(std::unique_ptr<<br>
SmallVector<std::pair<const SDNodeInfo*, Matcher*>, 8> Cases;<br>
for (unsigned i = 0, e = NewOptionsToMatch.size(); i != e; ++i) {<br>
CheckOpcodeMatcher *COM = cast<CheckOpcodeMatcher>(NewOptionsToMatch[i]);<br>
- assert(Opcodes.insert(COM->getOpcode().getEnumName()) &&<br>
+ assert(Opcodes.insert(COM->getOpcode().getEnumName()).second &&<br>
"Duplicate opcodes not factored?");<br>
Cases.push_back(std::make_pair(&COM->getOpcode(), COM->getNext()));<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="javascript:_e(%7B%7D,'cvml','llvm-commits@cs.uiuc.edu');" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>
</blockquote>