[llvm] r222302 - Make StringSet::insert return pair<iterator, bool> like other self-associative containers

[David Blaikie]

On Tue, Nov 18, 2014 at 9:45 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Nov 18, 2014 at 9:24 PM, Joe Groff <arcata at gmail.com> wrote:
>
>> This change makes it so that you can't use StringSet with SetVector:
>>
>> /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'
>>
>>     bool result = set_.insert(X);
>>
>>          ^        ~~~~~~~~~~~~~~
>>
>
> Are there any in-tree uses of this? I guess not.
>
> But great - SmallSet has the same API oddity... will fix that to & then
> update SetVector so it's use is STL-compatible.
>

Fixed in 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.


>
>
>>
>> -Joe
>>
>> On Tue, Nov 18, 2014 at 6:56 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> Author: dblaikie
>>> Date: Tue Nov 18 20:56:00 2014
>>> New Revision: 222302
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=222302&view=rev
>>> Log:
>>> Make StringSet::insert return pair<iterator, bool> like other
>>> self-associative containers
>>>
>>> StringSet is still a bit dodgy in that it exposes the raw iterator of
>>> the StringMap parent, which exposes the weird detail that StringSet
>>> actually has a 'value'... but anyway, this is useful for a handful of
>>> clients that want to reference the newly inserted/persistent string data
>>> in the StringSet/Map/Entry/thing.
>>>
>>> Modified:
>>>     llvm/trunk/include/llvm/ADT/StringSet.h
>>>     llvm/trunk/tools/opt/BreakpointPrinter.cpp
>>>     llvm/trunk/utils/FileCheck/FileCheck.cpp
>>>     llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/ADT/StringSet.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringSet.h?rev=222302&r1=222301&r2=222302&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ADT/StringSet.h (original)
>>> +++ llvm/trunk/include/llvm/ADT/StringSet.h Tue Nov 18 20:56:00 2014
>>> @@ -24,20 +24,9 @@ namespace llvm {
>>>      typedef llvm::StringMap<char, AllocatorTy> base;
>>>    public:
>>>
>>> -    /// insert - Insert the specified key into the set.  If the key
>>> already
>>> -    /// exists in the set, return false and ignore the request,
>>> otherwise insert
>>> -    /// it and return true.
>>> -    bool insert(StringRef Key) {
>>> -      // Get or create the map entry for the key; if it doesn't exist
>>> the value
>>> -      // type will be default constructed which we use to detect insert.
>>> -      //
>>> -      // We use '+' as the sentinel value in the map.
>>> +    std::pair<typename base::iterator, bool> insert(StringRef Key) {
>>>        assert(!Key.empty());
>>> -      StringMapEntry<char> &Entry = this->GetOrCreateValue(Key);
>>> -      if (Entry.getValue() == '+')
>>> -        return false;
>>> -      Entry.setValue('+');
>>> -      return true;
>>> +      return base::insert(std::make_pair(Key, '\0'));
>>>      }
>>>    };
>>>  }
>>>
>>> Modified: llvm/trunk/tools/opt/BreakpointPrinter.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/BreakpointPrinter.cpp?rev=222302&r1=222301&r2=222302&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/opt/BreakpointPrinter.cpp (original)
>>> +++ llvm/trunk/tools/opt/BreakpointPrinter.cpp Tue Nov 18 20:56:00 2014
>>> @@ -62,7 +62,7 @@ struct BreakpointPrinter : public Module
>>>            continue;
>>>          getContextName(SP.getContext().resolve(TypeIdentifierMap),
>>> Name);
>>>          Name = Name + SP.getDisplayName().str();
>>> -        if (!Name.empty() && Processed.insert(Name)) {
>>> +        if (!Name.empty() && Processed.insert(Name).second) {
>>>            Out << Name << "\n";
>>>          }
>>>        }
>>>
>>> Modified: llvm/trunk/utils/FileCheck/FileCheck.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/FileCheck/FileCheck.cpp?rev=222302&r1=222301&r2=222302&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/utils/FileCheck/FileCheck.cpp (original)
>>> +++ llvm/trunk/utils/FileCheck/FileCheck.cpp Tue Nov 18 20:56:00 2014
>>> @@ -1219,7 +1219,7 @@ static bool ValidateCheckPrefixes() {
>>>      if (Prefix == "")
>>>        return false;
>>>
>>> -    if (!PrefixSet.insert(Prefix))
>>> +    if (!PrefixSet.insert(Prefix).second)
>>>        return false;
>>>
>>>      if (!ValidateCheckPrefix(Prefix))
>>>
>>> Modified: llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp?rev=222302&r1=222301&r2=222302&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp (original)
>>> +++ llvm/trunk/utils/TableGen/DAGISelMatcherOpt.cpp Tue Nov 18 20:56:00
>>> 2014
>>> @@ -454,7 +454,7 @@ static void FactorNodes(std::unique_ptr<
>>>      SmallVector<std::pair<const SDNodeInfo*, Matcher*>, 8> Cases;
>>>      for (unsigned i = 0, e = NewOptionsToMatch.size(); i != e; ++i) {
>>>        CheckOpcodeMatcher *COM =
>>> cast<CheckOpcodeMatcher>(NewOptionsToMatch[i]);
>>> -      assert(Opcodes.insert(COM->getOpcode().getEnumName()) &&
>>> +      assert(Opcodes.insert(COM->getOpcode().getEnumName()).second &&
>>>               "Duplicate opcodes not factored?");
>>>        Cases.push_back(std::make_pair(&COM->getOpcode(),
>>> COM->getNext()));
>>>      }
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141119/fe8d646c/attachment.html>