[Libreoffice-commits] core.git: compilerplugins/clang solenv/CompilerTest_compilerplugins_clang.mk
Noel Grandin
noel.grandin at collabora.co.uk
Wed Sep 20 10:43:44 UTC 2017
compilerplugins/clang/flatten.cxx | 262 +++++++++++++++++++++++++++
compilerplugins/clang/test/flatten.cxx | 29 ++
solenv/CompilerTest_compilerplugins_clang.mk | 1
3 files changed, 292 insertions(+)
New commits:
commit dc97ede7cffbb5ce704602456ba0f560caee22a2
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Wed Sep 20 12:18:04 2017 +0200
new loplugin flatten
look for places where we can flatten the control flow in a method by
exiting early with a throw, ie. instead of
if (cond)
stuff();
else
throw ex;
we change it to:
if (!cond)
throw ex;
stuff();
Change-Id: I8b6bdf883b325807c7e3a3ef698e4f4606e7d38b
diff --git a/compilerplugins/clang/flatten.cxx b/compilerplugins/clang/flatten.cxx
new file mode 100644
index 000000000000..f3c49591c1a7
--- /dev/null
+++ b/compilerplugins/clang/flatten.cxx
@@ -0,0 +1,262 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+#include "plugin.hxx"
+
+/**
+ Look for places where we can flatten the control flow in a method.
+
+ */
+
+namespace {
+
+class Flatten:
+ public RecursiveASTVisitor<Flatten>, public loplugin::RewritePlugin
+{
+public:
+ explicit Flatten(InstantiationData const & data): RewritePlugin(data) {}
+
+ virtual void run() override
+ {
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ }
+
+ bool TraverseCXXCatchStmt(CXXCatchStmt * );
+ bool VisitIfStmt(const IfStmt * );
+private:
+ bool rewrite(const IfStmt * );
+ SourceRange ignoreMacroExpansions(SourceRange range);
+ std::string getSourceAsString(SourceRange range);
+};
+
+static const Stmt * containsSingleThrowExpr(const Stmt * stmt)
+{
+ if (auto compoundStmt = dyn_cast<CompoundStmt>(stmt)) {
+ if (compoundStmt->size() != 1)
+ return nullptr;
+ stmt = *compoundStmt->body_begin();
+ }
+ if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(stmt)) {
+ stmt = exprWithCleanups->getSubExpr();
+ }
+ return dyn_cast<CXXThrowExpr>(stmt);
+}
+
+bool Flatten::TraverseCXXCatchStmt(CXXCatchStmt* )
+{
+ // ignore stuff inside catch statements, where doing a "if...else..throw" is more natural
+ return true;
+}
+
+bool Flatten::VisitIfStmt(const IfStmt* ifStmt)
+{
+ if (ignoreLocation(ifStmt))
+ return true;
+
+ if (!ifStmt->getElse())
+ return true;
+
+ // ignore if/then/else/if chains for now
+ if (isa<IfStmt>(ifStmt->getElse()))
+ return true;
+
+ // ignore if we are part of an if/then/else/if chain
+ auto parentIfStmt = dyn_cast<IfStmt>(parentStmt(ifStmt));
+ if (parentIfStmt && parentIfStmt->getElse() == ifStmt)
+ return true;
+
+ auto throwExpr = containsSingleThrowExpr(ifStmt->getElse());
+ if (!throwExpr)
+ return true;
+
+ // if both the "if" and the "else" contain throws, no improvement
+ if (containsSingleThrowExpr(ifStmt->getThen()))
+ return true;
+
+ if (!rewrite(ifStmt))
+ {
+ report(
+ DiagnosticsEngine::Warning,
+ "unconditional throw in else branch, rather invert the condition, throw early, and flatten the normal case",
+ throwExpr->getLocStart())
+ << throwExpr->getSourceRange();
+ report(
+ DiagnosticsEngine::Note,
+ "if condition here",
+ ifStmt->getLocStart())
+ << ifStmt->getSourceRange();
+ }
+ return true;
+}
+
+static std::string stripOpenAndCloseBrace(std::string s);
+static std::string deindentThenStmt(std::string const & s);
+static std::vector<std::string> split(std::string const & s);
+
+bool Flatten::rewrite(const IfStmt* ifStmt)
+{
+ if (!rewriter)
+ return false;
+
+ auto conditionRange = ignoreMacroExpansions(ifStmt->getCond()->getSourceRange());
+ if (!conditionRange.isValid()) {
+ return false;
+ }
+ auto thenRange = ignoreMacroExpansions(ifStmt->getThen()->getSourceRange());
+ if (!thenRange.isValid()) {
+ return false;
+ }
+ auto elseRange = ignoreMacroExpansions(ifStmt->getElse()->getSourceRange());
+ if (!elseRange.isValid()) {
+ return false;
+ }
+ auto elseKeywordRange = ifStmt->getElseLoc();
+
+ // in adjusting the formatting I assume that "{" starts on a new line
+
+ std::string conditionString = getSourceAsString(conditionRange);
+ conditionString = "(!" + conditionString + ")";
+
+ std::string thenString = getSourceAsString(thenRange);
+ bool thenIsCompound = false;
+ if (auto compoundStmt = dyn_cast<CompoundStmt>(ifStmt->getThen())) {
+ if (compoundStmt->getLBracLoc().isValid()) {
+ thenIsCompound = true;
+ thenString = stripOpenAndCloseBrace(thenString);
+ }
+ }
+ thenString = deindentThenStmt(thenString);
+
+ std::string elseString = getSourceAsString(elseRange);
+ bool elseIsCompound = false;
+ if (auto compoundStmt = dyn_cast<CompoundStmt>(ifStmt->getElse())) {
+ if (compoundStmt->getLBracLoc().isValid()) {
+ elseIsCompound = true;
+ }
+ }
+ // indent else block if necessary
+ if (thenIsCompound && !elseIsCompound)
+ elseString = " " + elseString;
+
+ if (!replaceText(elseRange, thenString)) {
+ return false;
+ }
+ if (!replaceText(elseKeywordRange, "")) {
+ return false;
+ }
+ if (!replaceText(thenRange, elseString)) {
+ return false;
+ }
+ if (!replaceText(conditionRange, conditionString)) {
+ return false;
+ }
+
+ return true;
+}
+
+std::string stripOpenAndCloseBrace(std::string s)
+{
+ size_t openBrace = s.find_first_of("{");
+ if (openBrace != std::string::npos) {
+ size_t openLineEnd = s.find_first_of("\n", openBrace + 1);
+ if (openLineEnd != std::string::npos)
+ s = s.substr(openLineEnd + 1);
+ else
+ s = s.substr(openBrace + 1);
+ }
+ size_t closeBrace = s.find_last_of("}");
+ if (closeBrace != std::string::npos) {
+ size_t closeLineEnd = s.find_last_of("\n", closeBrace);
+ if (closeLineEnd != std::string::npos)
+ s = s.substr(0, closeLineEnd - 1);
+ else
+ s = s.substr(0, closeBrace - 1);
+ }
+ return s;
+}
+
+std::string deindentThenStmt(std::string const & s)
+{
+ std::vector<std::string> lines = split(s);
+ std::string rv;
+ for (auto s : lines) {
+ rv += s.length() > 4 ? s.substr(4) : s;
+ rv += "\n";
+ }
+ return rv;
+}
+
+std::vector<std::string> split(std::string const & s)
+{
+ size_t next = -1;
+ std::vector<std::string> rv;
+ do
+ {
+ size_t current = next + 1;
+ next = s.find_first_of( "\n", current );
+ rv.push_back(s.substr( current, next - current ));
+ }
+ while (next != std::string::npos);
+ return rv;
+}
+
+SourceRange Flatten::ignoreMacroExpansions(SourceRange range) {
+ while (compiler.getSourceManager().isMacroArgExpansion(range.getBegin())) {
+ range.setBegin(
+ compiler.getSourceManager().getImmediateMacroCallerLoc(
+ range.getBegin()));
+ }
+ if (range.getBegin().isMacroID()) {
+ SourceLocation loc;
+ if (Lexer::isAtStartOfMacroExpansion(
+ range.getBegin(), compiler.getSourceManager(),
+ compiler.getLangOpts(), &loc))
+ {
+ range.setBegin(loc);
+ }
+ }
+ while (compiler.getSourceManager().isMacroArgExpansion(range.getEnd())) {
+ range.setEnd(
+ compiler.getSourceManager().getImmediateMacroCallerLoc(
+ range.getEnd()));
+ }
+ if (range.getEnd().isMacroID()) {
+ SourceLocation loc;
+ if (Lexer::isAtEndOfMacroExpansion(
+ range.getEnd(), compiler.getSourceManager(),
+ compiler.getLangOpts(), &loc))
+ {
+ range.setEnd(loc);
+ }
+ }
+ return range.getBegin().isMacroID() || range.getEnd().isMacroID()
+ ? SourceRange() : range;
+}
+
+std::string Flatten::getSourceAsString(SourceRange range)
+{
+ SourceManager& SM = compiler.getSourceManager();
+ SourceLocation startLoc = range.getBegin();
+ SourceLocation endLoc = range.getEnd();
+ const char *p1 = SM.getCharacterData( startLoc );
+ const char *p2 = SM.getCharacterData( endLoc );
+ unsigned n = Lexer::MeasureTokenLength( endLoc, SM, compiler.getLangOpts());
+ return std::string( p1, p2 - p1 + n);
+}
+
+loplugin::Plugin::Registration< Flatten > X("flatten", false);
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/flatten.cxx b/compilerplugins/clang/test/flatten.cxx
new file mode 100644
index 000000000000..d6d55d6a1771
--- /dev/null
+++ b/compilerplugins/clang/test/flatten.cxx
@@ -0,0 +1,29 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <exception>
+
+extern int foo();
+extern int bar();
+
+int main() {
+ if (foo() == 1) { // expected-note {{if condition here [loplugin:flatten]}}
+ } else {
+ throw std::exception(); // expected-error {{unconditional throw in else branch, rather invert the condition, throw early, and flatten the normal case [loplugin:flatten]}}
+ }
+
+ // no warning expected
+ if (bar() == 3) {
+ throw std::exception();
+ } else {
+ throw std::exception();
+ }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 02d6155d3dd5..8df1c2b495f7 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -19,6 +19,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/externvar \
compilerplugins/clang/test/expressionalwayszero \
compilerplugins/clang/test/finalprotected \
+ compilerplugins/clang/test/flatten \
compilerplugins/clang/test/loopvartoosmall \
compilerplugins/clang/test/oncevar \
compilerplugins/clang/test/oslendian-1 \
More information about the Libreoffice-commits
mailing list