[PATCH] D25639: Add ctor for string literal to StringRef, and make explicit the conversion from const char *

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 14 18:07:29 PDT 2016


mehdi_amini created this revision.
mehdi_amini added reviewers: dexonsmith, zturner, pete.
mehdi_amini added a subscriber: llvm-commits.
Herald added subscribers: nhaehnle, ki.stfu, nemanjai, mzolotukhin, jyknight, arsenm, dschuff, danalbert, qcolombet, MatzeB, tberghammer, emaste, jholewinski, klimek.
Herald added a reviewer: tstellarAMD.

The implicit conversion from const char * to StringRef involve calling strlen.
Having "expensive" calls happening implicitly with a class like StringRef which
is supposed to be a lightweight wrapper does not seem like not a good idea to me.
I already landed over 30 patches converting multiple APIs to use StringRef, this
is the last one.

Making the conversion explicit allows to have a constexpr constructor for string
literals (or literal char arrays), making global tables of StringRef possible
without needing a static initializer.

After this series of patches, an LTO link of llvm-tblgen is calling ~1.5M times
strlen() vs over 11M before.

A frequent pattern I found that making this constructor explicit helps avoiding
is a conversion from std::string or StringRef to `const char *` back to
StringRef, often in a single expression. For Example:

  llvm::SmallString<256> CleanPath(Entry.first.data());

Removing the .data() avoids a call to strlen.

Most of the call sites here have been updated with a simple clang-tidy check,
with some manual tweaking when needed.

Here is the check if anyone needs to update some codebase downstream:

void StringrefCheck::registerMatchers(MatchFinder *Finder) {

  Finder->addMatcher(
      materializeTemporaryExpr(
          hasType(namedDecl(hasName("llvm::StringRef"))),
          has(expr(ignoringImpCasts(cxxConstructExpr(
              argumentCountIs(1),
              hasArgument(0, expr(hasType(pointsTo(isAnyCharacter())))))))))
          .bind("imp_conv"),
      this);

}
void StringrefCheck::check(const MatchFinder::MatchResult &Result) {

  const auto *MatchedCtor =
      Result.Nodes.getNodeAs<MaterializeTemporaryExpr>("imp_conv");
  const auto FixCondStart = FixItHint::CreateInsertion(
      MatchedCtor->getLocStart(), "llvm::StringRef(");
  const auto FixCondEnd = FixItHint::CreateInsertion(
      Lexer::getLocForEndOfToken(MatchedCtor->getLocEnd(), 0,
                                 *Result.SourceManager,
                                 Result.Context->getLangOpts()),
      ")");
  
  auto Diag = diag(MatchedCtor->getExprLoc(), "Explicit StringRef ctor:");
  Diag << FixCondStart << FixCondEnd;

}


https://reviews.llvm.org/D25639

Files:
  clang-tools-extra/change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  clang-tools-extra/clang-move/tool/ClangMoveMain.cpp
  clang-tools-extra/clang-query/tool/ClangQuery.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/misc/FoldInitTypeCheck.cpp
  clang-tools-extra/clang-tidy/misc/NonCopyableObjects.cpp
  clang-tools-extra/clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedRAIICheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/ImplicitBoolCastCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp
  clang-tools-extra/include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
  clang-tools-extra/modularize/Modularize.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/modularize/PreprocessorTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/tool-template/ToolTemplate.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
  clang/examples/clang-interpreter/main.cpp
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/ARCMigrate/TransBlockObjCVariable.cpp
  clang/lib/ARCMigrate/TransProperties.cpp
  clang/lib/ARCMigrate/Transforms.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/CommentCommandTraits.cpp
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/Type.cpp
  clang/lib/ASTMatchers/Dynamic/Parser.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/FileSystemStatCache.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Warnings.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/MinGWToolChain.cpp
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains.cpp
  clang/lib/Driver/Tools.cpp
  clang/lib/Driver/Types.cpp
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Frontend/Rewrite/RewriteMacros.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/PTHLexer.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp
  clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SelectorExtras.h
  clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  clang/lib/Tooling/CommonOptionsParser.cpp
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/clang-check/ClangCheck.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
  clang/tools/diagtool/TreeView.cpp
  clang/tools/diagtool/diagtool_main.cpp
  clang/tools/driver/driver.cpp
  clang/tools/libclang/ARCMigrate.cpp
  clang/tools/libclang/BuildSystem.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/tools/libclang/CIndexUSRs.cpp
  clang/tools/libclang/CIndexer.cpp
  clang/tools/libclang/CXCompilationDatabase.cpp
  clang/tools/libclang/CXLoadedDiagnostic.cpp
  clang/tools/libclang/CXType.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/AST/CommentLexer.cpp
  clang/unittests/AST/CommentParser.cpp
  clang/unittests/AST/DeclPrinterTest.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Tooling/CommentHandlerTest.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTestCallVisitor.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  lld/COFF/Driver.cpp
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.cpp
  lld/ELF/Mips.cpp
  lld/ELF/OutputSections.cpp
  lld/ELF/Relocations.cpp
  lld/ELF/Writer.cpp
  lld/lib/ReaderWriter/MachO/LayoutPass.cpp
  lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryReader.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h
  lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
  lld/lib/ReaderWriter/MachO/StubsPass.cpp
  lld/tools/lld/lld.cpp
  lldb/include/lldb/Expression/IRExecutionUnit.h
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/AnsiTerminal.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBHostOS.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  (408 more files...)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25639.74762.patch
Type: text/x-patch
Size: 929062 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161015/6ea28804/attachment-0001.bin>


More information about the llvm-commits mailing list