<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="mailto: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 class="">On Tue, Nov 18, 2014 at 9:24 PM, Joe Groff <span dir="ltr"><<a href="mailto: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 class="h5"><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="mailto: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="mailto: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>