ENH: prevent conversion of string to regExp in stringListOps (closes #739)

* For most cases, this conversion would be largely unintentional
  and also less efficient. If the regex is desirable, the caller
  should invoke it explicitly.
  For example,

      findStrings(regExp(str), listOfStrings);

  Or use one of the keyType, wordRe, wordRes variants instead.
  If string is to be used as a plain (non-regex) matcher,
  this can be directly invoked

      findMatchingStrings(str, listOfStrings);

  or using the ListOps instead:

      findIndices(listOfStrings, str);

* provide function interfaces for keyType.
This commit is contained in:
Mark Olesen
2018-02-22 09:28:03 +01:00
parent f959927910
commit ec38e7a408
16 changed files with 264 additions and 226 deletions

View File

@ -53,7 +53,7 @@ int main(int argc, char *argv[])
Info<< "stringList " << strLst << nl; Info<< "stringList " << strLst << nl;
labelList matches = findStrings(".*ee.*", strLst); labelList matches = findStrings(regExp(".*ee.*"), strLst);
Info<< "matches found for regexp .*ee.* :" << nl << matches << nl; Info<< "matches found for regexp .*ee.* :" << nl << matches << nl;
forAll(matches, i) forAll(matches, i)
@ -71,7 +71,7 @@ int main(int argc, char *argv[])
} }
Info<< endl; Info<< endl;
stringList subLst = subsetStrings(".*ee.*", strLst); stringList subLst = subsetStrings(regExp(".*ee.*"), strLst);
Info<< "subset stringList: " << subLst << nl; Info<< "subset stringList: " << subLst << nl;
subLst = subsetStrings(reLst, strLst); subLst = subsetStrings(reLst, strLst);
@ -80,7 +80,7 @@ int main(int argc, char *argv[])
inplaceSubsetStrings(reLst, strLst); inplaceSubsetStrings(reLst, strLst);
Info<< "subsetted stringList: " << strLst << nl; Info<< "subsetted stringList: " << strLst << nl;
inplaceSubsetStrings(".*l.*", strLst); inplaceSubsetStrings(regExp(".*l.*"), strLst);
Info<< "subsetted stringList: " << strLst << nl; Info<< "subsetted stringList: " << strLst << nl;
Info<< "\nEnd\n" << endl; Info<< "\nEnd\n" << endl;

View File

@ -178,7 +178,6 @@ bool addEntry
} }
// List of indices into thisKeys // List of indices into thisKeys
labelList findMatches labelList findMatches
( (
@ -194,19 +193,19 @@ labelList findMatches
{ {
// Wildcard match // Wildcard match
matches = findStrings(key, thisKeys); matches = findStrings(key, thisKeys);
} }
else if (shortcuts.size()) else if (shortcuts.size())
{ {
// See if patchGroups expand to valid thisKeys // See if patchGroups expand to valid thisKeys
labelList indices = findStrings(key, shortcutNames); labelList indices = findStrings(key, shortcutNames);
forAll(indices, i)
for (const label idx : indices)
{ {
const word& name = shortcutNames[indices[i]]; const word& name = shortcutNames[idx];
const wordList& keys = shortcuts[name]; const wordList& keys = shortcuts[name];
forAll(keys, j) forAll(keys, j)
{ {
label index = thisKeys.find(keys[j]); const label index = thisKeys.find(keys[j]);
if (index != -1) if (index != -1)
{ {
matches.append(index); matches.append(index);

View File

@ -137,6 +137,7 @@ int main(int argc, char *argv[])
labelList regionIDs = labelList regionIDs =
findStrings(regionName, surf.regions()); findStrings(regionName, surf.regions());
if (modifier().modify(regionIDs, surf)) if (modifier().modify(regionIDs, surf))
{ {
changed = true; changed = true;

View File

@ -576,63 +576,65 @@ Foam::labelList Foam::polyBoundaryMesh::findIndices
{ {
DynamicList<label> indices; DynamicList<label> indices;
if (!key.empty()) if (key.empty())
{ {
if (key.isPattern()) // no-op
}
else if (key.isPattern())
{
const regExp keyRe(key);
indices = findStrings(keyRe, this->names());
if (usePatchGroups && groupPatchIDs().size())
{ {
indices = findStrings(key, this->names()); labelHashSet indexSet(indices);
if (usePatchGroups && groupPatchIDs().size()) const wordList allGroupNames = groupPatchIDs().toc();
labelList groupIDs = findStrings(keyRe, allGroupNames);
forAll(groupIDs, i)
{ {
labelHashSet indexSet(indices); const word& grpName = allGroupNames[groupIDs[i]];
const labelList& patchIDs = groupPatchIDs()[grpName];
const wordList allGroupNames = groupPatchIDs().toc(); forAll(patchIDs, j)
labelList groupIDs = findStrings(key, allGroupNames);
forAll(groupIDs, i)
{ {
const word& grpName = allGroupNames[groupIDs[i]]; if (indexSet.insert(patchIDs[j]))
const labelList& patchIDs = groupPatchIDs()[grpName];
forAll(patchIDs, j)
{ {
if (indexSet.insert(patchIDs[j])) indices.append(patchIDs[j]);
{
indices.append(patchIDs[j]);
}
} }
} }
} }
} }
else }
else
{
// Literal string. Special version of above to avoid
// unnecessary memory allocations
indices.setCapacity(1);
forAll(*this, i)
{ {
// Literal string. Special version of above to avoid if (key == operator[](i).name())
// unnecessary memory allocations
indices.setCapacity(1);
forAll(*this, i)
{ {
if (key == operator[](i).name()) indices.append(i);
{ break;
indices.append(i);
break;
}
} }
}
if (usePatchGroups && groupPatchIDs().size()) if (usePatchGroups && groupPatchIDs().size())
{
const auto iter = groupPatchIDs().cfind(key);
if (iter.found())
{ {
const HashTable<labelList>::const_iterator iter = labelHashSet indexSet(indices);
groupPatchIDs().find(key);
if (iter.found()) const labelList& patchIDs = iter();
forAll(patchIDs, j)
{ {
labelHashSet indexSet(indices); if (indexSet.insert(patchIDs[j]))
const labelList& patchIDs = iter();
forAll(patchIDs, j)
{ {
if (indexSet.insert(patchIDs[j])) indices.append(patchIDs[j]);
{
indices.append(patchIDs[j]);
}
} }
} }
} }
@ -645,26 +647,27 @@ Foam::labelList Foam::polyBoundaryMesh::findIndices
Foam::label Foam::polyBoundaryMesh::findIndex(const keyType& key) const Foam::label Foam::polyBoundaryMesh::findIndex(const keyType& key) const
{ {
if (!key.empty()) if (key.empty())
{ {
if (key.isPattern()) // no-op
{ }
labelList indices = this->findIndices(key); else if (key.isPattern())
{
labelList indices = this->findIndices(key);
// return first element // return first element
if (!indices.empty()) if (!indices.empty())
{
return indices[0];
}
}
else
{ {
forAll(*this, i) return indices[0];
}
}
else
{
forAll(*this, i)
{
if (key == operator[](i).name())
{ {
if (key == operator[](i).name()) return i;
{
return i;
}
} }
} }
} }

View File

@ -342,25 +342,26 @@ Foam::labelList Foam::ZoneMesh<ZoneType, MeshType>::findIndices
{ {
labelList indices; labelList indices;
if (!key.empty()) if (key.empty())
{ {
if (key.isPattern()) // no-op
}
else if (key.isPattern())
{
indices = findStrings(key, this->names());
}
else
{
indices.setSize(this->size());
label count = 0;
forAll(*this, i)
{ {
indices = findStrings(key, this->names()); if (key == operator[](i).name())
}
else
{
indices.setSize(this->size());
label count = 0;
forAll(*this, i)
{ {
if (key == operator[](i).name()) indices[count++] = i;
{
indices[count++] = i;
}
} }
indices.setSize(count);
} }
indices.setSize(count);
} }
return indices; return indices;
@ -373,26 +374,26 @@ Foam::label Foam::ZoneMesh<ZoneType, MeshType>::findIndex
const keyType& key const keyType& key
) const ) const
{ {
if (!key.empty()) if (key.empty())
{ {
if (key.isPattern()) // no-op
{ }
labelList indices = this->findIndices(key); else if (key.isPattern())
{
labelList indices = this->findIndices(key);
// return first element if (!indices.empty())
if (!indices.empty())
{
return indices[0];
}
}
else
{ {
forAll(*this, i) return indices.first(); // first match
}
}
else
{
forAll(*this, i)
{
if (key == operator[](i).name())
{ {
if (key == operator[](i).name()) return i;
{
return i;
}
} }
} }
} }
@ -412,7 +413,7 @@ Foam::label Foam::ZoneMesh<ZoneType, MeshType>::findZoneID
forAll(zones, zonei) forAll(zones, zonei)
{ {
if (zones[zonei].name() == zoneName) if (zoneName == zones[zonei].name())
{ {
return zonei; return zonei;
} }

View File

@ -43,7 +43,7 @@ SourceFiles
namespace Foam namespace Foam
{ {
//- Extract list indices //- Extract list indices for all matches.
// The unary match predicate has the following signature: // The unary match predicate has the following signature:
// \code // \code
// bool operator()(const std::string& text); // bool operator()(const std::string& text);
@ -78,26 +78,17 @@ namespace Foam
template<class StringType> template<class StringType>
labelList findStrings labelList findStrings
( (
const char* re, const keyType& matcher,
const UList<StringType>& input, const UList<StringType>& input,
const bool invert=false const bool invert=false
) )
{ {
return findMatchingStrings(regExp(re), input, invert); return
} (
matcher.isPattern()
? findMatchingStrings(regExp(matcher), input, invert)
//- Return list indices for strings matching the regular expression : findMatchingStrings(matcher, input, invert)
// Template partial specialization of findMatchingStrings );
template<class StringType>
labelList findStrings
(
const std::string& re,
const UList<StringType>& input,
const bool invert=false
)
{
return findMatchingStrings(regExp(re), input, invert);
} }
@ -178,25 +169,17 @@ namespace Foam
template<class StringListType> template<class StringListType>
StringListType subsetStrings StringListType subsetStrings
( (
const char* re, const keyType& matcher,
const StringListType& input, const StringListType& input,
const bool invert=false const bool invert=false
) )
{ {
return subsetMatchingStrings(regExp(re), input, invert); return
} (
matcher.isPattern()
//- Extract elements of StringList when regular expression matches ? subsetMatchingStrings(regExp(matcher), input, invert)
// Template partial specialization of subsetMatchingStrings : subsetMatchingStrings(matcher, input, invert)
template<class StringListType> );
StringListType subsetStrings
(
const std::string& re,
const StringListType& input,
const bool invert=false
)
{
return subsetMatchingStrings(regExp(re), input, invert);
} }
//- Extract elements of StringList when regular expression matches //- Extract elements of StringList when regular expression matches
@ -265,30 +248,22 @@ namespace Foam
inplaceSubsetMatchingStrings(matcher, input, invert); inplaceSubsetMatchingStrings(matcher, input, invert);
} }
//- Inplace extract elements of StringList when regular expression matches //- Extract elements of StringList when regular expression matches
// Template partial specialization of inplaceSubsetMatchingStrings // Template partial specialization of subsetMatchingStrings
template<class StringListType> template<class StringListType>
void inplaceSubsetStrings void inplaceSubsetStrings
( (
const char* re, const keyType& matcher,
StringListType& input, StringListType& input,
const bool invert=false const bool invert=false
) )
{ {
inplaceSubsetMatchingStrings(regExp(re), input, invert); return
} (
matcher.isPattern()
//- Inplace extract elements of StringList when regular expression matches ? inplaceSubsetMatchingStrings(regExp(matcher), input, invert)
// Template partial specialization of inplaceSubsetMatchingStrings : inplaceSubsetMatchingStrings(matcher, input, invert)
template<class StringListType> );
void inplaceSubsetStrings
(
const std::string& re,
StringListType& input,
const bool invert=false
)
{
inplaceSubsetMatchingStrings(regExp(re), input, invert);
} }
//- Inplace extract elements of StringList when regular expression matches //- Inplace extract elements of StringList when regular expression matches
@ -330,6 +305,79 @@ namespace Foam
inplaceSubsetMatchingStrings(wordRes::matcher(regexs), input, invert); inplaceSubsetMatchingStrings(wordRes::matcher(regexs), input, invert);
} }
//- Find using C-string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use regExp, keyType, wordRe instead, or findMatchingStrings()
template<class StringType>
labelList findStrings
(
const char* disallowed,
const UList<StringType>& input,
const bool invert=false
) = delete;
//- Find using string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use regExp, keyType, wordRe instead, or findMatchingStrings()
template<class StringType>
labelList findStrings
(
const std::string& disallowed,
const UList<StringType>& input,
const bool invert=false
) = delete;
//- Subset using C-string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use regExp, keyType, wordRe instead, or subsetMatchingStrings()
template<class StringListType>
StringListType subsetStrings
(
const char* disallowed,
const StringListType& input,
const bool invert=false
) = delete;
//- Subset using string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use regExp, keyType, wordRe instead, or subsetMatchingStrings()
template<class StringListType>
StringListType subsetStrings
(
const std::string& disallowed,
const StringListType& input,
const bool invert=false
) = delete;
//- Subset using C-string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use regExp, keyType, wordRe instead, or inplaceSubsetMatchingStrings()
template<class StringListType>
void inplaceSubsetStrings
(
const char* disallowed,
StringListType& input,
const bool invert=false
) = delete;
//- Subset using string as a regex
// \deprecated (FEB-2018) Treating string as regex may be inefficient
// and lead to unintended results.
// Use keyType, wordRe instead, or inplaceSubsetMatchingStrings()
template<class StringListType>
void inplaceSubsetStrings
(
const std::string& disallowed,
StringListType& input,
const bool invert=false
) = delete;
} }

View File

@ -149,11 +149,7 @@ void Foam::ensightMesh::correct()
if (!matcher.empty()) if (!matcher.empty())
{ {
useAll = false; useAll = false;
matched = findMatchingStrings matched = findStrings(matcher, patchNames);
(
matcher,
patchNames
);
} }
} }
@ -248,11 +244,7 @@ void Foam::ensightMesh::correct()
const wordRes& matcher = option().faceZoneSelection(); const wordRes& matcher = option().faceZoneSelection();
wordList selectZones = mesh_.faceZones().names(); wordList selectZones = mesh_.faceZones().names();
inplaceSubsetMatchingStrings subsetMatchingStrings(matcher, selectZones);
(
matcher,
selectZones
);
// have same order as later with sortedToc() // have same order as later with sortedToc()
Foam::sort(selectZones); Foam::sort(selectZones);

View File

@ -194,25 +194,26 @@ Foam::labelList Foam::faBoundaryMesh::findIndices
{ {
DynamicList<label> indices; DynamicList<label> indices;
if (!key.empty()) if (key.empty())
{ {
if (key.isPattern()) // no-op
{ }
indices = findStrings(key, this->names()); else if (key.isPattern())
} {
else indices = findStrings(key, this->names());
{ }
// Literal string. Special version of above to avoid else
// unnecessary memory allocations {
// Literal string. Special version of above to avoid
// unnecessary memory allocations
indices.setCapacity(1); indices.setCapacity(1);
forAll(*this, i) forAll(*this, i)
{
if (key == operator[](i).name())
{ {
if (key == operator[](i).name()) indices.append(i);
{ break;
indices.append(i);
break;
}
} }
} }
} }

View File

@ -77,27 +77,27 @@ Foam::ParticleErosion<CloudType>::ParticleErosion
psi_(this->coeffDict().template lookupOrDefault<scalar>("psi", 2.0)), psi_(this->coeffDict().template lookupOrDefault<scalar>("psi", 2.0)),
K_(this->coeffDict().template lookupOrDefault<scalar>("K", 2.0)) K_(this->coeffDict().template lookupOrDefault<scalar>("K", 2.0))
{ {
const wordList allPatchNames = owner.mesh().boundaryMesh().names(); const wordList allPatchNames(owner.mesh().boundaryMesh().names());
wordList patchName(this->coeffDict().lookup("patches")); const wordReList patchNames(this->coeffDict().lookup("patches"));
labelHashSet uniquePatchIDs; labelHashSet uniqIds;
forAllReverse(patchName, i) for (const wordRe& re : patchNames)
{ {
labelList patchIDs = findStrings(patchName[i], allPatchNames); labelList ids = findStrings(re, allPatchNames);
if (patchIDs.empty()) if (ids.empty())
{ {
WarningInFunction WarningInFunction
<< "Cannot find any patch names matching " << patchName[i] << "Cannot find any patch names matching " << re
<< endl; << endl;
} }
uniquePatchIDs.insert(patchIDs); uniqIds.insert(ids);
} }
patchIDs_ = uniquePatchIDs.toc(); patchIDs_ = uniqIds.sortedToc();
// trigger ther creation of the Q field // trigger creation of the Q field
preEvolve(); preEvolve();
} }
@ -117,13 +117,6 @@ Foam::ParticleErosion<CloudType>::ParticleErosion
{} {}
// * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * //
template<class CloudType>
Foam::ParticleErosion<CloudType>::~ParticleErosion()
{}
// * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * //
template<class CloudType> template<class CloudType>

View File

@ -120,7 +120,7 @@ public:
//- Destructor //- Destructor
virtual ~ParticleErosion(); virtual ~ParticleErosion() = default;
// Member Functions // Member Functions

View File

@ -134,33 +134,32 @@ Foam::PatchPostProcessing<CloudType>::PatchPostProcessing
times_(), times_(),
patchData_() patchData_()
{ {
const wordList allPatchNames = owner.mesh().boundaryMesh().names(); const wordList allPatchNames(owner.mesh().boundaryMesh().names());
wordList patchName(this->coeffDict().lookup("patches")); const wordReList patchNames(this->coeffDict().lookup("patches"));
labelHashSet uniquePatchIDs; labelHashSet uniqIds;
forAllReverse(patchName, i) for (const wordRe& re : patchNames)
{ {
labelList patchIDs = findStrings(patchName[i], allPatchNames); labelList ids = findStrings(re, allPatchNames);
if (patchIDs.empty()) if (ids.empty())
{ {
WarningInFunction WarningInFunction
<< "Cannot find any patch names matching " << patchName[i] << "Cannot find any patch names matching " << re
<< endl; << endl;
} }
uniquePatchIDs.insert(patchIDs); uniqIds.insert(ids);
} }
patchIDs_ = uniquePatchIDs.toc(); patchIDs_ = uniqIds.sortedToc();
if (debug) if (debug)
{ {
forAll(patchIDs_, i) for (const label patchi : patchIDs_)
{ {
const label patchi = patchIDs_[i]; Info<< "Post-process patch "
const word& patchName = owner.mesh().boundaryMesh()[patchi].name(); << owner.mesh().boundaryMesh()[patchi].name() << endl;
Info<< "Post-process patch " << patchName << endl;
} }
} }
@ -183,13 +182,6 @@ Foam::PatchPostProcessing<CloudType>::PatchPostProcessing
{} {}
// * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * //
template<class CloudType>
Foam::PatchPostProcessing<CloudType>::~PatchPostProcessing()
{}
// * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * //
template<class CloudType> template<class CloudType>

View File

@ -115,7 +115,7 @@ public:
//- Destructor //- Destructor
virtual ~PatchPostProcessing(); virtual ~PatchPostProcessing() = default;
// Member Functions // Member Functions

View File

@ -46,22 +46,22 @@ Foam::patchInteractionDataList::patchInteractionDataList
patchGroupIDs_(this->size()) patchGroupIDs_(this->size())
{ {
const polyBoundaryMesh& bMesh = mesh.boundaryMesh(); const polyBoundaryMesh& bMesh = mesh.boundaryMesh();
const wordList allPatchNames = bMesh.names(); const wordList allPatchNames(bMesh.names());
const List<patchInteractionData>& items = *this; const List<patchInteractionData>& items = *this;
forAllReverse(items, i) forAllReverse(items, i)
{ {
const word& patchName = items[i].patchName(); const word& patchName = items[i].patchName();
labelList patchIDs = findStrings(patchName, allPatchNames); labelList ids = findIndices(allPatchNames, patchName);
if (patchIDs.empty()) if (ids.empty())
{ {
WarningInFunction WarningInFunction
<< "Cannot find any patch names matching " << patchName << "Cannot find any patch names matching " << patchName
<< endl; << endl;
} }
patchGroupIDs_[i].transfer(patchIDs); patchGroupIDs_[i].transfer(ids);
} }
// Check that all patches are specified // Check that all patches are specified

View File

@ -101,22 +101,26 @@ const Foam::coordinateSystems& Foam::coordinateSystems::New
Foam::labelList Foam::coordinateSystems::findIndices(const keyType& key) const Foam::labelList Foam::coordinateSystems::findIndices(const keyType& key) const
{ {
labelList indices; labelList indices;
if (key.isPattern()) if (key.empty())
{ {
indices = findStrings(key, toc()); // no-op
}
else if (key.isPattern())
{
indices = findStrings(key, this->toc());
} }
else else
{ {
indices.setSize(size()); indices.setSize(this->size());
label nFound = 0; label count = 0;
forAll(*this, i) forAll(*this, i)
{ {
if (key == operator[](i).name()) if (key == operator[](i).name())
{ {
indices[nFound++] = i; indices[count++] = i;
} }
} }
indices.setSize(nFound); indices.setSize(count);
} }
return indices; return indices;
@ -125,13 +129,16 @@ Foam::labelList Foam::coordinateSystems::findIndices(const keyType& key) const
Foam::label Foam::coordinateSystems::findIndex(const keyType& key) const Foam::label Foam::coordinateSystems::findIndex(const keyType& key) const
{ {
if (key.isPattern()) if (key.empty())
{
// no-op
}
else if (key.isPattern())
{ {
labelList indices = findIndices(key); labelList indices = findIndices(key);
// return first element
if (!indices.empty()) if (!indices.empty())
{ {
return indices[0]; return indices.first(); // first match
} }
} }
else else
@ -145,6 +152,7 @@ Foam::label Foam::coordinateSystems::findIndex(const keyType& key) const
} }
} }
// Not found
return -1; return -1;
} }

View File

@ -120,12 +120,12 @@ Foam::label Foam::triSurfaceLoader::select(const wordRe& mat)
if (mat.isPattern()) if (mat.isPattern())
{ {
foundIds = findMatchingStrings(mat, available_); foundIds = findStrings(mat, available_);
sort(foundIds); sort(foundIds);
} }
else else
{ {
const word& plain = static_cast<const word&>(mat); const word& plain = mat;
if (available_.found(plain)) if (available_.found(plain))
{ {
foundIds.append(available_[plain]); foundIds.append(available_[plain]);
@ -164,7 +164,7 @@ Foam::label Foam::triSurfaceLoader::select(const UList<wordRe>& matcher)
{ {
if (mat.isPattern()) if (mat.isPattern())
{ {
labelList indices = findMatchingStrings(mat, available_); labelList indices = findStrings(mat, available_);
sort(indices); sort(indices);
for (const label idx : indices) for (const label idx : indices)
@ -177,7 +177,7 @@ Foam::label Foam::triSurfaceLoader::select(const UList<wordRe>& matcher)
} }
else else
{ {
const word& plain = static_cast<const word&>(mat); const word& plain = mat;
if (available_.found(plain)) if (available_.found(plain))
{ {
const label idx = available_[plain]; const label idx = available_[plain];

View File

@ -249,7 +249,7 @@ curvatureSeparation::curvatureSeparation
forAllReverse(prIn, i) forAllReverse(prIn, i)
{ {
labelList patchIDs = findStrings(prIn[i].first(), allPatchNames); labelList patchIDs = findIndices(allPatchNames, prIn[i].first());
forAll(patchIDs, j) forAll(patchIDs, j)
{ {
const label patchi = patchIDs[j]; const label patchi = patchIDs[j];