Skip to content

[NFC] Replace uses of find(x) != end() idiom with contains(x) for StringRef and Set-like types #35229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions include/swift/IDE/APIDigesterData.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ struct CommonDiffItem: public APIDiffItem {

bool rightCommentUnderscored() const {
DeclNameViewer Viewer(RightComment);
auto HasUnderScore =
[](StringRef S) { return S.find('_') != StringRef::npos; };
auto HasUnderScore = [](StringRef S) { return S.contains('_'); };
auto Args = Viewer.args();
return HasUnderScore(Viewer.base()) ||
std::any_of(Args.begin(), Args.end(), HasUnderScore);
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ class swift::MemberLookupTable {
/// Returns \c true if the lookup table has a complete accounting of the
/// given name.
bool isLazilyComplete(DeclBaseName name) const {
return LazilyCompleteNames.find(name) != LazilyCompleteNames.end();
return LazilyCompleteNames.contains(name);
}

/// Mark a given lazily-loaded name as being complete.
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ImportMacro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ static ValueDecl *importNumericLiteral(ClangImporter::Implementation &Impl,
Impl.getClangPreprocessor().getSpelling(tok, SpellingBuffer, &Invalid);
if (Invalid)
return nullptr;
if (TokSpelling.find('_') != StringRef::npos)
if (TokSpelling.contains('_'))
return nullptr;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ static void validateCompilationConditionArgs(DiagnosticEngine &diags,
const ArgList &args) {
for (const Arg *A : args.filtered(options::OPT_D)) {
StringRef name = A->getValue();
if (name.find('=') != StringRef::npos) {
if (name.contains('=')) {
diags.diagnose(SourceLoc(),
diag::cannot_assign_value_to_conditional_compilation_flag,
name);
Expand Down Expand Up @@ -660,7 +660,7 @@ static SmallVector<StringRef, 8> findRemovedInputs(
SmallVector<StringRef, 8> missingInputs;
for (auto &previousInput : previousInputs) {
auto previousInputArg = previousInput.getKey();
if (inputArgs.find(previousInputArg) == inputArgs.end()) {
if (!inputArgs.contains(previousInputArg)) {
missingInputs.push_back(previousInputArg);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Frontend/PrintingDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ static void annotateSnippetWithInfo(SourceManager &SM,
for (auto fixIt : Info.FixIts) {
// Don't print multi-line fix-its inline, only include them at the end of
// the message.
if (fixIt.getText().find("\n") == std::string::npos)
if (!fixIt.getText().contains("\n"))
Snippet.addFixIt(fixIt.getRange(), fixIt.getText());
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/FrontendTool/TBD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static bool validateSymbols(DiagnosticEngine &diags,
irNotTBD.push_back(unmangledName);
}
} else {
assert(symbolSet.find(name) == symbolSet.end() &&
assert(!symbolSet.contains(name) &&
"non-global value in value symbol table");
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/ModuleInterfacePrinting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ getDeclsFromCrossImportOverlay(ModuleDecl *Overlay, ModuleDecl *Declaring,
return false;

// Ignore an imports of modules also imported by the underlying module.
if (PrevImported.find(Imported) != PrevImported.end())
if (PrevImported.contains(Imported))
return false;
}
if (auto *VD = dyn_cast<ValueDecl>(D)) {
Expand Down
3 changes: 1 addition & 2 deletions lib/IDE/SyntaxModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1620,8 +1620,7 @@ class DocFieldParser {

public:
DocFieldParser(StringRef text) : ptr(text.begin()), end(text.end()) {
assert(text.rtrim().find('\n') == StringRef::npos &&
"expected single line");
assert(!text.rtrim().contains('\n') && "expected single line");
}

// Case-insensitively match one of the following patterns:
Expand Down
4 changes: 2 additions & 2 deletions lib/IDE/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,8 @@ void ide::collectModuleNames(StringRef SDKPath,
std::vector<std::string> &Modules) {
std::string SDKName = getSDKName(SDKPath);
std::string lowerSDKName = StringRef(SDKName).lower();
bool isOSXSDK = StringRef(lowerSDKName).find("macosx") != StringRef::npos;
bool isDeviceOnly = StringRef(lowerSDKName).find("iphoneos") != StringRef::npos;
bool isOSXSDK = StringRef(lowerSDKName).contains("macosx");
bool isDeviceOnly = StringRef(lowerSDKName).contains("iphoneos");
auto Mods = isOSXSDK ? getOSXModuleList() : getiOSModuleList();
Modules.insert(Modules.end(), Mods.begin(), Mods.end());
if (isDeviceOnly) {
Expand Down
4 changes: 2 additions & 2 deletions lib/IRGen/IRGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ llvm::SmallString<32> getTargetDependentLibraryOption(const llvm::Triple &T,
llvm::SmallString<32> buffer;

if (T.isWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment()) {
bool quote = library.find(' ') != StringRef::npos;
bool quote = library.contains(' ');

buffer += "/DEFAULTLIB:";
if (quote)
Expand All @@ -1174,7 +1174,7 @@ llvm::SmallString<32> getTargetDependentLibraryOption(const llvm::Triple &T,
if (quote)
buffer += '"';
} else if (T.isPS4()) {
bool quote = library.find(' ') != StringRef::npos;
bool quote = library.contains(' ');

buffer += "\01";
if (quote)
Expand Down
3 changes: 1 addition & 2 deletions lib/LLVMPasses/ARCEntryPointBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ class ARCEntryPointBuilder {
// intrinsics are atomic today.
if (I->getIntrinsicID() != llvm::Intrinsic::not_intrinsic)
return false;
return (I->getCalledFunction()->getName().find("nonatomic") !=
llvm::StringRef::npos);
return (I->getCalledFunction()->getName().contains("nonatomic"));
}

bool isAtomic(CallInst *I) {
Expand Down
6 changes: 3 additions & 3 deletions lib/LLVMPasses/LLVMARCOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ static bool canonicalizeInputFunction(Function &F, ARCEntryPointBuilder &B,
// Have not encountered a strong retain/release. keep it in the
// unknown retain/release list for now. It might get replaced
// later.
if (NativeRefs.find(ArgVal) == NativeRefs.end()) {
UnknownObjectRetains[ArgVal].push_back(&CI);
if (!NativeRefs.contains(ArgVal)) {
Copy link
Contributor

@LucianoPAlmeida LucianoPAlmeida Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the build is failing becausellvm::DenseSet doesn't have a contains method, @gottesmm we normally use count so perhaps we could use this here as well? :)

Copy link
Contributor Author

@mininny mininny Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the change to llvm::DenseSet hasn't landed on swift/main branch so it's causing this issue..? The contains method exists on llvm-project's apple/stable/20210107 branch; I'm not sure how the CI works but can this be fixed by moving the base branch? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems like it hasn't ... I would say that use count instead for Set like would just be fine and already an improvement, but since the whole point of the SR is to use contains I'm not really sure about that, so cc @varungandhi-apple @gottesmm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I seem to have made a mistake here. I looked at the git log and I saw that contains was added for SmallPtrSet, so I figured methods for all the Set-like data types were available on swift/main, since they were merged in succession to LLVM's main branch. However, looks like it's only present for SmallPtrSet on swift/main. Sorry about that.

We can wait for a couple of weeks; I'll ping this PR when we have the rebranch so we can run tests and land the change. I don't think cherry-picking changes onto other branches is the right fix here, as that might lead to more merge conflicts.

UnknownObjectRetains[ArgVal].push_back(&CI);
} else {
B.setInsertPoint(&CI);
B.createRetain(ArgVal, &CI);
Expand Down Expand Up @@ -189,7 +189,7 @@ static bool canonicalizeInputFunction(Function &F, ARCEntryPointBuilder &B,
// Have not encountered a strong retain/release. keep it in the
// unknown retain/release list for now. It might get replaced
// later.
if (NativeRefs.find(ArgVal) == NativeRefs.end()) {
if (!NativeRefs.contains(ArgVal)) {
UnknownObjectReleases[ArgVal].push_back(&CI);
} else {
B.setInsertPoint(&CI);
Expand Down
5 changes: 2 additions & 3 deletions lib/Migrator/APIDiffMigratorPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1358,8 +1358,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
llvm::raw_svector_ostream OS(Buffer);
if (swift::ide::printValueDeclUSR(OD, OS))
return SourceLoc();
return OverridingRemoveNames.find(OS.str()) == OverridingRemoveNames.end() ?
SourceLoc() : OverrideLoc;
return OverridingRemoveNames.contains(OS.str()) ? OverrideLoc : SourceLoc();
}

struct SuperRemoval: public ASTWalker {
Expand All @@ -1380,7 +1379,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
auto *RD = DSC->getFn()->getReferencedDecl().getDecl();
if (swift::ide::printValueDeclUSR(RD, OS))
return false;
return USRs.find(OS.str()) != USRs.end();
return USRs.contains(OS.str());
}
}
// We should handle try super.foo() too.
Expand Down
2 changes: 1 addition & 1 deletion lib/PrintAsObjC/DeclAndTypePrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ static bool isClangKeyword(Identifier name) {

if (name.empty())
return false;
return keywords.find(name.str()) != keywords.end();
return keywords.contains(name.str());
}


Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Analysis/ARCAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ findMatchingRetains(SILBasicBlock *BB) {
SA = nullptr;

for (auto X : R.first->getPredecessorBlocks()) {
if (HandledBBs.find(X) != HandledBBs.end())
if (HandledBBs.contains(X))
continue;
// Try to use the predecessor edge-value.
if (SA && SA->getIncomingPhiValue(X)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Analysis/EpilogueARCAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ void EpilogueARCContext::initializeDataflow() {
SILValue CArg = ToProcess.pop_back_val();
if (!CArg)
continue;
if (Processed.find(CArg) != Processed.end())
continue;
if (Processed.contains(CArg))
continue;
Processed.insert(CArg);
if (auto *A = dyn_cast<SILPhiArgument>(CArg)) {
// Find predecessor and break the SILArgument to predecessors.
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ static void removeToken(SILValue Op) {
if (!(GAI->use_empty() || GAI->hasOneUse()))
return;
// If it is not a *_token global variable, bail.
if (!Global || Global->getName().find("_token") == StringRef::npos)
if (!Global || !Global->getName().contains("_token"))
return;
GAI->getModule().eraseGlobalVariable(Global);
GAI->replaceAllUsesWithUndef();
Expand Down
22 changes: 8 additions & 14 deletions lib/SILOptimizer/PassManager/PassManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,11 @@ static bool doPrintBefore(SILTransform *T, SILFunction *F) {
return false;

if (!SILPrintOnlyFuns.empty() && F &&
F->getName().find(SILPrintOnlyFuns, 0) == StringRef::npos)
!F->getName().contains(SILPrintOnlyFuns))
return false;

auto MatchFun = [&](const std::string &Str) -> bool {
return T->getTag().find(Str) != StringRef::npos
|| T->getID().find(Str) != StringRef::npos;
return T->getTag().contains(Str) || T->getID().contains(Str);
};

if (SILPrintBefore.end() !=
Expand All @@ -188,12 +187,11 @@ static bool doPrintAfter(SILTransform *T, SILFunction *F, bool Default) {
return false;

if (!SILPrintOnlyFuns.empty() && F &&
F->getName().find(SILPrintOnlyFuns, 0) == StringRef::npos)
!F->getName().contains(SILPrintOnlyFuns))
return false;

auto MatchFun = [&](const std::string &Str) -> bool {
return T->getTag().find(Str) != StringRef::npos
|| T->getID().find(Str) != StringRef::npos;
return T->getTag().contains(Str) || T->getID().contains(Str);
};

if (SILPrintAfter.end() !=
Expand All @@ -215,8 +213,7 @@ static bool isDisabled(SILTransform *T, SILFunction *F = nullptr) {
return false;

for (const std::string &NamePattern : SILDisablePass) {
if (T->getTag().find(NamePattern) != StringRef::npos
|| T->getID().find(NamePattern) != StringRef::npos) {
if (T->getTag().contains(NamePattern) || T->getID().contains(NamePattern)) {
return true;
}
}
Expand All @@ -233,8 +230,7 @@ static void printModule(SILModule *Mod, bool EmitVerboseSIL) {
std::find(SILPrintOnlyFun.begin(), SILPrintOnlyFun.end(), F.getName()))
F.dump(EmitVerboseSIL);

if (!SILPrintOnlyFuns.empty() &&
F.getName().find(SILPrintOnlyFuns, 0) != StringRef::npos)
Copy link
Contributor

@gottesmm gottesmm Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note for myself. Whats with the 0? (I am doing a surface level review scan now).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mininny I looked in StringRef.h. Looks like the 0 is a default argument to find that says start checking at the beginning. (Just answering my own question).

if (!SILPrintOnlyFuns.empty() && F.getName().contains(SILPrintOnlyFuns))
F.dump(EmitVerboseSIL);
}
}
Expand Down Expand Up @@ -429,8 +425,7 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
CurrentPassHasInvalidated = false;

auto MatchFun = [&](const std::string &Str) -> bool {
return SFT->getTag().find(Str) != StringRef::npos ||
SFT->getID().find(Str) != StringRef::npos;
return SFT->getTag().contains(Str) || SFT->getID().contains(Str);
};
if ((SILVerifyBeforePass.end() != std::find_if(SILVerifyBeforePass.begin(),
SILVerifyBeforePass.end(),
Expand Down Expand Up @@ -597,8 +592,7 @@ void SILPassManager::runModulePass(unsigned TransIdx) {
}

auto MatchFun = [&](const std::string &Str) -> bool {
return SMT->getTag().find(Str) != StringRef::npos ||
SMT->getID().find(Str) != StringRef::npos;
return SMT->getTag().contains(Str) || SMT->getID().contains(Str);
};
if ((SILVerifyBeforePass.end() != std::find_if(SILVerifyBeforePass.begin(),
SILVerifyBeforePass.end(),
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Transforms/Outliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@ class Outliner : public SILFunctionTransform {

// Dump function if requested.
if (DumpFuncsBeforeOutliner.size() &&
Fun->getName().find(DumpFuncsBeforeOutliner, 0) != StringRef::npos) {
Fun->getName().contains(DumpFuncsBeforeOutliner)) {
Fun->dump();
}

Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/UtilityPasses/CFGPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class SILCFGPrinter : public SILFunctionTransform {
if (!SILViewCFGOnlyFun.empty() && F && F->getName() != SILViewCFGOnlyFun)
return;
if (!SILViewCFGOnlyFuns.empty() && F &&
F->getName().find(SILViewCFGOnlyFuns, 0) == StringRef::npos)
!F->getName().contains(SILViewCFGOnlyFuns))
return;

F->viewCFG();
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/UtilityPasses/LoopRegionPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class LoopRegionViewText : public SILModuleTransform {
if (!SILViewCFGOnlyFun.empty() && fn.getName() != SILViewCFGOnlyFun)
continue;
if (!SILViewCFGOnlyFuns.empty() &&
fn.getName().find(SILViewCFGOnlyFuns, 0) == StringRef::npos)
!fn.getName().contains(SILViewCFGOnlyFuns))
continue;

// Ok, we are going to analyze this function. Invalidate all state
Expand All @@ -74,7 +74,7 @@ class LoopRegionViewCFG : public SILModuleTransform {
if (!SILViewCFGOnlyFun.empty() && fn.getName() != SILViewCFGOnlyFun)
continue;
if (!SILViewCFGOnlyFuns.empty() &&
fn.getName().find(SILViewCFGOnlyFuns, 0) == StringRef::npos)
!fn.getName().contains(SILViewCFGOnlyFuns))
continue;

// Ok, we are going to analyze this function. Invalidate all state
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -790,8 +790,8 @@ SILFunction *swift::getEligibleFunction(FullApplySite AI,
return nullptr;
}

if (!SILInlineNeverFuns.empty()
&& Callee->getName().find(SILInlineNeverFuns, 0) != StringRef::npos)
if (!SILInlineNeverFuns.empty() &&
Callee->getName().contains(SILInlineNeverFuns))
return nullptr;

if (!SILInlineNeverFun.empty() &&
Expand Down
2 changes: 1 addition & 1 deletion tools/SourceKit/lib/Support/UIDRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void UIdent::print(llvm::raw_ostream &OS) const {

void *UIDRegistryImpl::get(StringRef Str) {
assert(!Str.empty());
assert(Str.find(' ') == StringRef::npos);
assert(!Str.contains(' '));
EntryTy *Ptr = 0;
Queue.dispatchSync([&]{
HashTableTy::iterator It = HashTable.find(Str);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ sourcekitd_object_t YAMLRequestParser::createObjFromNode(
if (!Raw.getAsInteger(10, val))
return sourcekitd_request_int64_create(val);

if (Raw.find(' ') != StringRef::npos)
if (Raw.contains(' '))
return withError("Found space in non-string value", Value, Error);

return sourcekitd_request_uid_create(
Expand Down
4 changes: 2 additions & 2 deletions tools/driver/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ static bool shouldRunAsSubcommand(StringRef ExecName,
// Otherwise, we have a program argument. If it looks like an option or a
// path, then invoke in interactive mode with the arguments as given.
StringRef FirstArg(Args[1]);
if (FirstArg.startswith("-") || FirstArg.find('.') != StringRef::npos ||
FirstArg.find('/') != StringRef::npos)
if (FirstArg.startswith("-") || FirstArg.contains('.') ||
FirstArg.contains('/'))
return false;

// Otherwise, we should have some sort of subcommand. Get the subcommand name
Expand Down
3 changes: 1 addition & 2 deletions tools/swift-api-digester/ModuleAnalyzerNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2291,8 +2291,7 @@ int swift::ide::api::findDeclUsr(StringRef dumpPath, CheckerOptions Opts) {
FinderByLocation(StringRef Location): Location(Location) {}
void visit(SDKNode* Node) override {
if (auto *D = dyn_cast<SDKNodeDecl>(Node)) {
if (D->getLocation().find(Location) != StringRef::npos &&
!D->getUsr().empty()) {
if (D->getLocation().contains(Location) && !D->getUsr().empty()) {
llvm::outs() << D->getFullyQualifiedName() << ": " << D->getUsr() << "\n";
}
}
Expand Down
12 changes: 7 additions & 5 deletions tools/swift-api-digester/swift-api-digester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2508,11 +2508,13 @@ static int generateMigrationScript(StringRef LeftPath, StringRef RightPath,
TypeAliasDiffFinder(RightModule, LeftModule, RevertAliasMap).search();
populateAliasChanges(RevertAliasMap, AllItems, /*IsRevert*/true);

AllItems.erase(std::remove_if(AllItems.begin(), AllItems.end(),
[&](CommonDiffItem &Item) {
return Item.DiffKind == NodeAnnotation::RemovedDecl &&
IgnoredRemoveUsrs.find(Item.LeftUsr) != IgnoredRemoveUsrs.end();
}), AllItems.end());
AllItems.erase(
std::remove_if(AllItems.begin(), AllItems.end(),
[&](CommonDiffItem &Item) {
return Item.DiffKind == NodeAnnotation::RemovedDecl &&
IgnoredRemoveUsrs.contains(Item.LeftUsr);
}),
AllItems.end());

NoEscapeFuncParamVector AllNoEscapingFuncs;
NoEscapingFuncEmitter::collectDiffItems(RightModule, AllNoEscapingFuncs);
Expand Down