Skip to content

Commit

Permalink
Replace usage of StringRef::find_last_of with a string literal of siz…
Browse files Browse the repository at this point in the history
…e one by the equivalent char literal
  • Loading branch information
serge-sans-paille committed Nov 15, 2023
1 parent 2060bfc commit 33b5158
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion clang/lib/Driver/ToolChains/ZOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ void zos::Linker::ConstructJob(Compilation &C, const JobAction &JA,
StringRef OutputName = Output.getFilename();
// Strip away the last file suffix in presence from output name and add
// a new .x suffix.
size_t Suffix = OutputName.find_last_of(".");
size_t Suffix = OutputName.find_last_of('.');
const char *SideDeckName =
Args.MakeArgString(OutputName.substr(0, Suffix) + ".x");
CmdArgs.push_back("-x");
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void AMDGPUPrintfRuntimeBindingImpl::getConversionSpecifiers(
bool ArgDump = false;
StringRef CurFmt = Fmt.substr(PrevFmtSpecifierIdx,
CurFmtSpecifierIdx - PrevFmtSpecifierIdx);
size_t pTag = CurFmt.find_last_of("%");
size_t pTag = CurFmt.find_last_of('%');
if (pTag != StringRef::npos) {
ArgDump = true;
while (pTag && CurFmt[--pTag] == '%') {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ lookupBuiltin(StringRef DemangledCall,
// the information after angle brackets and return type removed.
if (BuiltinName.find('<') && BuiltinName.back() == '>') {
BuiltinName = BuiltinName.substr(0, BuiltinName.find('<'));
BuiltinName = BuiltinName.substr(BuiltinName.find_last_of(" ") + 1);
BuiltinName = BuiltinName.substr(BuiltinName.find_last_of(' ') + 1);
}

// Check if the extracted name begins with "__spirv_ImageSampleExplicitLod"
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/Tools/lsp-server-support/SourceMgrUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ lsp::extractSourceDocComment(llvm::SourceMgr &sourceMgr, SMLoc loc) {

// Pop the last line from the buffer string.
auto popLastLine = [&]() -> std::optional<StringRef> {
size_t newlineOffset = buffer.find_last_of("\n");
size_t newlineOffset = buffer.find_last_of('\n');
if (newlineOffset == StringRef::npos)
return std::nullopt;
StringRef lastLine = buffer.drop_front(newlineOffset).trim();
Expand Down

3 comments on commit 33b5158

@dwblaikie
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could reduce the chance of this failure, or make it not a problem by adding an array ref overload (though there's man yof these functions to add such defenses to) of length 1 (I guess it could assert than the 1 character isn't null - can't think of a reason you'd want to find_last_of... nothing? I guess some caller might do something quirky and expect find_last_of("") to successfully fail to find anything)? Probably not worth it, but just a thought.

@serge-sans-paille
Copy link
Collaborator Author

@serge-sans-paille serge-sans-paille commented on 33b5158 Nov 16, 2023 via email

Choose a reason for hiding this comment

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

@dwblaikie
Copy link
Collaborator

Choose a reason for hiding this comment

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

On Wed, Nov 15, 2023 at 08:57:38AM -0800, David Blaikie wrote: Maybe we could reduce the chance of this failure, or make it not a problem by adding an array ref overload (though there's man yof these functions to add such defenses to) of length 1 (I guess it could assert than the 1 character isn't null - can't think of a reason you'd want to find_last_of... nothing? I guess some caller might do something quirky and expect find_last_of("") to successfully fail to find anything)? Probably not worth it, but just a thought.
Yeah, I thought of that, but don't think it's worth the API clutter. There's this check that should do the trick: clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp

Ah, good stuff (is there now a nice way to integrate clang-tidy into the developer workflow? and/or running these as part of precommit checks, etc?)

Please sign in to comment.