compilerplugins/clang/ostr.cxx | 237 +++++++++++++++++++++++++++++++++++- compilerplugins/clang/test/ostr.cxx | 17 ++ 2 files changed, 248 insertions(+), 6 deletions(-)
New commits: commit 769899853557831ae53d020497e81c8fe572874b Author: Stephan Bergmann <[email protected]> AuthorDate: Thu Oct 19 10:33:19 2023 +0200 Commit: Stephan Bergmann <[email protected]> CommitDate: Sat Oct 21 08:54:33 2023 +0200 Extended loplugin:ostr: Automatic rewrite some O[U]StringLiteral -> O[U]String Change-Id: I8c08bf41b96d4a6085e7d72cb39e629efa556d09 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158300 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <[email protected]> diff --git a/compilerplugins/clang/ostr.cxx b/compilerplugins/clang/ostr.cxx index 462916641421..9675913c8576 100644 --- a/compilerplugins/clang/ostr.cxx +++ b/compilerplugins/clang/ostr.cxx @@ -37,9 +37,125 @@ public: void run() override { - if (compiler.getLangOpts().CPlusPlus) + if (compiler.getLangOpts().CPlusPlus + && TraverseDecl(compiler.getASTContext().getTranslationUnitDecl())) { - TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + for (auto const& i : vars_) + { + auto const utf16 + = bool(loplugin::TypeCheck(i.first->getType()).Class("OUStringLiteral")); + if (i.second.singleUse == nullptr) + { + if (!(i.first->getDeclContext()->isFunctionOrMethod() + || compiler.getSourceManager().isInMainFile(i.first->getLocation()) + || compiler.getDiagnosticOpts().VerifyDiagnostics)) + { + //TODO, rewriting these in include files could trigger + // loplugin:redundantfcast in other translation units: + continue; + } + if (rewriter != nullptr) + { + auto e = i.first->getInit()->IgnoreParenImpCasts(); + if (auto const e2 = dyn_cast<ConstantExpr>(e)) + { + e = e2->getSubExpr()->IgnoreParenImpCasts(); + } + if (auto const e2 = dyn_cast<CXXConstructExpr>(e)) + { + assert(e2->getNumArgs() == 1); + e = e2->getArg(0)->IgnoreParenImpCasts(); + } + e = dyn_cast<clang::StringLiteral>(e); + // e is null when this OUStringLiteral is initialized with another + // OUStringLiteral: + if (e == nullptr + || insertTextAfterToken(e->getEndLoc(), utf16 ? "_ustr" : "_ostr")) + { + auto ok = true; + for (auto d = i.first->getMostRecentDecl(); d != nullptr; + d = d->getPreviousDecl()) + { + auto const l1 = d->getTypeSpecStartLoc(); + auto l2 = d->getTypeSpecEndLoc(); + l2 = l2.getLocWithOffset(Lexer::MeasureTokenLength( + l2, compiler.getSourceManager(), compiler.getLangOpts())); + if (!replaceText(l1, delta(l1, l2), utf16 ? "OUString" : "OString")) + { + ok = false; + } + } + for (auto const i : i.second.explicitConversions) + { + auto const e2 = i->getArg(0); + auto l1 = i->getBeginLoc(); + auto l2 = e2->getBeginLoc(); + auto l3 = e2->getEndLoc(); + auto l4 = i->getEndLoc(); + while (compiler.getSourceManager().isMacroArgExpansion(l1) + && compiler.getSourceManager().isMacroArgExpansion(l2) + && compiler.getSourceManager().isMacroArgExpansion(l3) + && compiler.getSourceManager().isMacroArgExpansion(l4)) + //TODO: check all four locations are part of the same macro argument + // expansion + { + l1 = compiler.getSourceManager().getImmediateMacroCallerLoc(l1); + l2 = compiler.getSourceManager().getImmediateMacroCallerLoc(l2); + l3 = compiler.getSourceManager().getImmediateMacroCallerLoc(l3); + l4 = compiler.getSourceManager().getImmediateMacroCallerLoc(l4); + } + l3 = l3.getLocWithOffset(Lexer::MeasureTokenLength( + l3, compiler.getSourceManager(), compiler.getLangOpts())); + l4 = l4.getLocWithOffset(Lexer::MeasureTokenLength( + l4, compiler.getSourceManager(), compiler.getLangOpts())); + removeText(l1, delta(l1, l2)); + removeText(l3, delta(l3, l4)); + } + if (ok) + { + continue; + } + } + } + report(DiagnosticsEngine::Warning, + "use '%select{OString|OUString}0', created from a %select{_ostr|_ustr}0 " + "user-defined string literal, instead of " + "'%select{OStringLiteral|OUStringLiteral}0' for the variable %1", + i.first->getLocation()) + << utf16 << i.first->getName() << i.first->getSourceRange(); + for (auto d = i.first->getMostRecentDecl(); d != nullptr; + d = d->getPreviousDecl()) + { + if (d != i.first) + { + report(DiagnosticsEngine::Note, "variable %0 declared here", + d->getLocation()) + << d->getName() << d->getSourceRange(); + } + } + } + else + { + if (!compiler.getDiagnosticOpts().VerifyDiagnostics) + { + //TODO, left for later: + continue; + } + report(DiagnosticsEngine::Warning, + "directly use a %select{_ostr|_ustr}0 user-defined string literal " + "instead of introducing the intermediary " + "'%select{OStringLiteral|OUStringLiteral}0' variable %1", + i.second.singleUse->getExprLoc()) + << utf16 << i.first->getName() << i.second.singleUse->getSourceRange(); + for (auto d = i.first->getMostRecentDecl(); d != nullptr; + d = d->getPreviousDecl()) + { + report(DiagnosticsEngine::Note, "intermediary variable %0 declared here", + d->getLocation()) + << d->getName() << d->getSourceRange(); + } + } + } } } @@ -67,16 +183,117 @@ public: return ret; } + bool VisitVarDecl(VarDecl const* decl) + { + if (ignoreLocation(decl)) + { + return true; + } + if (!decl->isThisDeclarationADefinition()) + { + return true; + } + loplugin::TypeCheck const tc(decl->getType()); + if (!(tc.Class("OStringLiteral").Namespace("rtl").GlobalNamespace() + || tc.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace())) + { + return true; + } + if (suppressWarningAt(decl->getLocation())) + { + return true; + } + vars_[decl].multipleUses + = decl->getDeclContext()->isFileContext() + ? !compiler.getSourceManager().isInMainFile(decl->getLocation()) + : decl->isExternallyVisible(); + return true; + } + + bool VisitDeclRefExpr(DeclRefExpr const* expr) + { + if (ignoreLocation(expr)) + { + return true; + } + auto const d1 = dyn_cast<VarDecl>(expr->getDecl()); + if (d1 == nullptr) + { + return true; + } + auto const d2 = d1->getDefinition(); + if (d2 == nullptr) + { + return true; + } + auto const i = vars_.find(d2); + if (i == vars_.end()) + { + return true; + } + if (!i->second.multipleUses) + { + if (i->second.singleUse == nullptr) + { + i->second.singleUse = expr; + } + else + { + i->second.multipleUses = true; + i->second.singleUse = nullptr; + } + } + return true; + } + bool VisitCXXConstructExpr(CXXConstructExpr const* expr) { if (ignoreLocation(expr)) { return true; } - if (!loplugin::DeclCheck(expr->getConstructor()->getParent()) - .Class("OUString") - .Namespace("rtl") - .GlobalNamespace()) + auto const dc = expr->getConstructor()->getParent(); + auto const utf16 + = bool(loplugin::DeclCheck(dc).Class("OUString").Namespace("rtl").GlobalNamespace()); + if (!(utf16 || loplugin::DeclCheck(dc).Class("OString").Namespace("rtl").GlobalNamespace())) + { + return true; + } + if (expr->getNumArgs() == 1 + && loplugin::TypeCheck(expr->getArg(0)->getType()) + .Class(utf16 ? "OUStringLiteral" : "OStringLiteral") + .Namespace("rtl") + .GlobalNamespace()) + { + if (functionalCasts_.empty() + || functionalCasts_.top()->getSubExpr()->IgnoreImplicit() != expr) + { + return true; + } + auto const e = dyn_cast<DeclRefExpr>(expr->getArg(0)->IgnoreParenImpCasts()); + if (e == nullptr) + { + return true; + } + auto const d1 = dyn_cast<VarDecl>(e->getDecl()); + if (d1 == nullptr) + { + return true; + } + auto const d2 = d1->getDefinition(); + if (d2 == nullptr) + { + return true; + } + auto const i = vars_.find(d2); + if (i == vars_.end()) + { + return true; + } + i->second.explicitConversions.insert(expr); + return true; + } + if (!utf16) { return true; } @@ -195,9 +412,17 @@ private: - compiler.getSourceManager().getDecomposedLoc(loc1).second; } + struct Var + { + bool multipleUses = false; + DeclRefExpr const* singleUse = nullptr; + std::set<CXXConstructExpr const*> explicitConversions; + }; + std::set<Expr const*> defaultArgs_; std::stack<CXXFunctionalCastExpr const*> functionalCasts_; std::set<SourceLocation> locs_; + std::map<VarDecl const*, Var> vars_; }; loplugin::Plugin::Registration<Ostr> X("ostr", true); diff --git a/compilerplugins/clang/test/ostr.cxx b/compilerplugins/clang/test/ostr.cxx index 6a09728f3ff6..28e2d746a447 100644 --- a/compilerplugins/clang/test/ostr.cxx +++ b/compilerplugins/clang/test/ostr.cxx @@ -53,6 +53,23 @@ void f() // expansion: // expected-error-re@+1 {{use a _ustr user-defined string literal instead of constructing an instance of '{{(rtl::)?}}OUString' from an ordinary string literal [loplugin:ostr]}} M("foo"); + + // expected-note@+1 {{intermediary variable l1 declared here [loplugin:ostr]}} + constexpr OStringLiteral l1("foo"); + // expected-error@+1 {{directly use a _ostr user-defined string literal instead of introducing the intermediary 'OStringLiteral' variable l1 [loplugin:ostr]}} + (void)l1; + // expected-error@+1 {{use 'OString', created from a _ostr user-defined string literal, instead of 'OStringLiteral' for the variable l2 [loplugin:ostr]}} + constexpr OStringLiteral l2("foo"); + (void)l2; + (void)l2; + // expected-note@+1 {{intermediary variable l3 declared here [loplugin:ostr]}} + OUStringLiteral l3(u"foo"); + // expected-error@+1 {{directly use a _ustr user-defined string literal instead of introducing the intermediary 'OUStringLiteral' variable l3 [loplugin:ostr]}} + (void)l3; + // expected-error@+1 {{use 'OUString', created from a _ustr user-defined string literal, instead of 'OUStringLiteral' for the variable l4 [loplugin:ostr]}} + OUStringLiteral l4(u"foo"); + (void)l4; + (void)l4; } /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
