[llvm] r222302 - Make StringSet::insert return pair<iterator, bool> like other self-associative containers
Joe Groff
arcata at gmail.com
Wed Nov 19 07:09:57 PST 2014
Awesome, thanks David! This definitely helps make llvm's sets a bit less
weird.
On Wednesday, November 19, 2014, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Tue, Nov 18, 2014 at 9:45 PM, David Blaikie <dblaikie at gmail.com
> <javascript:_e(%7B%7D,'cvml','dblaikie at gmail.com');>> wrote:
>
>>
>>
>> On Tue, Nov 18, 2014 at 9:24 PM, Joe Groff <arcata at gmail.com
>> <javascript:_e(%7B%7D,'cvml','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
>>> <javascript:_e(%7B%7D,'cvml','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
>>>> <javascript:_e(%7B%7D,'cvml','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/998aac32/attachment.html>
More information about the llvm-commits
mailing list