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

David Blaikie dblaikie at gmail.com
Tue Nov 18 21:45:08 PST 2014


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.


>
> -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/20141118/4ebdc1c5/attachment.html>


More information about the llvm-commits mailing list