From e81d8af50be5ec4b62b2b2890bc72ddb0f163b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Fri, 9 Oct 2015 17:06:52 +0200 Subject: [PATCH 01/21] Started integrating source transformation capability --- include/core/AbstractModuleConsumer.h | 1 + include/core/module/ModuleContext.h | 4 +++- include/core/transformation/TransformationHandler.h | 7 +++++++ include/core/transformation/TransformationUtil.h | 5 +++-- include/core/utility/ClangUtil.h | 1 + src/core/AbstractFactory.cpp | 4 ++-- src/core/AbstractModuleConsumer.cpp | 5 ++++- src/core/module/ModuleContext.cpp | 9 ++++++++- src/core/transformation/TransformationHandler.cpp | 12 ++++++++++++ src/modules/ImplicitConditionMatcher.cpp | 6 ++++++ test/src/test_explicitcast.cpp | 2 +- test/src/tests/ExplicitCastNoMatchSet.inl | 4 +++- 12 files changed, 51 insertions(+), 9 deletions(-) diff --git a/include/core/AbstractModuleConsumer.h b/include/core/AbstractModuleConsumer.h index c24d8e7..ddb0c69 100644 --- a/include/core/AbstractModuleConsumer.h +++ b/include/core/AbstractModuleConsumer.h @@ -24,6 +24,7 @@ class AbstractModuleConsumer : public clang::ASTConsumer { public: AbstractModuleConsumer(Module* module, ModuleContext* mcontext); + virtual void Initialize(clang::ASTContext &Context) override; void HandleTranslationUnit(clang::ASTContext& Context) override; virtual ~AbstractModuleConsumer(); }; diff --git a/include/core/module/ModuleContext.h b/include/core/module/ModuleContext.h index 2dc4a30..51f473a 100644 --- a/include/core/module/ModuleContext.h +++ b/include/core/module/ModuleContext.h @@ -34,10 +34,12 @@ class ModuleContext { clang::ASTContext* context; IssueHandler* ihandler; TransformationHandler* thandler; + std::string current_src; public: ModuleContext(Configuration* config, IssueHandler* ihandler, TransformationHandler* thandler); - void setASTContext(clang::ASTContext* context); + void initContext(clang::ASTContext* context); + void setCurrentSource(const std::string& source); clang::ASTContext& getASTContext(); clang::SourceManager& getSourceManager(); IssueHandler& getIssueHandler(); diff --git a/include/core/transformation/TransformationHandler.h b/include/core/transformation/TransformationHandler.h index 5de63fb..76e6767 100644 --- a/include/core/transformation/TransformationHandler.h +++ b/include/core/transformation/TransformationHandler.h @@ -8,6 +8,8 @@ #ifndef TRANSFORMATIONHANDLER_H_ #define TRANSFORMATIONHANDLER_H_ +#include + #include #include @@ -15,6 +17,8 @@ #include namespace clang { +class SourceManager; +class LangOptions; namespace tooling { class Replacement; class TranslationUnitReplacements; @@ -30,15 +34,18 @@ class TransformationHandler { TUReplacementsMap replacements; std::string source; std::unique_ptr includes; + clang::Rewriter rewriter; public: TransformationHandler(); void setSource(const std::string& current); + void initRewriter(clang::SourceManager& sm, const clang::LangOptions& langOpts); void addReplacements(const clang::tooling::Replacement& replacement); void addReplacements(const std::vector& replacements); TUReplacementsMap& getAllReplacements(); void setIncludeDirectives(IncludeDirectives* include); IncludeDirectives* getIncludeDirectives(); + clang::Rewriter& getRewriter(); virtual ~TransformationHandler(); }; diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index 42c5a6f..6e985a9 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -38,6 +38,7 @@ static inline bool isWhitespace(unsigned char c) { template inline StringRef whitespaces(const clang::SourceManager& sm, T node) { + // Taken from the clang::Rewriter source code. SourceRange range = locOf(sm, node); // const char* data = sm.getCharacterData(range.getBegin()); std::pair V = sm.getDecomposedLoc(range.getBegin()); @@ -69,7 +70,7 @@ inline std::vector addExplicitCompare(const clang:: replace += "(0.0)"; if (isa(expr)) { replace += ")"; - return {Replacement(sm, dyn_cast(expr)->getSubExpr()->getLocStart(), 0, "("), + return {Replacement(sm, llvm::dyn_cast(expr)->getSubExpr()->getLocStart(), 0, "("), Replacement(sm, range.getEnd(), 0, replace)}; } else { return {Replacement(sm, range.getEnd(), 0, replace)}; @@ -125,7 +126,7 @@ inline clang::tooling::Replacement removeNode(const clang::SourceManager& sm, T // FIXME does not remove empty line SourceRange loc = locOf(sm, node, 1); CharSourceRange cast_crange = CharSourceRange::getCharRange(loc); - return Replacement(sm, cast_crange, "\b"); + return Replacement(sm, cast_crange, ""); } template diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index bde13fa..82e8f6a 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include diff --git a/src/core/AbstractFactory.cpp b/src/core/AbstractFactory.cpp index 71d191a..fa28f5e 100644 --- a/src/core/AbstractFactory.cpp +++ b/src/core/AbstractFactory.cpp @@ -31,9 +31,9 @@ AbstractFactory::AbstractFactory(Configuration* config, IssueHandler* ihandler, bool AbstractFactory::handleBeginSource(clang::CompilerInstance& CI, llvm::StringRef Filename) { currentSource = Filename; - thandler->setSource(currentSource); + this->context->setCurrentSource(currentSource); + // FIXME relocate to modulecontext class thandler->setIncludeDirectives(new IncludeDirectives(CI)); - ihandler->setSource(currentSource); return true; } diff --git a/src/core/AbstractModuleConsumer.cpp b/src/core/AbstractModuleConsumer.cpp index d9c6fc0..7a0bea5 100644 --- a/src/core/AbstractModuleConsumer.cpp +++ b/src/core/AbstractModuleConsumer.cpp @@ -18,10 +18,13 @@ AbstractModuleConsumer::AbstractModuleConsumer(Module* module, ModuleContext* mc , mcontext(mcontext) { } +void AbstractModuleConsumer::Initialize(clang::ASTContext& Context) { + mcontext->initContext(&Context); +} + void AbstractModuleConsumer::HandleTranslationUnit(clang::ASTContext& Context) { // std::shared_ptr m_context = // std::make_shared(); - mcontext->setASTContext(&Context); LOG_DEBUG("AbstractModuleConsumer: Execute Module: " << module->moduleName()); module->execute(mcontext); } diff --git a/src/core/module/ModuleContext.cpp b/src/core/module/ModuleContext.cpp index 150ad81..ffee57e 100644 --- a/src/core/module/ModuleContext.cpp +++ b/src/core/module/ModuleContext.cpp @@ -28,8 +28,15 @@ ModuleContext::ModuleContext(Configuration* config, IssueHandler* ihandler, Tran issues.reserve(100); } -void ModuleContext::setASTContext(clang::ASTContext* context) { +void ModuleContext::initContext(clang::ASTContext* context) { this->context = context; + thandler->initRewriter(context->getSourceManager(), context->getLangOpts()); +} + +void ModuleContext::setCurrentSource(const std::string& source) { + this->current_src = source; + this->thandler->setSource(current_src); + this->ihandler->setSource(current_src); } clang::ASTContext& ModuleContext::getASTContext() { diff --git a/src/core/transformation/TransformationHandler.cpp b/src/core/transformation/TransformationHandler.cpp index 9e07970..3b12a20 100644 --- a/src/core/transformation/TransformationHandler.cpp +++ b/src/core/transformation/TransformationHandler.cpp @@ -9,6 +9,7 @@ #include #include +#include namespace opov { @@ -24,12 +25,19 @@ void TransformationHandler::setSource(const std::string& current) { source = current; } +void TransformationHandler::initRewriter(clang::SourceManager& sm, const clang::LangOptions& langOpts) { + rewriter.setSourceMgr(sm, langOpts); +} + void TransformationHandler::addReplacements(const clang::tooling::Replacement& replacement) { + replacement.apply(rewriter); + /* TranslationUnitReplacements& tunit = replacements[source]; if (tunit.MainSourceFile.empty()) { tunit.MainSourceFile = source; } tunit.Replacements.push_back(replacement); + */ } void TransformationHandler::addReplacements(const std::vector& replacements) { @@ -46,6 +54,10 @@ IncludeDirectives* TransformationHandler::getIncludeDirectives() { return includes.get(); } +clang::Rewriter& TransformationHandler::getRewriter() { + return rewriter; +} + TUReplacementsMap& TransformationHandler::getAllReplacements() { return replacements; } diff --git a/src/modules/ImplicitConditionMatcher.cpp b/src/modules/ImplicitConditionMatcher.cpp index 01778b7..e6e5031 100644 --- a/src/modules/ImplicitConditionMatcher.cpp +++ b/src/modules/ImplicitConditionMatcher.cpp @@ -13,6 +13,9 @@ #include #include +#include +#include + namespace opov { namespace module { @@ -46,6 +49,9 @@ void ImplicitConditionMatcher::run(const clang::ast_matchers::MatchFinder::Match const Expr* implicit_cast = result.Nodes.getStmtAs("implicit"); const Expr* unary_op = result.Nodes.getStmtAs("unary"); const Expr* invalid = !implicit_cast ? unary_op : implicit_cast; + auto e = result.Nodes.getMap(); + BoundNodes::IDToNodeMap::const_iterator BI; + auto& ihandle = context->getIssueHandler(); auto& sm = context->getSourceManager(); diff --git a/test/src/test_explicitcast.cpp b/test/src/test_explicitcast.cpp index 0b9aa49..a3a221b 100644 --- a/test/src/test_explicitcast.cpp +++ b/test/src/test_explicitcast.cpp @@ -25,7 +25,7 @@ #define SCAST_T(TYPE, CODE) "static_cast<" TYPE ">(" CODE ")" #define SCAST(CODE) MAKE_CODE("res = " SCAST_T("int", CODE)) #define SCAST_SCALAR(CODE) MAKE_CODE("res = " SCAST_T("scalar", CODE)) -#define SCAST_TYPE(TYPE, CODE) MAKE_CODE("res = " SCAST_T(TYPE, CODE)) +#define SCAST_TYPE(TYPE, CODE) MAKE_CODE(TYPE " cres = " SCAST_T(TYPE, CODE)) #define SCAST_S(CODE) SCAST_T("int", CODE) KICKOFF_TEST(opov::module::ExplicitCast(), SCAST("a"), SCAST_S("a")) diff --git a/test/src/tests/ExplicitCastNoMatchSet.inl b/test/src/tests/ExplicitCastNoMatchSet.inl index a3db52e..7bd3e66 100644 --- a/test/src/tests/ExplicitCastNoMatchSet.inl +++ b/test/src/tests/ExplicitCastNoMatchSet.inl @@ -11,4 +11,6 @@ SIMPLE_TEST0("A static cast of an unary." SIMPLE_TEST0("A static cast of a nested unary." , SCAST("!!a")); SIMPLE_TEST0("A static cast of a nested unary of a binary." - , SCAST("!!(a*a)")); \ No newline at end of file + , SCAST("!!(a*a)")); +SIMPLE_TEST0("A static cast of a scalar pointer." + , SCAST_TYPE("scalar*", "&a")); From 7a46cb9435c1d0c19dacc6c31cef404b4f1b27e8 Mon Sep 17 00:00:00 2001 From: ahueck Date: Sat, 10 Oct 2015 17:43:20 +0200 Subject: [PATCH 02/21] Small clean up/changes, preparation for source transformation --- include/core/issue/IssueHandler.h | 4 +++- include/core/issue/IssueHandler.hpp | 7 ++++--- include/core/transformation/TransformationUtil.h | 1 + src/core/issue/IssueHandler.cpp | 7 ++++++- src/core/module/ModuleContext.cpp | 1 + src/modules/AllImplicitConversion.cpp | 4 +--- src/modules/ConditionalAssgnMatcher.cpp | 4 +--- src/modules/ExplicitCast.cpp | 4 +--- src/modules/ExplicitConstructor.cpp | 4 +--- src/modules/GlobalScope.cpp | 4 +--- src/modules/ImplicitConditionMatcher.cpp | 4 +--- src/modules/ImplicitConversion.cpp | 4 +--- src/modules/UnionMatcher.cpp | 4 +--- 13 files changed, 23 insertions(+), 29 deletions(-) diff --git a/include/core/issue/IssueHandler.h b/include/core/issue/IssueHandler.h index 1532d2b..14622eb 100644 --- a/include/core/issue/IssueHandler.h +++ b/include/core/issue/IssueHandler.h @@ -25,12 +25,14 @@ class IssueHandler { private: std::string source; TUIssuesMap issues; + clang::ASTContext* ac; public: IssueHandler(); void setSource(const std::string& source); + void init(clang::ASTContext* ac); template - void addIssue(const clang::SourceManager& sm, const clang::ASTContext& ac, T node, const std::string& module, + void addIssue(T node, const std::string& module, const std::string& module_descr, std::string message = ""); TUIssuesMap& getAllIssues(); void clear(); diff --git a/include/core/issue/IssueHandler.hpp b/include/core/issue/IssueHandler.hpp index 6808118..3da1cc2 100644 --- a/include/core/issue/IssueHandler.hpp +++ b/include/core/issue/IssueHandler.hpp @@ -11,20 +11,21 @@ #include #include -#include #include +#include #include namespace opov { template -void IssueHandler::addIssue(const clang::SourceManager& sm, const clang::ASTContext& ac, T node, +void IssueHandler::addIssue(T node, const std::string& module, const std::string& module_descr, std::string message) { + auto& sm = ac->getSourceManager(); std::shared_ptr issue = std::make_shared(); std::string issue_file = clutil::fileOriginOf(sm, node); auto pos = clutil::posOf(sm, node); - std::string node_source = clutil::node2str(ac, node); + std::string node_source = clutil::node2str(*ac, node); issue->setModuleName(module); issue->setModuleDescription(module_descr); issue->setDescription(message); diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index 6e985a9..113649e 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -142,6 +142,7 @@ inline clang::tooling::Replacement insertNode(const clang::SourceManager& sm, T replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(sm, to_insert) + endl; return Replacement(sm, range.getEnd(), 0, replacement_str); } + } } diff --git a/src/core/issue/IssueHandler.cpp b/src/core/issue/IssueHandler.cpp index 358cdb3..b87a78c 100644 --- a/src/core/issue/IssueHandler.cpp +++ b/src/core/issue/IssueHandler.cpp @@ -11,13 +11,18 @@ namespace opov { IssueHandler::IssueHandler() : source("") - , issues() { + , issues() + , ac(nullptr) { } void IssueHandler::setSource(const std::string& current) { source = current; } +void IssueHandler::init(clang::ASTContext* ac) { + this->ac = ac; +} + TUIssuesMap& IssueHandler::getAllIssues() { return issues; } diff --git a/src/core/module/ModuleContext.cpp b/src/core/module/ModuleContext.cpp index ffee57e..e72773b 100644 --- a/src/core/module/ModuleContext.cpp +++ b/src/core/module/ModuleContext.cpp @@ -31,6 +31,7 @@ ModuleContext::ModuleContext(Configuration* config, IssueHandler* ihandler, Tran void ModuleContext::initContext(clang::ASTContext* context) { this->context = context; thandler->initRewriter(context->getSourceManager(), context->getLangOpts()); + ihandler->init(context); } void ModuleContext::setCurrentSource(const std::string& source) { diff --git a/src/modules/AllImplicitConversion.cpp b/src/modules/AllImplicitConversion.cpp index 0e975f7..618dc3a 100644 --- a/src/modules/AllImplicitConversion.cpp +++ b/src/modules/AllImplicitConversion.cpp @@ -43,10 +43,8 @@ void AllImplicitConversion::setupMatcher() { void AllImplicitConversion::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { const CXXConstructExpr* expr = result.Nodes.getStmtAs("conversion"); - auto& sm = context->getSourceManager(); auto& ihandle = context->getIssueHandler(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, expr, moduleName(), moduleDescription()); + ihandle.addIssue(expr, moduleName(), moduleDescription()); } std::string AllImplicitConversion::moduleName() { diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index f5d4f54..9a2f8ab 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -36,10 +36,8 @@ void ConditionalAssgnMatcher::setupMatcher() { void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { const ConditionalOperator* e = result.Nodes.getStmtAs("condassign"); - auto& sm = context->getSourceManager(); auto& ihandle = context->getIssueHandler(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, e, moduleName(), moduleDescription()); + ihandle.addIssue(e, moduleName(), moduleDescription()); } std::string ConditionalAssgnMatcher::moduleName() { diff --git a/src/modules/ExplicitCast.cpp b/src/modules/ExplicitCast.cpp index 3908248..c8585a5 100644 --- a/src/modules/ExplicitCast.cpp +++ b/src/modules/ExplicitCast.cpp @@ -48,10 +48,8 @@ void ExplicitCast::run(const clang::ast_matchers::MatchFinder::MatchResult& resu const ExplicitCastExpr* e = result.Nodes.getStmtAs("cast"); auto ecast = const_cast(e); - auto& sm = context->getSourceManager(); auto& ihandle = context->getIssueHandler(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, ecast, moduleName(), moduleDescription()); + ihandle.addIssue(ecast, moduleName(), moduleDescription()); } std::string ExplicitCast::moduleName() { diff --git a/src/modules/ExplicitConstructor.cpp b/src/modules/ExplicitConstructor.cpp index 2dd5f24..103448c 100644 --- a/src/modules/ExplicitConstructor.cpp +++ b/src/modules/ExplicitConstructor.cpp @@ -46,9 +46,7 @@ void ExplicitConstructor::run(const clang::ast_matchers::MatchFinder::MatchResul } auto& ihandle = context->getIssueHandler(); - auto& sm = context->getSourceManager(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, ctor, moduleName(), moduleDescription()); + ihandle.addIssue(ctor, moduleName(), moduleDescription()); } std::string ExplicitConstructor::moduleName() { diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index f389e1d..5495e86 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -36,9 +36,7 @@ void GlobalScope::run(const clang::ast_matchers::MatchFinder::MatchResult& resul const Expr* call = result.Nodes.getStmtAs("global"); auto& ihandle = context->getIssueHandler(); - auto& sm = context->getSourceManager(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, call, moduleName(), moduleDescription()); + ihandle.addIssue(call, moduleName(), moduleDescription()); } std::string GlobalScope::moduleName() { diff --git a/src/modules/ImplicitConditionMatcher.cpp b/src/modules/ImplicitConditionMatcher.cpp index e6e5031..5177acf 100644 --- a/src/modules/ImplicitConditionMatcher.cpp +++ b/src/modules/ImplicitConditionMatcher.cpp @@ -54,9 +54,7 @@ void ImplicitConditionMatcher::run(const clang::ast_matchers::MatchFinder::Match auto& ihandle = context->getIssueHandler(); - auto& sm = context->getSourceManager(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, invalid, moduleName(), moduleDescription()); + ihandle.addIssue(invalid, moduleName(), moduleDescription()); } std::string ImplicitConditionMatcher::moduleName() { diff --git a/src/modules/ImplicitConversion.cpp b/src/modules/ImplicitConversion.cpp index 24f4468..f9e4328 100644 --- a/src/modules/ImplicitConversion.cpp +++ b/src/modules/ImplicitConversion.cpp @@ -43,9 +43,7 @@ void ImplicitConversion::run(const clang::ast_matchers::MatchFinder::MatchResult const CXXConstructExpr* expr = result.Nodes.getStmtAs("conversion"); auto& ihandle = context->getIssueHandler(); - auto& sm = context->getSourceManager(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, expr, moduleName(), moduleDescription()); //, message.str()); + ihandle.addIssue(expr, moduleName(), moduleDescription()); //, message.str()); } std::string ImplicitConversion::moduleName() { diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index 9f0f83f..8510747 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -45,9 +45,7 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu << " fieldDecl violate the convention."; auto& ihandle = context->getIssueHandler(); - auto& sm = context->getSourceManager(); - auto& ac = context->getASTContext(); - ihandle.addIssue(sm, ac, inv_union, moduleName(), moduleDescription(), message.str()); + ihandle.addIssue(inv_union, moduleName(), moduleDescription(), message.str()); } std::string UnionMatcher::moduleName() { From ab93418d9e440e74ebdb11876e1785ae8f732bec Mon Sep 17 00:00:00 2001 From: ahueck Date: Sun, 11 Oct 2015 22:16:29 +0200 Subject: [PATCH 03/21] Added unique filter for issues. TODO: filtered results miss the files where the problems were found --- CMakeLists.txt | 2 + include/core/Application.h | 6 +++ include/core/issue/Issue.h | 12 +++++ include/core/issue/filter/FilterIssueStruct.h | 26 ++++++++++ include/core/issue/filter/Filtering.h | 24 +++++++++ include/core/issue/filter/IFilter.h | 21 ++++++++ include/core/issue/filter/UniqueFilter.h | 20 ++++++++ src/core/Application.cpp | 51 +++++++++---------- src/core/issue/filter/Filtering.cpp | 49 ++++++++++++++++++ src/core/issue/filter/UniqueFilter.cpp | 38 ++++++++++++++ src/core/reporting/CSVReporter.cpp | 1 + 11 files changed, 222 insertions(+), 28 deletions(-) create mode 100644 include/core/issue/filter/FilterIssueStruct.h create mode 100644 include/core/issue/filter/Filtering.h create mode 100644 include/core/issue/filter/IFilter.h create mode 100644 include/core/issue/filter/UniqueFilter.h create mode 100644 src/core/issue/filter/Filtering.cpp create mode 100644 src/core/issue/filter/UniqueFilter.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index b414868..98ba3d5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,6 +20,8 @@ src/core/configuration/JSONConfiguration.cpp include/external/IncludeDirectives.cpp include/external/ReplacementHandling.cpp src/core/issue/IssueHandler.cpp +src/core/issue/filter/Filtering.cpp +src/core/issue/filter/UniqueFilter.cpp src/core/reporting/ConsoleReporter.cpp src/core/reporting/CSVReporter.cpp src/core/transformation/TransformationHandler.cpp diff --git a/include/core/Application.h b/include/core/Application.h index 62fa5e6..8eb9449 100644 --- a/include/core/Application.h +++ b/include/core/Application.h @@ -27,6 +27,9 @@ class IssueReporter; class IssueHandler; class TransformationHandler; class Module; +namespace filter { +class IFilter; +} class Application { protected: @@ -36,17 +39,20 @@ class Application { std::unique_ptr actionFactory; std::unique_ptr ihandler; std::unique_ptr thandler; + std::unique_ptr filter; std::vector modules; virtual void loadConfig() = 0; virtual void createReporter() = 0; virtual void createIssueHandler(); + virtual void createFilter(); virtual void createTransformationHandler(); virtual void createFactory() = 0; virtual void createActionFactory(); virtual void initModules() = 0; public: + Application(); virtual void init(); virtual void cleanUp(); virtual int execute(const clang::tooling::CompilationDatabase& db, const std::vector& sources); diff --git a/include/core/issue/Issue.h b/include/core/issue/Issue.h index 0cd0c87..1d8193e 100644 --- a/include/core/issue/Issue.h +++ b/include/core/issue/Issue.h @@ -10,6 +10,8 @@ #include "map/PropertyMap.h" +#include + namespace opov { class Issue { @@ -19,9 +21,19 @@ class Issue { public: Issue() { } + const property_map& properties() { return _properties; } + + int hash() { + int hash = 7; + int pos_hash = getLineStart() ^ (getColumnStart() << 4) ^ (getLineEnd() << 8) ^ (getColumnEnd() << 16); + hash += std::hash()(getFile()); + hash = 17 * hash + std::hash()(getModuleName()); + return hash ^ pos_hash; + } + int IssueProperty(LineStart); int IssueProperty(LineEnd); int IssueProperty(ColumnStart); diff --git a/include/core/issue/filter/FilterIssueStruct.h b/include/core/issue/filter/FilterIssueStruct.h new file mode 100644 index 0000000..29ad401 --- /dev/null +++ b/include/core/issue/filter/FilterIssueStruct.h @@ -0,0 +1,26 @@ +#ifndef FILTER_ISSUE_STRUCT_H +#define FILTER_ISSUE_STRUCT_H + +#include "../Issue.h" + +#include +#include + +namespace opov { +namespace filter { + +struct FilterIssue { + int id; + std::shared_ptr issue; + + /*bool operator==(const FilterIssue& rhs) const { + return id == rhs.id && issue == rhs.issue; + }*/ +}; + +typedef std::map FilterIssueMap; + +} +} + +#endif diff --git a/include/core/issue/filter/Filtering.h b/include/core/issue/filter/Filtering.h new file mode 100644 index 0000000..4c06a5d --- /dev/null +++ b/include/core/issue/filter/Filtering.h @@ -0,0 +1,24 @@ +#ifndef FILTERING_H +#define FILTERING_H + +#include "../IssueHandlerStruct.h" + +namespace opov { +namespace filter { + +class IFilter; + +class Filtering { +public: + Filtering(IFilter* mainFilter); + ~Filtering(); + + TUIssuesMap filter(const TUIssuesMap& unfilteredMap); +private: + IFilter* mainFilter; +}; + +} +} + +#endif diff --git a/include/core/issue/filter/IFilter.h b/include/core/issue/filter/IFilter.h new file mode 100644 index 0000000..202c842 --- /dev/null +++ b/include/core/issue/filter/IFilter.h @@ -0,0 +1,21 @@ +#ifndef IFILTER_H +#define IFILTER_H + +#include "FilterIssueStruct.h" +#include "../Issue.h" + +namespace opov { +namespace filter { + +class IFilter { +public: + virtual FilterIssueMap apply(const FilterIssueMap& map) = 0; + virtual ~IFilter() { + + } +}; + +} +} + +#endif diff --git a/include/core/issue/filter/UniqueFilter.h b/include/core/issue/filter/UniqueFilter.h new file mode 100644 index 0000000..a0d13b5 --- /dev/null +++ b/include/core/issue/filter/UniqueFilter.h @@ -0,0 +1,20 @@ +#ifndef UNIQUE_FILTER_H +#define UNIQUE_FILTER_H + +#include "IFilter.h" + +namespace opov { +namespace filter { + +class UniqueFilter: public IFilter { +public: + UniqueFilter(); + virtual ~UniqueFilter(); + + virtual FilterIssueMap apply(const FilterIssueMap& map) override; +}; + +} +} + +#endif diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 8fccd5a..9a5a480 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -13,9 +13,12 @@ #include #include #include +#include +#include +#include #include -#include +//#include #include #include @@ -30,12 +33,14 @@ namespace opov { -// Application::Application() : executor(nullptr) { -//} +Application::Application() { + +} void Application::init() { loadConfig(); createIssueHandler(); + createFilter(); createTransformationHandler(); createReporter(); createFactory(); @@ -51,23 +56,17 @@ void Application::createIssueHandler() { ihandler = util::make_unique(); } +void Application::createFilter() { + filter = util::make_unique(); +} + void Application::createTransformationHandler() { thandler = util::make_unique(); } int Application::execute(const clang::tooling::CompilationDatabase& db, const std::vector& sources) { clang::tooling::ClangTool tool(db, sources); - ReplacementHandling replacementHandler; - std::string replacement_loc; - bool apply_replacements; - config->getValue("replacement:location", replacement_loc); - config->getValue("replacement:apply", apply_replacements); int sig = 0; - - if (!replacementHandler.findClangApplyReplacements("")) { - LOG_ERROR("Could not find clang-apply-replacement"); - } - for (auto module : modules) { executor->setModule(module); int sig = tool.run(actionFactory.get()); @@ -77,21 +76,13 @@ int Application::execute(const clang::tooling::CompilationDatabase& db, const st } } - reporter->addIssues(ihandler->getAllIssues()); + TUIssuesMap& issuesMap = ihandler->getAllIssues(); + filter::Filtering filtering(filter.get()); + TUIssuesMap filteredMap = filtering.filter(issuesMap); + + reporter->addIssues(filteredMap); ihandler->clear(); - /* - replacementHandler.setDestinationDir(replacement_loc); - if(!replacementHandler.serializeReplacements(thandler->getAllReplacements())) - { - LOG_DEBUG("Failed to serialize replacements"); - } - if(apply_replacements) { - if(!replacementHandler.applyReplacements()) { - LOG_DEBUG("Failed to apply replacements"); - } - } - thandler->getAllReplacements().clear(); - */ + return sig; } @@ -106,7 +97,11 @@ int Application::executeOnCode(const std::string& source, const std::vectoraddIssues(ihandler->getAllIssues()); + TUIssuesMap& issuesMap = ihandler->getAllIssues(); + filter::Filtering filtering(filter.get()); + TUIssuesMap filteredMap = filtering.filter(issuesMap); + + reporter->addIssues(filteredMap); ihandler->clear(); return sig; diff --git a/src/core/issue/filter/Filtering.cpp b/src/core/issue/filter/Filtering.cpp new file mode 100644 index 0000000..1cf3844 --- /dev/null +++ b/src/core/issue/filter/Filtering.cpp @@ -0,0 +1,49 @@ +#include +#include +#include + +#include + +namespace opov { +namespace filter { + +Filtering::Filtering(IFilter* mainFilter) : mainFilter(mainFilter) { +} + +Filtering::~Filtering() { +} + +TUIssuesMap Filtering::filter(const TUIssuesMap& unfilteredIssues) { + + //First give all issues a unique ID + FilterIssueMap filterIssueMap; + int id = 0; + for (auto& unit : unfilteredIssues) { + for (auto& issue : unit.second.Issues) { + FilterIssue fi; + fi.id = id; + fi.issue = issue; + filterIssueMap[id] = fi; + id++; + } + } + + if (mainFilter != nullptr) { + // Filtering + filterIssueMap = mainFilter->apply(filterIssueMap); + } + + // Transform map back into previous format + TUIssuesMap resultMap; + + for (auto& filterIssue : filterIssueMap) { + auto issue = filterIssue.second.issue; + std::string moduleName = issue->getModuleName(); + resultMap[moduleName].Issues.push_back(issue); + } + + return resultMap; +} + +} +} diff --git a/src/core/issue/filter/UniqueFilter.cpp b/src/core/issue/filter/UniqueFilter.cpp new file mode 100644 index 0000000..2b2ff86 --- /dev/null +++ b/src/core/issue/filter/UniqueFilter.cpp @@ -0,0 +1,38 @@ +#include +#include + +#include + +namespace opov { +namespace filter { + +UniqueFilter::UniqueFilter() { + +} + +UniqueFilter::~UniqueFilter() { + +} + +FilterIssueMap UniqueFilter::apply(const FilterIssueMap& map) { + std::map duplicates; + + FilterIssueMap filteredMap; + + for (auto& filterIssue : map) { + int hash = filterIssue.second.issue->hash(); + if (duplicates.find(hash) == duplicates.end()) { + filteredMap[filterIssue.first] = filterIssue.second; + duplicates[hash] = 0; + } else { + duplicates[hash]++; + } + } + + // TODO: What happens to the meta data? + return filteredMap; + +} + +} +} diff --git a/src/core/reporting/CSVReporter.cpp b/src/core/reporting/CSVReporter.cpp index 4169477..176b4de 100644 --- a/src/core/reporting/CSVReporter.cpp +++ b/src/core/reporting/CSVReporter.cpp @@ -27,6 +27,7 @@ void CSVReporter::addIssues(const TUIssuesMap& issues) { void CSVReporter::print(const TranslationUnitIssues& issue) { std::stringstream csv; + csv << "Main source;Module Name;File Path;Code;Line Start;Line End;Column Start;Column End\n"; for (auto& i : issue.Issues) { csv << issue.MainSourceFile << ";"; csv << i->getModuleName() << ";"; From 32cb0aa4349dff19d5f982b65f26b5cc714a5700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Mon, 12 Oct 2015 16:31:09 +0200 Subject: [PATCH 04/21] Modified filtering, now results are stored in a special data structure and carry information in which analyzed TUs the issue occured. Reporter are now able to handle this data structure. TODO temporary solution, better would be to have a abstraction between filtered and unfiltered view. --- CMakeLists.txt | 1 - include/core/issue/filter/FilterIssueStruct.h | 15 +++--- include/core/issue/filter/Filtering.h | 24 --------- include/core/issue/filter/IFilter.h | 3 +- include/core/issue/filter/UniqueFilter.h | 2 +- include/core/reporting/CSVReporter.h | 1 + include/core/reporting/ConsoleReporter.h | 1 + include/core/reporting/IssueReporter.h | 2 + src/core/Application.cpp | 25 +++++++--- src/core/issue/filter/Filtering.cpp | 49 ------------------- src/core/issue/filter/UniqueFilter.cpp | 35 +++++++------ src/core/reporting/CSVReporter.cpp | 20 ++++++++ src/core/reporting/ConsoleReporter.cpp | 4 ++ test/data/test_conf.json | 1 + test/data/test_double_conf.json | 1 + test/include/MockReporter.h | 4 ++ 16 files changed, 81 insertions(+), 107 deletions(-) delete mode 100644 include/core/issue/filter/Filtering.h delete mode 100644 src/core/issue/filter/Filtering.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 98ba3d5..014e3ce 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,7 +20,6 @@ src/core/configuration/JSONConfiguration.cpp include/external/IncludeDirectives.cpp include/external/ReplacementHandling.cpp src/core/issue/IssueHandler.cpp -src/core/issue/filter/Filtering.cpp src/core/issue/filter/UniqueFilter.cpp src/core/reporting/ConsoleReporter.cpp src/core/reporting/CSVReporter.cpp diff --git a/include/core/issue/filter/FilterIssueStruct.h b/include/core/issue/filter/FilterIssueStruct.h index 29ad401..ecfae39 100644 --- a/include/core/issue/filter/FilterIssueStruct.h +++ b/include/core/issue/filter/FilterIssueStruct.h @@ -5,20 +5,23 @@ #include #include +#include namespace opov { namespace filter { -struct FilterIssue { - int id; +struct IssuesFiltered { std::shared_ptr issue; + std::vector tunits; +}; - /*bool operator==(const FilterIssue& rhs) const { - return id == rhs.id && issue == rhs.issue; - }*/ +struct issue_compare { + bool operator() (const IssuesFiltered& a, const IssuesFiltered& b) { + return a.issue->hash() != b.issue->hash(); + } }; -typedef std::map FilterIssueMap; +typedef std::map IssueSet; } } diff --git a/include/core/issue/filter/Filtering.h b/include/core/issue/filter/Filtering.h deleted file mode 100644 index 4c06a5d..0000000 --- a/include/core/issue/filter/Filtering.h +++ /dev/null @@ -1,24 +0,0 @@ -#ifndef FILTERING_H -#define FILTERING_H - -#include "../IssueHandlerStruct.h" - -namespace opov { -namespace filter { - -class IFilter; - -class Filtering { -public: - Filtering(IFilter* mainFilter); - ~Filtering(); - - TUIssuesMap filter(const TUIssuesMap& unfilteredMap); -private: - IFilter* mainFilter; -}; - -} -} - -#endif diff --git a/include/core/issue/filter/IFilter.h b/include/core/issue/filter/IFilter.h index 202c842..bc7cf62 100644 --- a/include/core/issue/filter/IFilter.h +++ b/include/core/issue/filter/IFilter.h @@ -1,6 +1,7 @@ #ifndef IFILTER_H #define IFILTER_H +#include "../IssueHandlerStruct.h" #include "FilterIssueStruct.h" #include "../Issue.h" @@ -9,7 +10,7 @@ namespace filter { class IFilter { public: - virtual FilterIssueMap apply(const FilterIssueMap& map) = 0; + virtual IssueSet apply(const TUIssuesMap& map) = 0; virtual ~IFilter() { } diff --git a/include/core/issue/filter/UniqueFilter.h b/include/core/issue/filter/UniqueFilter.h index a0d13b5..e763e9a 100644 --- a/include/core/issue/filter/UniqueFilter.h +++ b/include/core/issue/filter/UniqueFilter.h @@ -11,7 +11,7 @@ class UniqueFilter: public IFilter { UniqueFilter(); virtual ~UniqueFilter(); - virtual FilterIssueMap apply(const FilterIssueMap& map) override; + virtual IssueSet apply(const TUIssuesMap& map) override; }; } diff --git a/include/core/reporting/CSVReporter.h b/include/core/reporting/CSVReporter.h index f57e14f..d58e58c 100644 --- a/include/core/reporting/CSVReporter.h +++ b/include/core/reporting/CSVReporter.h @@ -19,6 +19,7 @@ class CSVReporter : public opov::IssueReporter { CSVReporter(); virtual void addIssue(const TranslationUnitIssues& issue) override; virtual void addIssues(const TUIssuesMap& issues) override; + virtual void addIssues(const filter::IssueSet& issues) override; virtual ~CSVReporter(); private: diff --git a/include/core/reporting/ConsoleReporter.h b/include/core/reporting/ConsoleReporter.h index 0157986..d1b8f33 100644 --- a/include/core/reporting/ConsoleReporter.h +++ b/include/core/reporting/ConsoleReporter.h @@ -19,6 +19,7 @@ class ConsoleReporter : public opov::IssueReporter { ConsoleReporter(); virtual void addIssue(const TranslationUnitIssues& issue) override; virtual void addIssues(const TUIssuesMap& issues) override; + virtual void addIssues(const filter::IssueSet& issues) override; virtual ~ConsoleReporter(); private: diff --git a/include/core/reporting/IssueReporter.h b/include/core/reporting/IssueReporter.h index f57ea86..8d39d8b 100644 --- a/include/core/reporting/IssueReporter.h +++ b/include/core/reporting/IssueReporter.h @@ -9,6 +9,7 @@ #define ISSUEREPORTER_H_ #include "../issue/IssueHandlerStruct.h" +#include "../issue/filter/FilterIssueStruct.h" #include #include @@ -19,6 +20,7 @@ class IssueReporter { public: virtual void addIssue(const TranslationUnitIssues& issue) = 0; virtual void addIssues(const TUIssuesMap& issues) = 0; + virtual void addIssues(const filter::IssueSet& issues) = 0; virtual ~IssueReporter() { } }; diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 9a5a480..129a005 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include //#include @@ -76,11 +75,17 @@ int Application::execute(const clang::tooling::CompilationDatabase& db, const st } } + bool do_filter; + config->getValue("filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); - filter::Filtering filtering(filter.get()); - TUIssuesMap filteredMap = filtering.filter(issuesMap); - reporter->addIssues(filteredMap); + if(do_filter) { + auto filteredSet = filter->apply(issuesMap); + reporter->addIssues(filteredSet); + } else { + reporter->addIssues(issuesMap); + } + ihandler->clear(); return sig; @@ -97,11 +102,17 @@ int Application::executeOnCode(const std::string& source, const std::vectorgetValue("filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); - filter::Filtering filtering(filter.get()); - TUIssuesMap filteredMap = filtering.filter(issuesMap); - reporter->addIssues(filteredMap); + if(do_filter) { + auto filteredSet = filter->apply(issuesMap); + reporter->addIssues(filteredSet); + } else { + reporter->addIssues(issuesMap); + } + ihandler->clear(); return sig; diff --git a/src/core/issue/filter/Filtering.cpp b/src/core/issue/filter/Filtering.cpp deleted file mode 100644 index 1cf3844..0000000 --- a/src/core/issue/filter/Filtering.cpp +++ /dev/null @@ -1,49 +0,0 @@ -#include -#include -#include - -#include - -namespace opov { -namespace filter { - -Filtering::Filtering(IFilter* mainFilter) : mainFilter(mainFilter) { -} - -Filtering::~Filtering() { -} - -TUIssuesMap Filtering::filter(const TUIssuesMap& unfilteredIssues) { - - //First give all issues a unique ID - FilterIssueMap filterIssueMap; - int id = 0; - for (auto& unit : unfilteredIssues) { - for (auto& issue : unit.second.Issues) { - FilterIssue fi; - fi.id = id; - fi.issue = issue; - filterIssueMap[id] = fi; - id++; - } - } - - if (mainFilter != nullptr) { - // Filtering - filterIssueMap = mainFilter->apply(filterIssueMap); - } - - // Transform map back into previous format - TUIssuesMap resultMap; - - for (auto& filterIssue : filterIssueMap) { - auto issue = filterIssue.second.issue; - std::string moduleName = issue->getModuleName(); - resultMap[moduleName].Issues.push_back(issue); - } - - return resultMap; -} - -} -} diff --git a/src/core/issue/filter/UniqueFilter.cpp b/src/core/issue/filter/UniqueFilter.cpp index 2b2ff86..d6fc5a6 100644 --- a/src/core/issue/filter/UniqueFilter.cpp +++ b/src/core/issue/filter/UniqueFilter.cpp @@ -14,24 +14,23 @@ UniqueFilter::~UniqueFilter() { } -FilterIssueMap UniqueFilter::apply(const FilterIssueMap& map) { - std::map duplicates; - - FilterIssueMap filteredMap; - - for (auto& filterIssue : map) { - int hash = filterIssue.second.issue->hash(); - if (duplicates.find(hash) == duplicates.end()) { - filteredMap[filterIssue.first] = filterIssue.second; - duplicates[hash] = 0; - } else { - duplicates[hash]++; - } - } - - // TODO: What happens to the meta data? - return filteredMap; - +IssueSet UniqueFilter::apply(const TUIssuesMap& unfilteredIssues) { + IssueSet unique_issues; + for(auto& tu_issues : unfilteredIssues) { + for(auto& issue : tu_issues.second.Issues) { + const int ihash =issue->hash(); + auto result = unique_issues.find(ihash); + if(result != std::end(unique_issues)) { + result->second.tunits.push_back(tu_issues.second.MainSourceFile); + } else { + IssuesFiltered ifiltered; + ifiltered.issue = issue; + ifiltered.tunits.push_back(tu_issues.second.MainSourceFile); + unique_issues[ihash] = ifiltered; + } + } + } + return unique_issues; } } diff --git a/src/core/reporting/CSVReporter.cpp b/src/core/reporting/CSVReporter.cpp index 176b4de..bd38ce1 100644 --- a/src/core/reporting/CSVReporter.cpp +++ b/src/core/reporting/CSVReporter.cpp @@ -19,12 +19,32 @@ CSVReporter::CSVReporter() { void CSVReporter::addIssue(const TranslationUnitIssues& issue) { print(issue); } + void CSVReporter::addIssues(const TUIssuesMap& issues) { for (auto& unit : issues) { print(unit.second); } } +void CSVReporter::addIssues(const filter::IssueSet& set) { + std::stringstream csv; + csv << "Module Name;File Path;Code;Line Start;Line End;Column Start;Column End;Files\n"; + for (auto& ifiltered : set) { + auto i = ifiltered.second.issue; + csv << i->getModuleName() << ";"; + csv << i->getFile() << ";"; + csv << i->getCode() << ";"; + csv << i->getLineStart() << ";" << i->getLineEnd() << ";" << i->getColumnStart() << ";" << i->getColumnEnd() << ";"; + csv << "+"; + for (auto& tunit : ifiltered.second.tunits) { + csv << "+"; + } + LOG_MSG(csv.str()); + csv.str(""); + csv.clear(); + } +} + void CSVReporter::print(const TranslationUnitIssues& issue) { std::stringstream csv; csv << "Main source;Module Name;File Path;Code;Line Start;Line End;Column Start;Column End\n"; diff --git a/src/core/reporting/ConsoleReporter.cpp b/src/core/reporting/ConsoleReporter.cpp index 248131b..d60a237 100644 --- a/src/core/reporting/ConsoleReporter.cpp +++ b/src/core/reporting/ConsoleReporter.cpp @@ -23,6 +23,10 @@ void ConsoleReporter::addIssues(const TUIssuesMap& issues) { } } +void ConsoleReporter::addIssues(const filter::IssueSet& set) { + LOG_ERROR("Not yet implemented!"); +} + void ConsoleReporter::print(const TranslationUnitIssues& issue) { std::string module = ""; LOG_MSG(issue.MainSourceFile << ":"); diff --git a/test/data/test_conf.json b/test/data/test_conf.json index a908931..083ebc2 100644 --- a/test/data/test_conf.json +++ b/test/data/test_conf.json @@ -1,5 +1,6 @@ { "global" : { + "filter" : false, "type" : "scalar" } } diff --git a/test/data/test_double_conf.json b/test/data/test_double_conf.json index 7417c4c..327ef6c 100644 --- a/test/data/test_double_conf.json +++ b/test/data/test_double_conf.json @@ -1,5 +1,6 @@ { "global" : { + "filter" : false, "type" : "double" } } diff --git a/test/include/MockReporter.h b/test/include/MockReporter.h index 27d0cc4..c2809cd 100644 --- a/test/include/MockReporter.h +++ b/test/include/MockReporter.h @@ -46,6 +46,10 @@ class MockReporter: public opov::IssueReporter { } } + virtual void addIssues(const filter::IssueSet& issues) { + LOG_DEBUG("Not supposed to be called!"); + } + virtual ~MockReporter() { } From 703fc25e7738d16e705b182f41fb8714f91f491e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Mon, 12 Oct 2015 16:54:44 +0200 Subject: [PATCH 05/21] Fix to csv output function for filtered issues --- conf.json | 1 + src/core/Application.cpp | 4 ++-- src/core/reporting/CSVReporter.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/conf.json b/conf.json index b608682..7c0c822 100644 --- a/conf.json +++ b/conf.json @@ -1,5 +1,6 @@ { "global" : { + "filter" : true, "type" : "scalar" } } diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 129a005..71a1f10 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -76,7 +76,7 @@ int Application::execute(const clang::tooling::CompilationDatabase& db, const st } bool do_filter; - config->getValue("filter", do_filter, false); + config->getValue("global:filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); if(do_filter) { @@ -103,7 +103,7 @@ int Application::executeOnCode(const std::string& source, const std::vectorgetValue("filter", do_filter, false); + config->getValue("global:filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); if(do_filter) { diff --git a/src/core/reporting/CSVReporter.cpp b/src/core/reporting/CSVReporter.cpp index bd38ce1..6b495b5 100644 --- a/src/core/reporting/CSVReporter.cpp +++ b/src/core/reporting/CSVReporter.cpp @@ -37,7 +37,7 @@ void CSVReporter::addIssues(const filter::IssueSet& set) { csv << i->getLineStart() << ";" << i->getLineEnd() << ";" << i->getColumnStart() << ";" << i->getColumnEnd() << ";"; csv << "+"; for (auto& tunit : ifiltered.second.tunits) { - csv << "+"; + csv << tunit << "+"; } LOG_MSG(csv.str()); csv.str(""); From a7815e3f4966fa405caaca2da2a620905c9cbc9a Mon Sep 17 00:00:00 2001 From: ahueck Date: Tue, 13 Oct 2015 17:40:46 +0200 Subject: [PATCH 06/21] Added ST capabilities for several matchers. Not fully tested. --- conf.json | 6 +- include/core/Application.h | 1 + include/core/module/AbstractModule.h | 2 + .../transformation/TransformationHandler.h | 7 +- .../core/transformation/TransformationUtil.h | 175 ++++++++++-------- include/core/utility/ClangUtil.h | 19 +- include/modules/ConditionalAssgnMatcher.h | 5 +- include/modules/ExplicitCast.h | 1 - include/modules/GlobalScope.h | 5 +- include/modules/ImplicitConditionMatcher.h | 5 +- include/modules/ImplicitConversion.h | 6 +- include/modules/UnionMatcher.h | 1 - src/core/AbstractFactory.cpp | 3 + src/core/Application.cpp | 21 +-- src/core/module/ASTMatcherModule.cpp | 3 + src/core/module/AbstractModule.cpp | 5 +- .../transformation/TransformationHandler.cpp | 4 + src/modules/AllImplicitConversion.cpp | 2 +- src/modules/ConditionalAssgnMatcher.cpp | 2 +- src/modules/ExplicitCast.cpp | 16 +- src/modules/GlobalScope.cpp | 2 +- src/modules/ImplicitConditionMatcher.cpp | 19 +- src/modules/ImplicitConversion.cpp | 8 +- src/modules/UnionMatcher.cpp | 2 +- 24 files changed, 186 insertions(+), 134 deletions(-) diff --git a/conf.json b/conf.json index 7c0c822..c9bc3f8 100644 --- a/conf.json +++ b/conf.json @@ -1,6 +1,10 @@ { "global" : { + "transform" : true, "filter" : true, "type" : "scalar" - } + }, + "ExplicitCast" : { + "header" : "recast.h" + } } diff --git a/include/core/Application.h b/include/core/Application.h index 8eb9449..d73a739 100644 --- a/include/core/Application.h +++ b/include/core/Application.h @@ -58,6 +58,7 @@ class Application { virtual int execute(const clang::tooling::CompilationDatabase& db, const std::vector& sources); virtual int executeOnCode(const std::string& source, const std::vector& args = std::vector()); + virtual void report(); virtual void addModule(Module* module); virtual std::string getApplicationName(); virtual ~Application(); diff --git a/include/core/module/AbstractModule.h b/include/core/module/AbstractModule.h index 9b1478d..eaa5eee 100644 --- a/include/core/module/AbstractModule.h +++ b/include/core/module/AbstractModule.h @@ -24,6 +24,8 @@ class Configuration; class AbstractModule : public Module { protected: ModuleContext* context; + std::string type_s; + bool transform; public: AbstractModule(); diff --git a/include/core/transformation/TransformationHandler.h b/include/core/transformation/TransformationHandler.h index 76e6767..5b96cb0 100644 --- a/include/core/transformation/TransformationHandler.h +++ b/include/core/transformation/TransformationHandler.h @@ -40,10 +40,15 @@ class TransformationHandler { TransformationHandler(); void setSource(const std::string& current); void initRewriter(clang::SourceManager& sm, const clang::LangOptions& langOpts); + void setIncludeDirectives(IncludeDirectives* include); + + void addHeader(const std::string& header, clang::SourceLocation loc); void addReplacements(const clang::tooling::Replacement& replacement); void addReplacements(const std::vector& replacements); TUReplacementsMap& getAllReplacements(); - void setIncludeDirectives(IncludeDirectives* include); + + + IncludeDirectives* getIncludeDirectives(); clang::Rewriter& getRewriter(); virtual ~TransformationHandler(); diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index 113649e..cb08f86 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -54,94 +54,111 @@ inline StringRef whitespaces(const clang::SourceManager& sm, T node) { return spaces; } -inline clang::tooling::Replacement removeCastFromExpr(const clang::SourceManager& sm, - const clang::ExplicitCastExpr* expr) { - SourceRange range = locOf(sm, expr); - CharSourceRange cast_crange = CharSourceRange::getCharRange(range); - StringRef replacement_str(clutil::node2str(sm, expr->getSubExprAsWritten())); - return Replacement(sm, cast_crange, replacement_str); +inline clang::tooling::Replacement reCast(const clang::ASTContext& ac, const clang::ExplicitCastExpr* expr, const std::string& cast_from) { + auto& sm = ac.getSourceManager(); + //SourceRange range = locOf(sm, expr); + //CharSourceRange cast_crange = CharSourceRange::getCharRange(range); + std::string text("reCast<"); + text += typeOf(expr) + "," + cast_from + ">(" + clutil::node2str(ac, expr->getSubExprAsWritten()) + ")"; + return Replacement(sm, expr, text); } -inline std::vector addExplicitCompare(const clang::SourceManager& sm, - const clang::Expr* expr, StringRef with_type) { - SourceRange range = locOf(sm, expr); - SmallString<32> replace(" == "); - replace += with_type; - replace += "(0.0)"; - if (isa(expr)) { - replace += ")"; - return {Replacement(sm, llvm::dyn_cast(expr)->getSubExpr()->getLocStart(), 0, "("), - Replacement(sm, range.getEnd(), 0, replace)}; - } else { - return {Replacement(sm, range.getEnd(), 0, replace)}; - } -} -/* -inline std::vector addExplicitCompare(const -clang::SourceManager& sm, const clang::UnaryOperator* expr, StringRef with_type) -{ - SourceRange range = locOf(sm, expr); - SmallString<32> replace(" == "); - replace += with_type; - replace += "(0.0))"; - return {Replacement(sm, expr->getSubExpr()->getLocStart(), 0, "("), -Replacement(sm, range.getEnd(), 0, replace)}; -} -*/ -inline clang::tooling::Replacement insertString(const clang::SourceManager& sm, const clang::Stmt* expr, StringRef str, - bool before = false) { - SourceRange range = locOf(sm, expr); - std::string formattedStr; - if (before) { - formattedStr = str; - formattedStr += whitespaces(sm, expr); - } else { - formattedStr = whitespaces(sm, expr); - formattedStr += str; - } - return Replacement(sm, before ? range.getBegin() : range.getEnd(), 0, formattedStr); +inline clang::tooling::Replacement removeCastFromExpr(const clang::ASTContext& ac, + const clang::ExplicitCastExpr* expr) { + auto& sm = ac.getSourceManager(); + StringRef replacement_str(clutil::node2str(ac, expr->getSubExprAsWritten())); + return Replacement(sm, expr, replacement_str); } -inline std::vector castTheExpr(const clang::SourceManager& sm, const clang::Expr* expr, - const StringRef type, - int cast = Expr::CXXFunctionalCastExprClass) { - SmallString<32> castStr; - switch (cast) { - case Expr::CXXStaticCastExprClass: - castStr = "static_cast<"; - castStr += type; - castStr += ">"; - break; - case Expr::CXXFunctionalCastExprClass: - default: - castStr = type; +inline clang::tooling::Replacement addExplicitCompare(const clang::ASTContext& ac, + const clang::Expr* expr, const std::string& with_type) { + auto& sm = ac.getSourceManager(); + bool unary = isa(expr); + auto texpr = unary ? llvm::dyn_cast(expr)->getSubExpr() : expr; + std::string expr_str = clutil::node2str(ac, texpr) + " == " + with_type + "(0.0)"; + + if(unary) { + return Replacement(sm, texpr, "(" + expr_str + ")"); } - castStr += "("; - SourceRange range = locOf(sm, expr); - return {Replacement(sm, range.getBegin(), 0, castStr), Replacement(sm, range.getEnd(), 0, ")")}; + return Replacement(sm, texpr, expr_str); } -template -inline clang::tooling::Replacement removeNode(const clang::SourceManager& sm, T node) { - // FIXME does not remove empty line - SourceRange loc = locOf(sm, node, 1); - CharSourceRange cast_crange = CharSourceRange::getCharRange(loc); - return Replacement(sm, cast_crange, ""); +inline clang::tooling::Replacement castTheExpr(const clang::ASTContext& ac, const clang::Expr* expr, + const std::string& type) { + auto& sm = ac.getSourceManager(); + std::string castStr = type; + castStr += "(" + clutil::node2str(ac, expr) + ")"; + return Replacement(sm, expr, castStr); } -template -inline clang::tooling::Replacement insertNode(const clang::SourceManager& sm, T to_insert, U relative_to, - bool before = false, char endl = ';') { - // FIXME does not preserve indentation - SourceRange range = locOf(sm, relative_to, 1); - StringRef replacement_str; - if (before) { - replacement_str = clutil::node2str(sm, to_insert) + endl + "\n" + whitespaces(sm, relative_to).str(); - return Replacement(sm, range.getBegin(), 0, replacement_str); - } - replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(sm, to_insert) + endl; - return Replacement(sm, range.getEnd(), 0, replacement_str); -} + + +/* +//inline std::vector addExplicitCompare(const +//clang::SourceManager& sm, const clang::UnaryOperator* expr, StringRef with_type) +//{ +// SourceRange range = locOf(sm, expr); +// SmallString<32> replace(" == "); +// replace += with_type; +// replace += "(0.0))"; +// return {Replacement(sm, expr->getSubExpr()->getLocStart(), 0, "("), +//Replacement(sm, range.getEnd(), 0, replace)}; +//} +//*/ +//inline clang::tooling::Replacement insertString(const clang::SourceManager& sm, const clang::Stmt* expr, StringRef str, +// bool before = false) { +// SourceRange range = locOf(sm, expr); +// std::string formattedStr; +// if (before) { +// formattedStr = str; +// formattedStr += whitespaces(sm, expr); +// } else { +// formattedStr = whitespaces(sm, expr); +// formattedStr += str; +// } +// return Replacement(sm, before ? range.getBegin() : range.getEnd(), 0, formattedStr); +//} +// +//inline std::vector castTheExpr(const clang::SourceManager& sm, const clang::Expr* expr, +// const StringRef type, +// int cast = Expr::CXXFunctionalCastExprClass) { +// SmallString<32> castStr; +// switch (cast) { +// case Expr::CXXStaticCastExprClass: +// castStr = "static_cast<"; +// castStr += type; +// castStr += ">"; +// break; +// case Expr::CXXFunctionalCastExprClass: +// default: +// castStr = type; +// } +// castStr += "("; +// SourceRange range = locOf(sm, expr); +// return {Replacement(sm, range.getBegin(), 0, castStr), Replacement(sm, range.getEnd(), 0, ")")}; +//} +// +//template +//inline clang::tooling::Replacement removeNode(const clang::SourceManager& sm, T node) { +// // FIXME does not remove empty line +// SourceRange loc = locOf(sm, node, 1); +// CharSourceRange cast_crange = CharSourceRange::getCharRange(loc); +// return Replacement(sm, cast_crange, ""); +//} +// +//template +//inline clang::tooling::Replacement insertNode(const clang::SourceManager& sm, T to_insert, U relative_to, +// bool before = false, char endl = ';') { +// // FIXME does not preserve indentation +// SourceRange range = locOf(sm, relative_to, 1); +// StringRef replacement_str; +// if (before) { +// replacement_str = clutil::node2str(sm, to_insert) + endl + "\n" + whitespaces(sm, relative_to).str(); +// return Replacement(sm, range.getBegin(), 0, replacement_str); +// } +// replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(sm, to_insert) + endl; +// return Replacement(sm, range.getEnd(), 0, replacement_str); +//} } } diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index 82e8f6a..e094e8b 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -36,10 +36,23 @@ inline std::string typeOf(NODE node) { } template -inline std::string fileOriginOf(const clang::SourceManager& sm, T node) { +inline clang::FileID fileIDOf(const clang::SourceManager& sm, T node) { const auto range = locOf(sm, node); - const std::pair DecomposedLocation = sm.getDecomposedLoc(range.getBegin()); - const clang::FileEntry* Entry = sm.getFileEntryForID(DecomposedLocation.first); + return sm.getFileID(range.getBegin()); +} + +template +inline const clang::FileEntry* fileEntryOf(const clang::SourceManager& sm, T node) { + auto file_id = fileIDOf(sm, node); + return sm.getFileEntryForID(file_id); +} + +template +inline std::string fileOriginOf(const clang::SourceManager& sm, T node) { + //const auto range = locOf(sm, node); + //const std::pair DecomposedLocation = sm.getDecomposedLoc(range.getBegin()); + //const clang::FileEntry* Entry = sm.getFileEntryForID(DecomposedLocation.first); + auto Entry = fileEntryOf(sm, node); if (Entry != NULL) { llvm::SmallString<256> FilePath(Entry->getName()); auto EC = llvm::sys::fs::make_absolute(FilePath); diff --git a/include/modules/ConditionalAssgnMatcher.h b/include/modules/ConditionalAssgnMatcher.h index 3926246..e790167 100644 --- a/include/modules/ConditionalAssgnMatcher.h +++ b/include/modules/ConditionalAssgnMatcher.h @@ -19,10 +19,7 @@ namespace module { // class ConditionalAssgnVisitor; class ConditionalAssgnMatcher : public opov::ASTMatcherModule { - private: - std::string type_s; - // std::unique_ptr visitor; - public: +public: ConditionalAssgnMatcher(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/ExplicitCast.h b/include/modules/ExplicitCast.h index ca5555c..63350f7 100644 --- a/include/modules/ExplicitCast.h +++ b/include/modules/ExplicitCast.h @@ -20,7 +20,6 @@ class ExplicitCastVisitor; class ExplicitCast : public opov::ASTMatcherModule { private: - std::string type_s; std::string header_cast; std::string cast_stmt; // std::unique_ptr visitor; diff --git a/include/modules/GlobalScope.h b/include/modules/GlobalScope.h index 3eae64f..6ae03e3 100644 --- a/include/modules/GlobalScope.h +++ b/include/modules/GlobalScope.h @@ -16,10 +16,7 @@ namespace opov { namespace module { class GlobalScope : public opov::ASTMatcherModule { - private: - std::string type_s; - - public: +public: GlobalScope(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/ImplicitConditionMatcher.h b/include/modules/ImplicitConditionMatcher.h index 22a67ca..128b1ad 100644 --- a/include/modules/ImplicitConditionMatcher.h +++ b/include/modules/ImplicitConditionMatcher.h @@ -15,10 +15,7 @@ namespace opov { namespace module { class ImplicitConditionMatcher : public opov::ASTMatcherModule { - private: - std::string type_s; - - public: +public: ImplicitConditionMatcher(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/ImplicitConversion.h b/include/modules/ImplicitConversion.h index 2bd155e..f894b24 100644 --- a/include/modules/ImplicitConversion.h +++ b/include/modules/ImplicitConversion.h @@ -17,11 +17,7 @@ namespace opov { namespace module { class ImplicitConversion : public opov::ASTMatcherModule { - private: - std::string type_s; - // std::unique_ptr visitor; - - public: +public: ImplicitConversion(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/UnionMatcher.h b/include/modules/UnionMatcher.h index 98eaa98..c343fc8 100644 --- a/include/modules/UnionMatcher.h +++ b/include/modules/UnionMatcher.h @@ -20,7 +20,6 @@ class FieldDeclCollector; class UnionMatcher : public opov::ASTMatcherModule { private: - std::string type_s; std::unique_ptr visitor; public: diff --git a/src/core/AbstractFactory.cpp b/src/core/AbstractFactory.cpp index fa28f5e..85c1d72 100644 --- a/src/core/AbstractFactory.cpp +++ b/src/core/AbstractFactory.cpp @@ -38,6 +38,9 @@ bool AbstractFactory::handleBeginSource(clang::CompilerInstance& CI, llvm::Strin } void AbstractFactory::handleEndSource() { + if(thandler->getRewriter().overwriteChangedFiles()) { + LOG_ERROR("Error while writing source transformations to disk."); + } } void AbstractFactory::setModule(Module* m) { diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 71a1f10..66c7bc5 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -74,19 +74,7 @@ int Application::execute(const clang::tooling::CompilationDatabase& db, const st // return sig; } } - - bool do_filter; - config->getValue("global:filter", do_filter, false); - TUIssuesMap& issuesMap = ihandler->getAllIssues(); - - if(do_filter) { - auto filteredSet = filter->apply(issuesMap); - reporter->addIssues(filteredSet); - } else { - reporter->addIssues(issuesMap); - } - - ihandler->clear(); + report(); return sig; } @@ -101,7 +89,12 @@ int Application::executeOnCode(const std::string& source, const std::vectorgetValue("global:filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); @@ -114,8 +107,6 @@ int Application::executeOnCode(const std::string& source, const std::vectorclear(); - - return sig; } void Application::addModule(Module* module) { diff --git a/src/core/module/ASTMatcherModule.cpp b/src/core/module/ASTMatcherModule.cpp index 54faa24..586e35b 100644 --- a/src/core/module/ASTMatcherModule.cpp +++ b/src/core/module/ASTMatcherModule.cpp @@ -7,6 +7,7 @@ #include #include +#include namespace opov { @@ -14,6 +15,8 @@ ASTMatcherModule::ASTMatcherModule() { } void ASTMatcherModule::init(const Configuration* config) { + config->getValue("global:type", type_s); + config->getValue("global:transform", transform, false); setupOnce(config); setupMatcher(); } diff --git a/src/core/module/AbstractModule.cpp b/src/core/module/AbstractModule.cpp index a208dcc..1ccffb7 100644 --- a/src/core/module/AbstractModule.cpp +++ b/src/core/module/AbstractModule.cpp @@ -15,10 +15,13 @@ namespace opov { AbstractModule::AbstractModule() - : context(nullptr) { + : context(nullptr) + , transform(false) { } void AbstractModule::init(const Configuration* config) { + config->getValue("global:type", type_s); + config->getValue("global:transform", transform, false); setupOnce(config); } diff --git a/src/core/transformation/TransformationHandler.cpp b/src/core/transformation/TransformationHandler.cpp index 3b12a20..a82c99a 100644 --- a/src/core/transformation/TransformationHandler.cpp +++ b/src/core/transformation/TransformationHandler.cpp @@ -40,6 +40,10 @@ void TransformationHandler::addReplacements(const clang::tooling::Replacement& r */ } +void TransformationHandler::addHeader(const std::string& header, clang::SourceLocation loc) { + addReplacements(includes->addAngledInclude(loc, header)); +} + void TransformationHandler::addReplacements(const std::vector& replacements) { for (auto& r : replacements) { addReplacements(r); diff --git a/src/modules/AllImplicitConversion.cpp b/src/modules/AllImplicitConversion.cpp index 618dc3a..29affc3 100644 --- a/src/modules/AllImplicitConversion.cpp +++ b/src/modules/AllImplicitConversion.cpp @@ -41,7 +41,7 @@ void AllImplicitConversion::setupMatcher() { } void AllImplicitConversion::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const CXXConstructExpr* expr = result.Nodes.getStmtAs("conversion"); + const CXXConstructExpr* expr = result.Nodes.getNodeAs("conversion"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(expr, moduleName(), moduleDescription()); diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 9a2f8ab..14cb1eb 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -34,7 +34,7 @@ void ConditionalAssgnMatcher::setupMatcher() { } void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const ConditionalOperator* e = result.Nodes.getStmtAs("condassign"); + const ConditionalOperator* e = result.Nodes.getNodeAs("condassign"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(e, moduleName(), moduleDescription()); diff --git a/src/modules/ExplicitCast.cpp b/src/modules/ExplicitCast.cpp index c8585a5..639f2b3 100644 --- a/src/modules/ExplicitCast.cpp +++ b/src/modules/ExplicitCast.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace opov { namespace module { @@ -25,7 +26,8 @@ ExplicitCast::ExplicitCast() { } void ExplicitCast::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); + config->getValue("ExplicitCast:header", header_cast, "recast.h"); + config->getValue("ExplicitCast:function", header_cast, "reCast"); } void ExplicitCast::setupMatcher() { @@ -45,11 +47,19 @@ void ExplicitCast::setupMatcher() { } void ExplicitCast::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const ExplicitCastExpr* e = result.Nodes.getStmtAs("cast"); - auto ecast = const_cast(e); + const ExplicitCastExpr* ecast = result.Nodes.getNodeAs("cast"); + //auto ecast = const_cast(e); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(ecast, moduleName(), moduleDescription()); + + if(transform) { + LOG_MSG("Transforming - explicit"); + auto& thandle = context->getTransformationHandler(); + auto replace = trutil::reCast(context->getASTContext(), ecast, type_s); + thandle.addHeader(header_cast, clutil::locOf(context->getSourceManager(), ecast).getBegin()); + thandle.addReplacements(replace); + } } std::string ExplicitCast::moduleName() { diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index 5495e86..5160da3 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -33,7 +33,7 @@ void GlobalScope::setupMatcher() { } void GlobalScope::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const Expr* call = result.Nodes.getStmtAs("global"); + const Expr* call = result.Nodes.getNodeAs("global"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(call, moduleName(), moduleDescription()); diff --git a/src/modules/ImplicitConditionMatcher.cpp b/src/modules/ImplicitConditionMatcher.cpp index 5177acf..3ef4f67 100644 --- a/src/modules/ImplicitConditionMatcher.cpp +++ b/src/modules/ImplicitConditionMatcher.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -26,7 +27,7 @@ ImplicitConditionMatcher::ImplicitConditionMatcher() { } void ImplicitConditionMatcher::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); + } void ImplicitConditionMatcher::setupMatcher() { @@ -46,15 +47,19 @@ void ImplicitConditionMatcher::setupMatcher() { } void ImplicitConditionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const Expr* implicit_cast = result.Nodes.getStmtAs("implicit"); - const Expr* unary_op = result.Nodes.getStmtAs("unary"); + /* + * match unary nodes of "scalar" but only transform OperatorKind (UO_Not, UO_LNot) + */ + const Expr* implicit_cast = result.Nodes.getNodeAs("implicit"); + const Expr* unary_op = result.Nodes.getNodeAs("unary"); const Expr* invalid = !implicit_cast ? unary_op : implicit_cast; - auto e = result.Nodes.getMap(); - BoundNodes::IDToNodeMap::const_iterator BI; - - auto& ihandle = context->getIssueHandler(); ihandle.addIssue(invalid, moduleName(), moduleDescription()); + + if(transform) { + auto& thandle = context->getTransformationHandler(); + thandle.addReplacements(trutil::addExplicitCompare(context->getASTContext(), invalid, type_s)); + } } std::string ImplicitConditionMatcher::moduleName() { diff --git a/src/modules/ImplicitConversion.cpp b/src/modules/ImplicitConversion.cpp index f9e4328..6309f20 100644 --- a/src/modules/ImplicitConversion.cpp +++ b/src/modules/ImplicitConversion.cpp @@ -14,6 +14,7 @@ #include #include //#include +#include namespace opov { namespace module { @@ -40,10 +41,15 @@ void ImplicitConversion::setupMatcher() { } void ImplicitConversion::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const CXXConstructExpr* expr = result.Nodes.getStmtAs("conversion"); + const CXXConstructExpr* expr = result.Nodes.getNodeAs("conversion"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(expr, moduleName(), moduleDescription()); //, message.str()); + + if(transform) { + auto& thandle = context->getTransformationHandler(); + thandle.addReplacements(trutil::castTheExpr(context->getASTContext(), expr, type_s)); + } } std::string ImplicitConversion::moduleName() { diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index 8510747..72aa54b 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -36,7 +36,7 @@ void UnionMatcher::setupMatcher() { } void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const CXXRecordDecl* inv_union = result.Nodes.getDeclAs("union"); + const CXXRecordDecl* inv_union = result.Nodes.getNodeAs("union"); const bool is_anon = inv_union->isAnonymousStructOrUnion(); auto fieldDecls = visitor->extractDecl(const_cast(inv_union)); From 7de85bd92f652198c504b50b74c65c27f3636a21 Mon Sep 17 00:00:00 2001 From: ahueck Date: Tue, 13 Oct 2015 21:40:36 +0200 Subject: [PATCH 07/21] Added ST capabilities for UnionMatcher (untested). TODO implement a (recursive) solution for conditional assignment operators. --- .../core/transformation/TransformationUtil.h | 51 +++++++++++-------- src/modules/ConditionalAssgnMatcher.cpp | 6 +++ src/modules/UnionMatcher.cpp | 20 ++++++++ 3 files changed, 56 insertions(+), 21 deletions(-) diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index cb08f86..c895e44 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -91,7 +91,37 @@ inline clang::tooling::Replacement castTheExpr(const clang::ASTContext& ac, cons return Replacement(sm, expr, castStr); } +template +inline clang::tooling::Replacement replaceStmt(const clang::ASTContext& ac, const T* stmt, const U* with) { + auto& sm = ac.getSourceManager(); + return Replacement(sm, stmt, clutil::node2str(ac, with)); +} +template +inline clang::tooling::Replacement replaceStmt(const clang::ASTContext& ac, const T* stmt, const std::string with) { + auto& sm = ac.getSourceManager(); + return Replacement(sm, stmt, with); +} + +template +inline clang::tooling::Replacement insertNode(const clang::ASTContext& ac, T to_insert, U relative_to, + bool before = false, char endl = ';') { + auto& sm = ac.getSourceManager(); + SourceRange range = locOf(sm, relative_to, 1); + std::string replacement_str; + if (before) { + replacement_str = clutil::node2str(ac, to_insert) + endl + "\n" + whitespaces(sm, relative_to).str(); + return Replacement(sm, range.getBegin(), 0, replacement_str); + } + replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(ac, to_insert) + endl; + return Replacement(sm, range.getEnd(), 0, replacement_str); +} + +template +inline clang::tooling::Replacement removeNode(const clang::ASTContext& ac, T node) { + auto& sm = ac.getSourceManager(); + return Replacement(sm, node, ""); +} /* //inline std::vector addExplicitCompare(const @@ -138,28 +168,7 @@ inline clang::tooling::Replacement castTheExpr(const clang::ASTContext& ac, cons // return {Replacement(sm, range.getBegin(), 0, castStr), Replacement(sm, range.getEnd(), 0, ")")}; //} // -//template -//inline clang::tooling::Replacement removeNode(const clang::SourceManager& sm, T node) { -// // FIXME does not remove empty line -// SourceRange loc = locOf(sm, node, 1); -// CharSourceRange cast_crange = CharSourceRange::getCharRange(loc); -// return Replacement(sm, cast_crange, ""); -//} // -//template -//inline clang::tooling::Replacement insertNode(const clang::SourceManager& sm, T to_insert, U relative_to, -// bool before = false, char endl = ';') { -// // FIXME does not preserve indentation -// SourceRange range = locOf(sm, relative_to, 1); -// StringRef replacement_str; -// if (before) { -// replacement_str = clutil::node2str(sm, to_insert) + endl + "\n" + whitespaces(sm, relative_to).str(); -// return Replacement(sm, range.getBegin(), 0, replacement_str); -// } -// replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(sm, to_insert) + endl; -// return Replacement(sm, range.getEnd(), 0, replacement_str); -//} - } } diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 14cb1eb..a790c13 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -38,6 +38,12 @@ void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchR auto& ihandle = context->getIssueHandler(); ihandle.addIssue(e, moduleName(), moduleDescription()); + + if(transform) { + auto& thandle = context->getTransformationHandler(); + auto& ast_ctx = context->getASTContext(); + // TODO implement transformation + } } std::string ConditionalAssgnMatcher::moduleName() { diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index 72aa54b..07f06bf 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace opov { namespace module { @@ -46,6 +47,25 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu auto& ihandle = context->getIssueHandler(); ihandle.addIssue(inv_union, moduleName(), moduleDescription(), message.str()); + + // TODO improve by using a custom union traversal and slowly building a replacement string + if(transform) { + auto& thandle = context->getTransformationHandler(); + auto& ast_ctx = context->getASTContext(); + if(is_anon) { + const bool remove_union = std::distance(inv_union->field_begin(), inv_union->field_end()) == 1; + if(remove_union) { + thandle.addReplacements(trutil::replaceStmt(ast_ctx, inv_union, fieldDecls.front())); + } else { + for(auto fd : fieldDecls) { + thandle.addReplacements(trutil::removeNode(ast_ctx, fd)); + thandle.addReplacements(trutil::insertNode(ast_ctx, fd, inv_union)); + } + } + } else { + thandle.addReplacements(tooling::Replacement(context->getSourceManager(), inv_union->getLocStart(), 5, "struct")); + } + } } std::string UnionMatcher::moduleName() { From 904c00080826ab0573d42ec5f3a8a559d324bd37 Mon Sep 17 00:00:00 2001 From: ahueck Date: Tue, 13 Oct 2015 21:43:09 +0200 Subject: [PATCH 08/21] Removed unnecessary config calls for the type, as it is now read in the super class. --- src/modules/ConditionalAssgnMatcher.cpp | 1 - src/modules/GlobalScope.cpp | 1 - src/modules/ImplicitConversion.cpp | 1 - src/modules/UnionMatcher.cpp | 1 - 4 files changed, 4 deletions(-) diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index a790c13..651fef0 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -24,7 +24,6 @@ ConditionalAssgnMatcher::ConditionalAssgnMatcher() { } void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); } void ConditionalAssgnMatcher::setupMatcher() { diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index 5160da3..b371c89 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -23,7 +23,6 @@ GlobalScope::GlobalScope() { } void GlobalScope::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); } void GlobalScope::setupMatcher() { diff --git a/src/modules/ImplicitConversion.cpp b/src/modules/ImplicitConversion.cpp index 6309f20..b5d0c39 100644 --- a/src/modules/ImplicitConversion.cpp +++ b/src/modules/ImplicitConversion.cpp @@ -26,7 +26,6 @@ ImplicitConversion::ImplicitConversion() { } void ImplicitConversion::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); } void ImplicitConversion::setupMatcher() { diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index 07f06bf..e9f6d82 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -26,7 +26,6 @@ UnionMatcher::UnionMatcher() { } void UnionMatcher::setupOnce(const Configuration* config) { - config->getValue("global:type", type_s); visitor = opov::util::make_unique(type_s); } From ef0eba9d4b6799172419fd7bbda3f2c337f7972d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Wed, 14 Oct 2015 11:01:34 +0200 Subject: [PATCH 09/21] Playing around with cond matcher --- include/core/utility/ClangMatcherExt.h | 8 ++++++++ src/modules/ConditionalAssgnMatcher.cpp | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index 288a0a2..e8a75a7 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -34,6 +34,14 @@ AST_MATCHER(Stmt, notABinaryExpr) { return !isa(Node); } +const internal::VariadicDynCastAllOfMatcher< + Stmt, + ParenExpr> parenExpr; + +//AST_MATCHER_P(ParenExpr, parenExpr, internal::Matcher, InnerMatcher) { +// return InnerMatcher.matches(Node, Finder, Builder);; +//} + AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); } diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 651fef0..970cfce 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -28,11 +28,30 @@ void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { void ConditionalAssgnMatcher::setupMatcher() { // TODO use ofType instead of just typedef? - StatementMatcher condassign = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("condassign"); + //StatementMatcher condassign = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("condassign"); + + auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))); + //auto assign_operator = binaryOperator(hasOperatorName("="), hasRHS(ignoringParenImpCasts(conditional))); + //auto conditional_root = allOf(conditional, unless(anyOf(hasParent(conditional), hasParent(assign_operator), hasParent(parenExpr(hasParent(conditional)))))); + //auto condassign = expr(anyOf(assign_operator + // , conditional_root)).bind("condassign"); + auto condassign = stmt(hasParent(compoundStmt()), hasDescendant(conditional)).bind("condassign"); + this->addMatcher(condassign); } void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { + auto* e = result.Nodes.getNodeAs("condassign"); + auto& ast_ctx = context->getASTContext(); + if(isa(e)) { + LOG_MSG("Binary operator matched: " << clutil::node2str(ast_ctx, e)); + } else if(isa(e)) { + LOG_MSG("COnditional operator matched: " << clutil::node2str(ast_ctx, e)); + } else { + LOG_MSG("Something else matched...: " << clutil::node2str(ast_ctx, e)); + } + + /* const ConditionalOperator* e = result.Nodes.getNodeAs("condassign"); auto& ihandle = context->getIssueHandler(); @@ -43,6 +62,7 @@ void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchR auto& ast_ctx = context->getASTContext(); // TODO implement transformation } + */ } std::string ConditionalAssgnMatcher::moduleName() { From 7a460ff399f3ae17302a3e3442fe9fa0adfe9f74 Mon Sep 17 00:00:00 2001 From: ahueck Date: Wed, 14 Oct 2015 22:41:51 +0200 Subject: [PATCH 10/21] Added ST capability for conditionaloperator detection. (Only 1 level of nesting) --- .../core/transformation/TransformationUtil.h | 18 +++-- include/core/utility/ClangMatcherExt.h | 8 +- include/core/utility/Util.h | 7 ++ include/modules/ConditionalAssgnMatcher.h | 11 ++- include/modules/ExplicitCastVisitor.h | 32 -------- src/modules/ConditionalAssgnMatcher.cpp | 77 ++++++++++++------- src/modules/ExplicitCast.cpp | 1 - test/collection/conditional_assign.h | 15 ++++ 8 files changed, 100 insertions(+), 69 deletions(-) delete mode 100644 include/modules/ExplicitCastVisitor.h create mode 100644 test/collection/conditional_assign.h diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index c895e44..e68440f 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -103,26 +103,34 @@ inline clang::tooling::Replacement replaceStmt(const clang::ASTContext& ac, cons return Replacement(sm, stmt, with); } -template -inline clang::tooling::Replacement insertNode(const clang::ASTContext& ac, T to_insert, U relative_to, - bool before = false, char endl = ';') { +template +inline clang::tooling::Replacement insertString(const clang::ASTContext& ac, const std::string& to_insert, U relative_to, + bool before = false, std::string endl = ";") { auto& sm = ac.getSourceManager(); SourceRange range = locOf(sm, relative_to, 1); std::string replacement_str; if (before) { - replacement_str = clutil::node2str(ac, to_insert) + endl + "\n" + whitespaces(sm, relative_to).str(); + replacement_str = whitespaces(sm, relative_to).str() + to_insert + endl + "\n"; return Replacement(sm, range.getBegin(), 0, replacement_str); } - replacement_str = whitespaces(sm, relative_to).str() + "\n" + clutil::node2str(ac, to_insert) + endl; + replacement_str = whitespaces(sm, relative_to).str() + "\n" + to_insert + endl; return Replacement(sm, range.getEnd(), 0, replacement_str); } + +template +inline clang::tooling::Replacement insertNode(const clang::ASTContext& ac, T to_insert, U relative_to, + bool before = false, std::string endl = ";") { + return insertString(ac, clutil::node2str(ac, to_insert), relative_to, before, endl); +} + template inline clang::tooling::Replacement removeNode(const clang::ASTContext& ac, T node) { auto& sm = ac.getSourceManager(); return Replacement(sm, node, ""); } + /* //inline std::vector addExplicitCompare(const //clang::SourceManager& sm, const clang::UnaryOperator* expr, StringRef with_type) diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index e8a75a7..5e04628 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -38,9 +38,11 @@ const internal::VariadicDynCastAllOfMatcher< Stmt, ParenExpr> parenExpr; -//AST_MATCHER_P(ParenExpr, parenExpr, internal::Matcher, InnerMatcher) { -// return InnerMatcher.matches(Node, Finder, Builder);; -//} +#define descendant_or_self(NODE) \ + anyOf(NODE, hasDescendant(NODE)) + +#define ancestor_or_self(NODE) \ + anyOf(NODE, hasAncestor(NODE)) AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); diff --git a/include/core/utility/Util.h b/include/core/utility/Util.h index 1486415..fcd2239 100644 --- a/include/core/utility/Util.h +++ b/include/core/utility/Util.h @@ -29,6 +29,13 @@ inline std::vector split(const std::string& input, char delimiter = return tokens; } +template +std::string num2str(T val) { + std::stringstream sstream; + sstream << val; + return sstream.str(); +} + /* inline std::vector split_str(const std::string& input, const std::string& regex_match) { diff --git a/include/modules/ConditionalAssgnMatcher.h b/include/modules/ConditionalAssgnMatcher.h index e790167..94146e8 100644 --- a/include/modules/ConditionalAssgnMatcher.h +++ b/include/modules/ConditionalAssgnMatcher.h @@ -11,7 +11,7 @@ #include #include -//#include +#include namespace opov { namespace module { @@ -19,6 +19,12 @@ namespace module { // class ConditionalAssgnVisitor; class ConditionalAssgnMatcher : public opov::ASTMatcherModule { +private: + unsigned var_counter; + typedef struct { + std::string type, variable, lhs, rhs, replacement; + } conditional_data; + public: ConditionalAssgnMatcher(); virtual void setupOnce(const Configuration* config) override; @@ -27,6 +33,9 @@ class ConditionalAssgnMatcher : public opov::ASTMatcherModule { virtual std::string moduleName() override; virtual std::string moduleDescription() override; virtual ~ConditionalAssgnMatcher(); +private: + void toString(clang::ASTContext& ac, const clang::Expr* e, conditional_data& d, int counter = 0); + conditional_data buildReplacement(clang::ASTContext& ac, const clang::ConditionalOperator* e); }; } /* namespace module */ diff --git a/include/modules/ExplicitCastVisitor.h b/include/modules/ExplicitCastVisitor.h deleted file mode 100644 index c349528..0000000 --- a/include/modules/ExplicitCastVisitor.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * ExplicitCastVisitor.h - * - * Created on: Jun 23, 2014 - * Author: ahueck - */ - -#ifndef EXPLICITCASTVISITOR_H_ -#define EXPLICITCASTVISITOR_H_ - -#include - -namespace opov { -namespace module { - -class ExplicitCastVisitor : public clang::RecursiveASTVisitor { - protected: - const std::string& type; - bool subtreeHasType; - - public: - ExplicitCastVisitor(const std::string& type = ""); - bool hasType(clang::ExplicitCastExpr* expr); - bool TraverseStmt(clang::Stmt* S); - bool VisitExplicitCastExpr(clang::ExplicitCastExpr* expr); - virtual ~ExplicitCastVisitor(); -}; - -} /* namespace module */ -} /* namespace opov */ - -#endif /* EXPLICITCASTVISITOR_H_ */ diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 970cfce..907cf78 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -12,6 +12,9 @@ #include #include #include +//#include +#include +#include namespace opov { namespace module { @@ -19,50 +22,70 @@ namespace module { using namespace clang; using namespace clang::ast_matchers; -ConditionalAssgnMatcher::ConditionalAssgnMatcher() { - // TODO Auto-generated constructor stub +ConditionalAssgnMatcher::ConditionalAssgnMatcher() +: var_counter(0) { + } void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { + //transformer = util::make_unique(type_s); } void ConditionalAssgnMatcher::setupMatcher() { // TODO use ofType instead of just typedef? - //StatementMatcher condassign = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("condassign"); - - auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))); - //auto assign_operator = binaryOperator(hasOperatorName("="), hasRHS(ignoringParenImpCasts(conditional))); - //auto conditional_root = allOf(conditional, unless(anyOf(hasParent(conditional), hasParent(assign_operator), hasParent(parenExpr(hasParent(conditional)))))); - //auto condassign = expr(anyOf(assign_operator - // , conditional_root)).bind("condassign"); - auto condassign = stmt(hasParent(compoundStmt()), hasDescendant(conditional)).bind("condassign"); + auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("conditional"); + auto condassign = stmt(hasParent(compoundStmt()), descendant_or_self(conditional)).bind("conditional_root"); this->addMatcher(condassign); + this->addMatcher(conditional); } -void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - auto* e = result.Nodes.getNodeAs("condassign"); - auto& ast_ctx = context->getASTContext(); - if(isa(e)) { - LOG_MSG("Binary operator matched: " << clutil::node2str(ast_ctx, e)); - } else if(isa(e)) { - LOG_MSG("COnditional operator matched: " << clutil::node2str(ast_ctx, e)); - } else { - LOG_MSG("Something else matched...: " << clutil::node2str(ast_ctx, e)); + +void ConditionalAssgnMatcher::toString(clang::ASTContext& ac, const Expr* e, conditional_data& d, int counter) { + auto cond = dyn_cast(e->IgnoreParenImpCasts()); + if(cond) { + d.type = clutil::typeOf(e); + d.variable = "temp_00" + util::num2str(counter); + d.replacement = d.type + " " + d.variable + ";\n" + "condassign(" + d.variable + + ", " + clutil::node2str(ac, cond->getCond()) + + ", " + (d.lhs == "" ? clutil::node2str(ac, cond->getLHS()) : d.lhs) + + ", " + (d.rhs == "" ? clutil::node2str(ac, cond->getRHS()) : d.rhs) + ");"; + } +} - /* - const ConditionalOperator* e = result.Nodes.getNodeAs("condassign"); +ConditionalAssgnMatcher::conditional_data ConditionalAssgnMatcher::buildReplacement(clang::ASTContext& ac, const ConditionalOperator* conditional) { +#define nl_str(STRUCT) \ + (STRUCT.replacement == "" ? "" : (STRUCT.replacement + "\n")) + + conditional_data lhs_dat, rhs_dat, cond_dat; + toString(ac, conditional->getRHS(), rhs_dat, ++var_counter); + toString(ac, conditional->getLHS(), lhs_dat, ++var_counter); + cond_dat.lhs = lhs_dat.variable; + cond_dat.rhs = rhs_dat.variable; + toString(ac, conditional, cond_dat, ++var_counter); + cond_dat.replacement = nl_str(lhs_dat) + nl_str(rhs_dat) + cond_dat.replacement; + return cond_dat; +} - auto& ihandle = context->getIssueHandler(); - ihandle.addIssue(e, moduleName(), moduleDescription()); +void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { + auto* root = result.Nodes.getNodeAs("conditional_root"); + auto* conditional = result.Nodes.getNodeAs("conditional"); + + if(root == nullptr) { + // We report every conditionalOperator (even nested ones) + auto& ihandle = context->getIssueHandler(); + ihandle.addIssue(conditional, moduleName(), moduleDescription()); + } - if(transform) { + if(transform && root && conditional) { + // We transform starting from root down to the last conditionalOperator auto& thandle = context->getTransformationHandler(); - auto& ast_ctx = context->getASTContext(); - // TODO implement transformation + auto& ac = context->getASTContext(); + conditional_data cond_dat = buildReplacement(ac, conditional); + thandle.addReplacements(clang::tooling::Replacement(ac.getSourceManager(), conditional, cond_dat.variable)); + thandle.addReplacements(trutil::insertString(ac, cond_dat.replacement, root, true, "")); } - */ } std::string ConditionalAssgnMatcher::moduleName() { diff --git a/src/modules/ExplicitCast.cpp b/src/modules/ExplicitCast.cpp index 639f2b3..a03e132 100644 --- a/src/modules/ExplicitCast.cpp +++ b/src/modules/ExplicitCast.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include diff --git a/test/collection/conditional_assign.h b/test/collection/conditional_assign.h new file mode 100644 index 0000000..554c6b2 --- /dev/null +++ b/test/collection/conditional_assign.h @@ -0,0 +1,15 @@ +typedef double scalar; +inline void test(scalar a) { +} + +inline scalar test_ret(scalar a) { + return 1.0; +} + +inline void broken() { + scalar a,b,c,d=1.0; + a = b > 0 ? c : d; + func(b > 0 ? c : d); + a = b > 0 ? (b > 0 ? c : d) : d; + test(test_ret(b > 0 ? c : d)); +} From 179d4e0bab979f5891d10305482c49e0fb52e8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Thu, 15 Oct 2015 13:44:31 +0200 Subject: [PATCH 11/21] Added first detection of if-else assignments (closely related to the existing conditionalassign matcher). --- CMakeLists.txt | 1 + include/core/utility/ClangMatcherExt.h | 12 ++++ include/modules/ConditionalAssgnMatcher.h | 1 - include/modules/IfElseAssign.h | 30 ++++++++++ src/OpOvApp.cpp | 2 + src/modules/ConditionalAssgnMatcher.cpp | 4 +- src/modules/IfElseAssign.cpp | 68 +++++++++++++++++++++++ test/collection/conditional_assign.h | 7 +++ 8 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 include/modules/IfElseAssign.h create mode 100644 src/modules/IfElseAssign.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 014e3ce..7e03bff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,6 +39,7 @@ src/modules/ImplicitConversion.cpp src/modules/ExplicitConstructor.cpp src/modules/AllImplicitConversion.cpp src/modules/GlobalScope.cpp +src/modules/IfElseAssign.cpp ) add_library(libopov ${LSOURCES}) diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index 5e04628..d9abc25 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -44,6 +44,18 @@ const internal::VariadicDynCastAllOfMatcher< #define ancestor_or_self(NODE) \ anyOf(NODE, hasAncestor(NODE)) +AST_MATCHER_P(IfStmt, hasThen, internal::Matcher, InnerMatcher) { + // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 + const Stmt *const Then = Node.getThen(); + return (Then != nullptr && InnerMatcher.matches(*Then, Finder, Builder)); +} + +AST_MATCHER_P(IfStmt, hasElse, internal::Matcher, InnerMatcher) { + // Taken from the current version of Clangs ASTMatchers.h file: Line 2934 + const Stmt *const Else = Node.getElse(); + return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); +} + AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); } diff --git a/include/modules/ConditionalAssgnMatcher.h b/include/modules/ConditionalAssgnMatcher.h index 94146e8..f413d34 100644 --- a/include/modules/ConditionalAssgnMatcher.h +++ b/include/modules/ConditionalAssgnMatcher.h @@ -11,7 +11,6 @@ #include #include -#include namespace opov { namespace module { diff --git a/include/modules/IfElseAssign.h b/include/modules/IfElseAssign.h new file mode 100644 index 0000000..b2c012d --- /dev/null +++ b/include/modules/IfElseAssign.h @@ -0,0 +1,30 @@ +/* + * IfElseAssign.h + * + * Created on: Oct 15, 2015 + * Author: ahueck + */ + +#ifndef IFELSEASSIGN_H_ +#define IFELSEASSIGN_H_ + +#include + +namespace opov { +namespace module { + +class IfElseAssign : public opov::ASTMatcherModule { +public: + IfElseAssign(); + virtual void setupOnce(const Configuration* config) override; + virtual void setupMatcher() override; + virtual void run(const clang::ast_matchers::MatchFinder::MatchResult& result) override; + virtual std::string moduleName() override; + virtual std::string moduleDescription() override; + virtual ~IfElseAssign(); +}; + +} /* namespace module */ +} /* namespace opov */ + +#endif /* IFELSEASSIGN_H_ */ diff --git a/src/OpOvApp.cpp b/src/OpOvApp.cpp index d4d6d4c..350e438 100644 --- a/src/OpOvApp.cpp +++ b/src/OpOvApp.cpp @@ -22,6 +22,7 @@ #include #include #include +#include OpOvApp::OpOvApp(const std::string& config_path) : config_path(config_path) { @@ -53,6 +54,7 @@ void OpOvApp::initModules() { addModule(new opov::module::ImplicitConversion()); addModule(new opov::module::ImplicitConditionMatcher()); addModule(new opov::module::ConditionalAssgnMatcher()); + addModule(new opov::module::IfElseAssign()); } OpOvApp::~OpOvApp() { diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 907cf78..7ce9572 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -33,6 +33,7 @@ void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { void ConditionalAssgnMatcher::setupMatcher() { // TODO use ofType instead of just typedef? + // we merely check for the existence of type scalar, ADOL-C speaks about all types being 'active', but we should warn more frequently. auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("conditional"); auto condassign = stmt(hasParent(compoundStmt()), descendant_or_self(conditional)).bind("conditional_root"); @@ -45,12 +46,11 @@ void ConditionalAssgnMatcher::toString(clang::ASTContext& ac, const Expr* e, con auto cond = dyn_cast(e->IgnoreParenImpCasts()); if(cond) { d.type = clutil::typeOf(e); - d.variable = "temp_00" + util::num2str(counter); + d.variable = "_oolint_t_" + util::num2str(cond); d.replacement = d.type + " " + d.variable + ";\n" + "condassign(" + d.variable + ", " + clutil::node2str(ac, cond->getCond()) + ", " + (d.lhs == "" ? clutil::node2str(ac, cond->getLHS()) : d.lhs) + ", " + (d.rhs == "" ? clutil::node2str(ac, cond->getRHS()) : d.rhs) + ");"; - } } diff --git a/src/modules/IfElseAssign.cpp b/src/modules/IfElseAssign.cpp new file mode 100644 index 0000000..de3c4e4 --- /dev/null +++ b/src/modules/IfElseAssign.cpp @@ -0,0 +1,68 @@ +/* + * IfElseAssign.cpp + * + * Created on: Oct 15, 2015 + * Author: ahueck + */ + +#include +#include +#include +#include +#include +#include +#include +//#include +#include +#include + +namespace opov { +namespace module { + +using namespace clang; +using namespace clang::ast_matchers; + +IfElseAssign::IfElseAssign() { + + +} + +void IfElseAssign::setupOnce(const Configuration* config) { + +} + +void IfElseAssign::setupMatcher() { + // we merely check for LHS of type scalar, ADOL-C speaks about all types being 'active', but we should warn more frequently. + auto assign = binaryOperator(hasOperatorName("="), hasLHS(descendant_or_self(expr(isTypedef(type_s))))); + auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); + auto conditional = ifStmt(anyOf(allOf(hasThen(single_expr), hasElse(single_expr)), allOf(hasThen(single_expr), unless(hasElse(single_expr))))).bind("conditional"); + this->addMatcher(conditional); +} + +void IfElseAssign::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { + auto* conditional = result.Nodes.getNodeAs("conditional"); + + auto& ihandle = context->getIssueHandler(); + ihandle.addIssue(conditional, moduleName(), moduleDescription()); + + + if(transform) { + auto& thandle = context->getTransformationHandler(); + auto& ac = context->getASTContext(); + } +} + +std::string IfElseAssign::moduleName() { + return "IfElseAssign"; +} + +std::string IfElseAssign::moduleDescription() { + return "Conditional assignments through if-else are not allowed with ADOL-C."; +} + +IfElseAssign::~IfElseAssign() { + +} + +} /* namespace module */ +} /* namespace opov */ diff --git a/test/collection/conditional_assign.h b/test/collection/conditional_assign.h index 554c6b2..109cf77 100644 --- a/test/collection/conditional_assign.h +++ b/test/collection/conditional_assign.h @@ -8,8 +8,15 @@ inline scalar test_ret(scalar a) { inline void broken() { scalar a,b,c,d=1.0; + if(b > 0) { + a = c; + } else { + int i = b; + } +/* a = b > 0 ? c : d; func(b > 0 ? c : d); a = b > 0 ? (b > 0 ? c : d) : d; test(test_ret(b > 0 ? c : d)); +*/ } From bbffcd11cbd372f51ea9d4e48c3dbb4e483f19a9 Mon Sep 17 00:00:00 2001 From: ahueck Date: Thu, 15 Oct 2015 21:38:12 +0200 Subject: [PATCH 12/21] Added detection and source transformation for if-else assignement constructs in the style of the assignmentoperator matcher. --- include/core/utility/ClangMatcherExt.h | 4 +- include/core/utility/ClangUtil.h | 8 ++++ include/modules/IfElseAssign.h | 2 + src/modules/ConditionalAssgnMatcher.cpp | 6 ++- src/modules/IfElseAssign.cpp | 53 ++++++++++++++++++--- test/collection/conditional_ifelse_assign.h | 25 ++++++++++ 6 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 test/collection/conditional_ifelse_assign.h diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index d9abc25..c5a6303 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -44,6 +44,7 @@ const internal::VariadicDynCastAllOfMatcher< #define ancestor_or_self(NODE) \ anyOf(NODE, hasAncestor(NODE)) +/* AST_MATCHER_P(IfStmt, hasThen, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 const Stmt *const Then = Node.getThen(); @@ -55,6 +56,7 @@ AST_MATCHER_P(IfStmt, hasElse, internal::Matcher, InnerMatcher) { const Stmt *const Else = Node.getElse(); return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); } +*/ AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); @@ -63,7 +65,7 @@ AST_MATCHER(TagDecl, isUnion) { AST_MATCHER_P(Stmt, ofType, std::string, type) { opov::clutil::TypeDeducer deducer(type); const bool is_type = deducer.hasType(const_cast(&Node)); - LOG_DEBUG("ofType: '" << type << "' : " << is_type); + //LOG_DEBUG("ofType: '" << type << "' : " << is_type); return is_type; } diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index e094e8b..c8c3ea2 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -35,6 +35,14 @@ inline std::string typeOf(NODE node) { return node->getType().getUnqualifiedType().getAsString(); } +inline std::string nameOf(const clang::NamedDecl* decl) { + return decl->getNameAsString(); +} + +inline std::string nameOf(const clang::DeclRefExpr* expr) { + return nameOf(expr->getDecl()); +} + template inline clang::FileID fileIDOf(const clang::SourceManager& sm, T node) { const auto range = locOf(sm, node); diff --git a/include/modules/IfElseAssign.h b/include/modules/IfElseAssign.h index b2c012d..0690867 100644 --- a/include/modules/IfElseAssign.h +++ b/include/modules/IfElseAssign.h @@ -22,6 +22,8 @@ class IfElseAssign : public opov::ASTMatcherModule { virtual std::string moduleName() override; virtual std::string moduleDescription() override; virtual ~IfElseAssign(); +private: + std::string toString(clang::ASTContext& ac, const clang::IfStmt* stmt, const clang::BinaryOperator* then, const clang::BinaryOperator* else_e); }; } /* namespace module */ diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 7ce9572..62030ab 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -33,7 +33,8 @@ void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { void ConditionalAssgnMatcher::setupMatcher() { // TODO use ofType instead of just typedef? - // we merely check for the existence of type scalar, ADOL-C speaks about all types being 'active', but we should warn more frequently. + // We warn whenever an active type is present for such code structures. + // Even if there is no assignement. auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("conditional"); auto condassign = stmt(hasParent(compoundStmt()), descendant_or_self(conditional)).bind("conditional_root"); @@ -83,7 +84,8 @@ void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchR auto& thandle = context->getTransformationHandler(); auto& ac = context->getASTContext(); conditional_data cond_dat = buildReplacement(ac, conditional); - thandle.addReplacements(clang::tooling::Replacement(ac.getSourceManager(), conditional, cond_dat.variable)); + // Delete if for some reason ?: has no assignement (root is equal to conditional pointer) + thandle.addReplacements(clang::tooling::Replacement(ac.getSourceManager(), conditional, root == conditional ? "" : cond_dat.variable)); thandle.addReplacements(trutil::insertString(ac, cond_dat.replacement, root, true, "")); } } diff --git a/src/modules/IfElseAssign.cpp b/src/modules/IfElseAssign.cpp index de3c4e4..71cc6d7 100644 --- a/src/modules/IfElseAssign.cpp +++ b/src/modules/IfElseAssign.cpp @@ -32,23 +32,64 @@ void IfElseAssign::setupOnce(const Configuration* config) { } void IfElseAssign::setupMatcher() { - // we merely check for LHS of type scalar, ADOL-C speaks about all types being 'active', but we should warn more frequently. - auto assign = binaryOperator(hasOperatorName("="), hasLHS(descendant_or_self(expr(isTypedef(type_s))))); - auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); - auto conditional = ifStmt(anyOf(allOf(hasThen(single_expr), hasElse(single_expr)), allOf(hasThen(single_expr), unless(hasElse(single_expr))))).bind("conditional"); + // ADOL-C speaks about all types being 'active', we should warn whenever an active type is involved (scalar). + // Caveat: We limit ourselves to conditionaloperator-like structures (as shown in ADOL-C tech paper) + // That is if or if-else with each having exactly one assignment with an active type involved + //auto assign = binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND); + //auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); +#define assign_bind(BIND) \ + binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND) +#define assign_expr(BIND) \ + anyOf(compoundStmt(statementCountIs(1), has(assign_bind(BIND))), assign_bind(BIND)) + + auto conditional = ifStmt(anyOf( + allOf(hasThen(assign_expr("then")), hasElse(assign_expr("else"))) + , allOf(hasThen(assign_expr("then")), unless(hasElse(stmt()))))).bind("conditional"); this->addMatcher(conditional); } +std::string IfElseAssign::toString(clang::ASTContext& ac, const IfStmt* stmt, const BinaryOperator* then, const BinaryOperator* else_e) { + auto then_ref = clutil::nameOf(dyn_cast(then->getLHS()->IgnoreImpCasts())); + + if(else_e && then_ref != clutil::nameOf(dyn_cast(else_e->getLHS()->IgnoreImpCasts()))) { + // only transform when in both blocks the same variable is assigned to! + return ""; + } + + auto replacement = "condassign(" + then_ref + + ", " + clutil::node2str(ac, stmt->getCond()) + + ", " +clutil::node2str(ac, then->getRHS()); + if(else_e) { + replacement += ", " + clutil::node2str(ac, else_e->getRHS()); + } + replacement += ")"; + if((else_e && isa(stmt->getElse())) || isa(stmt->getThen())) { + /* + * For structures as: + * if(...) + * a = b; + * the later Replacement does not remove the semicolon of "a = b;" + */ + replacement += ";"; + } + return replacement; +} + void IfElseAssign::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { auto* conditional = result.Nodes.getNodeAs("conditional"); + auto* then_expr = result.Nodes.getNodeAs("then"); + auto* else_expr = result.Nodes.getNodeAs("else"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(conditional, moduleName(), moduleDescription()); - if(transform) { - auto& thandle = context->getTransformationHandler(); auto& ac = context->getASTContext(); + auto replacement = toString(ac, conditional, then_expr, else_expr); + if(replacement != "") { + auto& thandle = context->getTransformationHandler(); + thandle.addReplacements(clang::tooling::Replacement(context->getSourceManager(), conditional, replacement)); + } } } diff --git a/test/collection/conditional_ifelse_assign.h b/test/collection/conditional_ifelse_assign.h new file mode 100644 index 0000000..c9622d2 --- /dev/null +++ b/test/collection/conditional_ifelse_assign.h @@ -0,0 +1,25 @@ +typedef double scalar; +inline void test(scalar a) { +} + +inline scalar test_ret(scalar a) { + return 1.0; +} + +inline void broken() { + scalar a,b,c,d=1.0; + if(b > 0) { + a = b; + } else { + a = c; + } + if(b > 0) { + a = d; + } + if(d == 0) + a = c; + if(a == 0) + b = c; + else + b = d; +} From e90090b8b81b7e88ff4423f9ace5026bbac879fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Fri, 16 Oct 2015 14:25:35 +0200 Subject: [PATCH 13/21] First step to improve unionmatcher source transformations. --- conf.json | 3 +- .../core/transformation/TransformationUtil.h | 15 ++- include/core/utility/ClangMatcherExt.h | 5 +- include/core/utility/ClangUtil.h | 108 +++++++++++------- include/modules/ExplicitCast.h | 2 +- src/modules/ConditionalAssgnMatcher.cpp | 2 +- src/modules/ExplicitCast.cpp | 8 +- src/modules/IfElseAssign.cpp | 2 +- src/modules/UnionMatcher.cpp | 7 +- test/CMakeLists.txt | 30 +---- test/collection/explicit_cast.cpp | 21 ++++ test/collection/reCast.h | 6 + test/collection/unions.cpp | 28 +++++ test/src/test_explicitcast.cpp | 4 +- test/src/test_ifelse.cpp | 51 +++++++++ test/src/tests/ExplicitCastNoMatchSet.inl | 6 +- test/src/tests/IfElseMatchSet.inl | 0 test/src/tests/IfElseNoMatchSet.inl | 0 unit_tests.sh | 3 +- 19 files changed, 213 insertions(+), 88 deletions(-) create mode 100644 test/collection/explicit_cast.cpp create mode 100644 test/collection/reCast.h create mode 100644 test/collection/unions.cpp create mode 100644 test/src/test_ifelse.cpp create mode 100644 test/src/tests/IfElseMatchSet.inl create mode 100644 test/src/tests/IfElseNoMatchSet.inl diff --git a/conf.json b/conf.json index c9bc3f8..51efbb6 100644 --- a/conf.json +++ b/conf.json @@ -5,6 +5,7 @@ "type" : "scalar" }, "ExplicitCast" : { - "header" : "recast.h" + "header" : "reCast.h", + "function" : "reCast" } } diff --git a/include/core/transformation/TransformationUtil.h b/include/core/transformation/TransformationUtil.h index e68440f..0d5a6c2 100644 --- a/include/core/transformation/TransformationUtil.h +++ b/include/core/transformation/TransformationUtil.h @@ -54,12 +54,12 @@ inline StringRef whitespaces(const clang::SourceManager& sm, T node) { return spaces; } -inline clang::tooling::Replacement reCast(const clang::ASTContext& ac, const clang::ExplicitCastExpr* expr, const std::string& cast_from) { +inline clang::tooling::Replacement reCast(const clang::ASTContext& ac, const clang::ExplicitCastExpr* expr, const std::string& cast_from, const std::string& function) { auto& sm = ac.getSourceManager(); //SourceRange range = locOf(sm, expr); //CharSourceRange cast_crange = CharSourceRange::getCharRange(range); - std::string text("reCast<"); - text += typeOf(expr) + "," + cast_from + ">(" + clutil::node2str(ac, expr->getSubExprAsWritten()) + ")"; + std::string text(function); + text += "<" + typeOf(expr) + "," + cast_from + ">(" + clutil::node2str(ac, expr->getSubExprAsWritten()) + ")"; return Replacement(sm, expr, text); } @@ -130,6 +130,15 @@ inline clang::tooling::Replacement removeNode(const clang::ASTContext& ac, T nod return Replacement(sm, node, ""); } +template +inline bool removeNode_rew(clang::Rewriter& rw, T node) { + SourceRange range = locOf(rw.getSourceMgr(), node); + CharSourceRange crange = CharSourceRange::getCharRange(range); + clang::Rewriter::RewriteOptions opts; + opts.RemoveLineIfEmpty = true; + return rw.RemoveText(crange, opts); +} + /* //inline std::vector addExplicitCompare(const diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index c5a6303..0b5d031 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -26,7 +26,6 @@ __CKIND(ConstructorConversion) AST_POLYMORPHIC_MATCHER_P(isTypedef, AST_POLYMORPHIC_SUPPORTED_TYPES_2(Expr, Decl), std::string, type) { const auto typeOf_expr = Node.getType().getUnqualifiedType().getAsString(); - // LOG_DEBUG("isTypedef of '" << type << "' : " << typeOf_expr); return type == typeOf_expr; } @@ -44,7 +43,7 @@ const internal::VariadicDynCastAllOfMatcher< #define ancestor_or_self(NODE) \ anyOf(NODE, hasAncestor(NODE)) -/* + AST_MATCHER_P(IfStmt, hasThen, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 const Stmt *const Then = Node.getThen(); @@ -56,7 +55,7 @@ AST_MATCHER_P(IfStmt, hasElse, internal::Matcher, InnerMatcher) { const Stmt *const Else = Node.getElse(); return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); } -*/ + AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index c8c3ea2..079c732 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -17,11 +17,14 @@ #include #include +#include namespace opov { namespace clutil { +inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl); + template inline clang::SourceRange locOf(const clang::SourceManager& sm, T node, unsigned int offset = 0) { // offset=1 includes ';' (assuming no whitespaces) @@ -30,6 +33,19 @@ inline clang::SourceRange locOf(const clang::SourceManager& sm, T node, unsigned return {start, end.getLocWithOffset(offset)}; } +template +inline clang::SourceRange locOf(clang::ASTContext& ac, T node, bool semicolon = false) { + clang::SourceLocation start(node->getLocStart()); + clang::SourceLocation end(node->getLocEnd()); + if(semicolon) { + clang::SourceLocation semi_end = findSemiAfterLocation(end, ac, llvm::isa(node)); + return {start, semi_end.isValid() ? semi_end : end}; + } else { + return {start, end}; + } +} + + template inline std::string typeOf(NODE node) { return node->getType().getUnqualifiedType().getAsString(); @@ -57,9 +73,6 @@ inline const clang::FileEntry* fileEntryOf(const clang::SourceManager& sm, T nod template inline std::string fileOriginOf(const clang::SourceManager& sm, T node) { - //const auto range = locOf(sm, node); - //const std::pair DecomposedLocation = sm.getDecomposedLoc(range.getBegin()); - //const clang::FileEntry* Entry = sm.getFileEntryForID(DecomposedLocation.first); auto Entry = fileEntryOf(sm, node); if (Entry != NULL) { llvm::SmallString<256> FilePath(Entry->getName()); @@ -70,6 +83,11 @@ inline std::string fileOriginOf(const clang::SourceManager& sm, T node) { } } +inline unsigned declCount(const clang::TagDecl* node) { + auto range = node->decls(); + return std::distance(range.begin(), range.end()); +} + /* * Taken from the official clang documentation: * see: http://clang.llvm.org/docs/LibASTMatchersTutorial.html @@ -84,21 +102,6 @@ inline bool areSameExpr(clang::ASTContext* context, const clang::Expr* first, co return FirstID == SecondID; } -/* -inline std::string posOf(const clang::SourceManager& sm, const clang::Stmt* -expr) { - std::stringstream ss; - ss << "Location -- Line (S/E): " - << sm.getPresumedLineNumber(expr->getLocStart()) << "/" - << sm.getPresumedLineNumber(expr->getLocEnd()) - << " Column (S/E): " - << sm.getPresumedColumnNumber(expr->getLocStart()) << -"/" - << sm.getPresumedColumnNumber(expr->getLocEnd()); - return ss.str(); -} -*/ - template inline std::tuple rowOf(const clang::SourceManager& sm, T node) { auto range = locOf(sm, node); @@ -139,31 +142,50 @@ inline std::string node2str(const clang::ASTContext& ac, const clang::Decl* node return s.str(); } -/* -template -inline std::string node2str(const clang::SourceManager& sm, NODE expr) { - - auto range = locOf(sm, expr); - auto startData = sm.getCharacterData(range.getBegin()); - auto endData = sm.getCharacterData(range.getEnd()); - const std::string source_code = std::string(startData, endData - -startData); - - return source_code; -// auto range = locOf(sm, expr); -// auto p = sm.getDecomposedLoc(range.getBegin()); -// auto pe = sm.getDecomposedLoc(range.getEnd()); -// -// std::string text = -clang::Lexer::getSourceText(clang::CharSourceRange::getTokenRange(range), sm, -clang::LangOptions(), 0); -// if (text.at(text.size()-1) == ',') -// return -clang::Lexer::getSourceText(clang::CharSourceRange::getCharRange(range), sm, -clang::LangOptions(), 0); -// return text; -} -*/ +// Taken from File "Transform.cpp": lib/ARCMigrate/Transforms.cpp +inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, + clang::ASTContext &Ctx, + bool IsDecl) { + /// \brief \arg Loc is the end of a statement range. This returns the location + /// of the semicolon following the statement. + /// If no semicolon is found or the location is inside a macro, the returned + /// source location will be invalid. + using namespace clang; + using namespace llvm; + SourceManager &SM = Ctx.getSourceManager(); + if (loc.isMacroID()) { + if (!Lexer::isAtEndOfMacroExpansion(loc, SM, Ctx.getLangOpts(), &loc)) + return SourceLocation(); + } + loc = Lexer::getLocForEndOfToken(loc, /*Offset=*/0, SM, Ctx.getLangOpts()); + + // Break down the source location. + std::pair locInfo = SM.getDecomposedLoc(loc); + + // Try to load the file buffer. + bool invalidTemp = false; + StringRef file = SM.getBufferData(locInfo.first, &invalidTemp); + if (invalidTemp) + return SourceLocation(); + + const char *tokenBegin = file.data() + locInfo.second; + + // Lex from the start of the given location. + Lexer lexer(SM.getLocForStartOfFile(locInfo.first), + Ctx.getLangOpts(), + file.begin(), tokenBegin, file.end()); + Token tok; + lexer.LexFromRawLexer(tok); + if (tok.isNot(tok::semi)) { + if (!IsDecl) + return SourceLocation(); + // Declaration may be followed with other tokens; such as an __attribute, + // before ending with a semicolon. + return findSemiAfterLocation(tok.getLocation(), Ctx, /*IsDecl*/true); + } + + return tok.getLocation(); +} class TypeDeducer : public clang::RecursiveASTVisitor { private: diff --git a/include/modules/ExplicitCast.h b/include/modules/ExplicitCast.h index 63350f7..2d0f5c3 100644 --- a/include/modules/ExplicitCast.h +++ b/include/modules/ExplicitCast.h @@ -21,7 +21,7 @@ class ExplicitCastVisitor; class ExplicitCast : public opov::ASTMatcherModule { private: std::string header_cast; - std::string cast_stmt; + std::string stmt_cast; // std::unique_ptr visitor; public: diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 62030ab..3edc461 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -34,7 +34,7 @@ void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { void ConditionalAssgnMatcher::setupMatcher() { // TODO use ofType instead of just typedef? // We warn whenever an active type is present for such code structures. - // Even if there is no assignement. + // (Even if there is no assignement.) auto conditional = conditionalOperator(hasDescendant(expr(isTypedef(type_s)))).bind("conditional"); auto condassign = stmt(hasParent(compoundStmt()), descendant_or_self(conditional)).bind("conditional_root"); diff --git a/src/modules/ExplicitCast.cpp b/src/modules/ExplicitCast.cpp index a03e132..a77dc47 100644 --- a/src/modules/ExplicitCast.cpp +++ b/src/modules/ExplicitCast.cpp @@ -25,8 +25,8 @@ ExplicitCast::ExplicitCast() { } void ExplicitCast::setupOnce(const Configuration* config) { - config->getValue("ExplicitCast:header", header_cast, "recast.h"); - config->getValue("ExplicitCast:function", header_cast, "reCast"); + config->getValue("ExplicitCast:header", header_cast, "reCast.h"); + config->getValue("ExplicitCast:function", stmt_cast, "reCast"); } void ExplicitCast::setupMatcher() { @@ -47,15 +47,13 @@ void ExplicitCast::setupMatcher() { void ExplicitCast::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { const ExplicitCastExpr* ecast = result.Nodes.getNodeAs("cast"); - //auto ecast = const_cast(e); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(ecast, moduleName(), moduleDescription()); if(transform) { - LOG_MSG("Transforming - explicit"); auto& thandle = context->getTransformationHandler(); - auto replace = trutil::reCast(context->getASTContext(), ecast, type_s); + auto replace = trutil::reCast(context->getASTContext(), ecast, type_s, stmt_cast); thandle.addHeader(header_cast, clutil::locOf(context->getSourceManager(), ecast).getBegin()); thandle.addReplacements(replace); } diff --git a/src/modules/IfElseAssign.cpp b/src/modules/IfElseAssign.cpp index 71cc6d7..c322da6 100644 --- a/src/modules/IfElseAssign.cpp +++ b/src/modules/IfElseAssign.cpp @@ -38,7 +38,7 @@ void IfElseAssign::setupMatcher() { //auto assign = binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND); //auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); #define assign_bind(BIND) \ - binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND) + binaryOperator(hasOperatorName("="), hasLHS(declRefExpr()), hasDescendant(expr(isTypedef(type_s)))).bind(BIND) #define assign_expr(BIND) \ anyOf(compoundStmt(statementCountIs(1), has(assign_bind(BIND))), assign_bind(BIND)) diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index e9f6d82..fff626b 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -51,13 +51,18 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu if(transform) { auto& thandle = context->getTransformationHandler(); auto& ast_ctx = context->getASTContext(); + auto& rw = thandle.getRewriter(); if(is_anon) { const bool remove_union = std::distance(inv_union->field_begin(), inv_union->field_end()) == 1; + //LOG_MSG("Count: " < + +#pragma clang diagnostic ignored "-Wunused-value" + +typedef double scalar; +inline void test(scalar a) { +} + +inline scalar test_ret(scalar a) { + return 1.0; +} + +inline void broken() { + scalar a,b,c,d=1.0; + scalar* ptr = &a; + static_cast(*ptr); + static_cast(&a); + static_cast(ptr); + std::size_t i = reinterpret_cast(ptr); + i = static_cast(a); +} diff --git a/test/collection/reCast.h b/test/collection/reCast.h new file mode 100644 index 0000000..1f95881 --- /dev/null +++ b/test/collection/reCast.h @@ -0,0 +1,6 @@ +template struct ForPartialSpecialization { + static To reCast(const From& from ) { return (To) from;} +}; +template To reCast(const From& from) { + return ForPartialSpecialization::reCast(from); +} \ No newline at end of file diff --git a/test/collection/unions.cpp b/test/collection/unions.cpp new file mode 100644 index 0000000..5a72c6c --- /dev/null +++ b/test/collection/unions.cpp @@ -0,0 +1,28 @@ +typedef double scalar; + +struct test { + scalar a; + union { + ; + int c; + }; +scalar b; + union { + ; + int e; + double f; + }; +scalar d; + struct XX { + scalar a; + }; + struct YY { + scalar b; + int c; + }; + struct ZZ { + scalar d; + int e; + double f; + }; +}; \ No newline at end of file diff --git a/test/src/test_explicitcast.cpp b/test/src/test_explicitcast.cpp index a3a221b..aff91ed 100644 --- a/test/src/test_explicitcast.cpp +++ b/test/src/test_explicitcast.cpp @@ -17,7 +17,7 @@ #define HEADER "typedef double scalar;\n" -#define VARS "scalar a = 10.0; scalar aa = 10.5; scalar ab = 11.0;\n" +#define VARS "scalar a = 10.0; scalar aa = 10.5; scalar ab = 11.0; scalar* ptr = &a;\n" #define START_CODE "void container() {\nint res = 0.0;\n" #define END_CODE "\n};\n" #define MAKE_CODE(stmt) HEADER VARS START_CODE stmt ";" END_CODE @@ -101,7 +101,7 @@ SCENARIO("Explicit casts with scalars/integers. Module looks for type double and #undef _TYPE_ #undef VARS #define _TYPE_ "double" -#define VARS "double a = 10.0; double aa = 10.5; double ab = 11.0;\n" +#define VARS "double a = 10.0; double aa = 10.5; double ab = 11.0; double* ptr = &a;\n" SCENARIO("Explicit casts. Module produces one match for type " _TYPE_, "[" _TYPE_ "_match]") { GIVEN("The 'ExplicitCast' module with type: " _TYPE_) { opov::test::TestApp app(conf_double); diff --git a/test/src/test_ifelse.cpp b/test/src/test_ifelse.cpp new file mode 100644 index 0000000..419576d --- /dev/null +++ b/test/src/test_ifelse.cpp @@ -0,0 +1,51 @@ +/* + * test_ifelse.cpp + * + * Created on: Oct 16, 2015 + * Author: ahueck + */ + + + + +#include +#include + +#include +#include +#include + +#define CATCH_CONFIG_MAIN +#include + + +#define HEADER "typedef double scalar;" +#define VARS "scalar a = 10.0; scalar b = 10.5; scalar c = 11.0; scalar d = 11.0;int ai = 0.0;int bi = 0.0;int ci = 0.0;int di = 0.0;" +#define START_CODE "\nvoid container() {\n" +#define END_CODE "\n};\n" +#define MAKE_CODE(stmt) HEADER VARS START_CODE stmt END_CODE + +#define IF(A, B) "if(a == 0.) { " A " } else { " B " }" +#define IF_E(A, B) "if(a == 0.) { " A " } else { " B " }" + + +KICKOFF_TEST(opov::module::IfElseAssign(), MAKE_CODE(IF("a = b;", "a = c;")), IF("a = b;", "a = c;")) + +#define _TYPE_ "scalar" +SCENARIO("Conditional assign op. Module produces one match for type " _TYPE_, "[" _TYPE_ "_match]") { + GIVEN("The 'ConditionalAssgnMatcher' module with type: " _TYPE_) { + opov::test::TestApp app(conf); + app.init(); + app.addModule(new opov::module::IfElseAssign()); +#include "tests/IfElseMatchSet.inl" + } +} + +SCENARIO("Conditional assign op. Module produces no match for type " _TYPE_, "[" _TYPE_ "_nmatch]") { + GIVEN("The 'ConditionalAssgnMatcher' module with type: " _TYPE_) { + opov::test::TestApp app(conf); + app.init(); + app.addModule(new opov::module::IfElseAssign()); +#include "tests/IfElseNoMatchSet.inl" + } +} diff --git a/test/src/tests/ExplicitCastNoMatchSet.inl b/test/src/tests/ExplicitCastNoMatchSet.inl index 7bd3e66..c413cae 100644 --- a/test/src/tests/ExplicitCastNoMatchSet.inl +++ b/test/src/tests/ExplicitCastNoMatchSet.inl @@ -13,4 +13,8 @@ SIMPLE_TEST0("A static cast of a nested unary." SIMPLE_TEST0("A static cast of a nested unary of a binary." , SCAST("!!(a*a)")); SIMPLE_TEST0("A static cast of a scalar pointer." - , SCAST_TYPE("scalar*", "&a")); + , SCAST_TYPE(_TYPE_ "*", "&a")); +SIMPLE_TEST0("A static cast of a native scalar pointer." + , SCAST_TYPE(_TYPE_ "*", "ptr")); +SIMPLE_TEST0("A static cast of a dereferenced scalar pointer." + , SCAST_TYPE(_TYPE_, "*ptr")); \ No newline at end of file diff --git a/test/src/tests/IfElseMatchSet.inl b/test/src/tests/IfElseMatchSet.inl new file mode 100644 index 0000000..e69de29 diff --git a/test/src/tests/IfElseNoMatchSet.inl b/test/src/tests/IfElseNoMatchSet.inl new file mode 100644 index 0000000..e69de29 diff --git a/unit_tests.sh b/unit_tests.sh index 891253c..7de9c4e 100755 --- a/unit_tests.sh +++ b/unit_tests.sh @@ -19,4 +19,5 @@ verify explicitcast verify conditionalassgn verify explicitconstructor verify allimplicitconversion -verify globalscope \ No newline at end of file +verify globalscope +verify ifelse From 6390f38d14d9e22215f91b97a8c9a45ec6a610f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Fri, 16 Oct 2015 14:26:51 +0200 Subject: [PATCH 14/21] Fixed test file code. --- test/collection/unions.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/collection/unions.cpp b/test/collection/unions.cpp index 5a72c6c..2c50e73 100644 --- a/test/collection/unions.cpp +++ b/test/collection/unions.cpp @@ -1,26 +1,26 @@ typedef double scalar; struct test { - scalar a; union { - ; + scalar a; + }; + union { + scalar b; int c; - }; -scalar b; + }; union { - ; + scalar d; int e; double f; - }; -scalar d; - struct XX { + }; + union XX { scalar a; }; - struct YY { + union YY { scalar b; int c; }; - struct ZZ { + union ZZ { scalar d; int e; double f; From 2e2705b4abb7dbf6753dc7779e45ba63f59620ca Mon Sep 17 00:00:00 2001 From: ahueck Date: Fri, 16 Oct 2015 19:44:09 +0200 Subject: [PATCH 15/21] UnionMatcher can now correctly transform partially using newly introduced FixItHint usage. --- .../transformation/TransformationHandler.h | 1 + include/core/utility/ClangMatcherExt.h | 4 +-- include/core/utility/ClangUtil.h | 28 ++++++++++++----- .../transformation/TransformationHandler.cpp | 31 +++++++++++++++++++ src/modules/UnionMatcher.cpp | 31 ++++++++++++------- test/collection/unions.cpp | 4 ++- 6 files changed, 76 insertions(+), 23 deletions(-) diff --git a/include/core/transformation/TransformationHandler.h b/include/core/transformation/TransformationHandler.h index 5b96cb0..220bdd2 100644 --- a/include/core/transformation/TransformationHandler.h +++ b/include/core/transformation/TransformationHandler.h @@ -45,6 +45,7 @@ class TransformationHandler { void addHeader(const std::string& header, clang::SourceLocation loc); void addReplacements(const clang::tooling::Replacement& replacement); void addReplacements(const std::vector& replacements); + void addReplacements(const clang::FixItHint& Hint); TUReplacementsMap& getAllReplacements(); diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index 0b5d031..3c0ab25 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -43,7 +43,7 @@ const internal::VariadicDynCastAllOfMatcher< #define ancestor_or_self(NODE) \ anyOf(NODE, hasAncestor(NODE)) - +/* AST_MATCHER_P(IfStmt, hasThen, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 const Stmt *const Then = Node.getThen(); @@ -55,7 +55,7 @@ AST_MATCHER_P(IfStmt, hasElse, internal::Matcher, InnerMatcher) { const Stmt *const Else = Node.getElse(); return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); } - +*/ AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index 079c732..900669c 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -23,6 +23,7 @@ namespace opov { namespace clutil { +inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl); inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl); template @@ -38,7 +39,8 @@ inline clang::SourceRange locOf(clang::ASTContext& ac, T node, bool semicolon = clang::SourceLocation start(node->getLocStart()); clang::SourceLocation end(node->getLocEnd()); if(semicolon) { - clang::SourceLocation semi_end = findSemiAfterLocation(end, ac, llvm::isa(node)); + clang::SourceLocation semi_end = findLocationAfterSemi(end, ac, llvm::isa(node)); + //LOG_MSG(semi_end.isValid() << ": " << semi_end.printToString(ac.getSourceManager()) << ", " << end.printToString(ac.getSourceManager())); return {start, semi_end.isValid() ? semi_end : end}; } else { return {start, end}; @@ -103,20 +105,20 @@ inline bool areSameExpr(clang::ASTContext* context, const clang::Expr* first, co } template -inline std::tuple rowOf(const clang::SourceManager& sm, T node) { - auto range = locOf(sm, node); +inline std::tuple rowOf(const clang::SourceManager& sm, T node, bool semicolon = false) { + auto range = locOf(sm, node, semicolon); return std::make_tuple(sm.getPresumedLineNumber(range.getBegin()), sm.getPresumedLineNumber(range.getEnd())); } template -inline std::tuple colOf(const clang::SourceManager& sm, T node) { - auto range = locOf(sm, node); +inline std::tuple colOf(const clang::SourceManager& sm, T node, bool semicolon = false) { + auto range = locOf(sm, node, semicolon); return std::make_tuple(sm.getPresumedColumnNumber(range.getBegin()), sm.getPresumedColumnNumber(range.getEnd())); } template -inline std::tuple posOf(const clang::SourceManager& sm, T node) { - return std::tuple_cat(rowOf(sm, node), colOf(sm, node)); +inline std::tuple posOf(const clang::SourceManager& sm, T node, bool semicolon = false) { + return std::tuple_cat(rowOf(sm, node, semicolon), colOf(sm, node, semicolon)); } template @@ -142,7 +144,17 @@ inline std::string node2str(const clang::ASTContext& ac, const clang::Decl* node return s.str(); } -// Taken from File "Transform.cpp": lib/ARCMigrate/Transforms.cpp + +// Both functions taken from File "Transform.cpp": lib/ARCMigrate/Transforms.cpp +inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl) { + using namespace clang; + SourceLocation SemiLoc = findSemiAfterLocation(loc, Ctx, IsDecl); + if (SemiLoc.isInvalid()) { + return SourceLocation(); + } + return SemiLoc.getLocWithOffset(1); +} + inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl) { diff --git a/src/core/transformation/TransformationHandler.cpp b/src/core/transformation/TransformationHandler.cpp index a82c99a..b95fca2 100644 --- a/src/core/transformation/TransformationHandler.cpp +++ b/src/core/transformation/TransformationHandler.cpp @@ -40,6 +40,37 @@ void TransformationHandler::addReplacements(const clang::tooling::Replacement& r */ } +void TransformationHandler::addReplacements(const clang::FixItHint& Hint) { + static clang::Rewriter::RewriteOptions nl_opt; + nl_opt.RemoveLineIfEmpty = true; + if (Hint.CodeToInsert.empty()) { + if (!Hint.InsertFromRange.isValid()) { + rewriter.RemoveText(Hint.RemoveRange, nl_opt); + //rewriter.ReplaceText(Hint.RemoveRange.getAsRange(), ""); + } + //if (Hint.InsertFromRange.isValid()) + // commit.insertFromRange(Hint.RemoveRange.getBegin(), Hint.InsertFromRange, /*afterToken=*/false, Hint.BeforePreviousInsertions); + //else + // commit.remove(Hint.RemoveRange); + } else { + if (Hint.RemoveRange.isTokenRange() ||Hint.RemoveRange.getBegin() != Hint.RemoveRange.getEnd()) { + //commit.replace(Hint.RemoveRange, Hint.CodeToInsert); + rewriter.ReplaceText(Hint.RemoveRange.getAsRange(), Hint.CodeToInsert); + } else { + //commit.insert(Hint.RemoveRange.getBegin(), Hint.CodeToInsert,/*afterToken=*/false, Hint.BeforePreviousInsertions); + rewriter.InsertText(Hint.RemoveRange.getBegin(),Hint.CodeToInsert, Hint.BeforePreviousInsertions, true); + //rewriter.ReplaceText(Hint.RemoveRange.getBegin(),0, Hint.CodeToInsert); + } + } + /* + TranslationUnitReplacements& tunit = replacements[source]; + if (tunit.MainSourceFile.empty()) { + tunit.MainSourceFile = source; + } + tunit.Replacements.push_back(replacement); + */ +} + void TransformationHandler::addHeader(const std::string& header, clang::SourceLocation loc) { addReplacements(includes->addAngledInclude(loc, header)); } diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index fff626b..c40edc4 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -47,23 +47,30 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu auto& ihandle = context->getIssueHandler(); ihandle.addIssue(inv_union, moduleName(), moduleDescription(), message.str()); - // TODO improve by using a custom union traversal and slowly building a replacement string if(transform) { auto& thandle = context->getTransformationHandler(); auto& ast_ctx = context->getASTContext(); - auto& rw = thandle.getRewriter(); if(is_anon) { - const bool remove_union = std::distance(inv_union->field_begin(), inv_union->field_end()) == 1; - //LOG_MSG("Count: " <field_begin()))); + for(auto it = inv_union->field_begin(); it != inv_union->field_end(); ++it) { + if(it == inv_union->field_begin()) { + continue; + } + thandle.addReplacements(FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, *it) + ";", true) ); + } + } else { + + for(auto fd : fieldDecls) { + thandle.addReplacements(FixItHint::CreateRemoval(clutil::locOf(ast_ctx, fd, true))); + thandle.addReplacements(FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, fd) + ";", true) ); + } } } } else { diff --git a/test/collection/unions.cpp b/test/collection/unions.cpp index 2c50e73..c5bc604 100644 --- a/test/collection/unions.cpp +++ b/test/collection/unions.cpp @@ -1,3 +1,5 @@ +#pragma clang diagnostic ignored "-Wnull-character" + typedef double scalar; struct test { @@ -25,4 +27,4 @@ struct test { int e; double f; }; -}; \ No newline at end of file +}; From 3f7f31f00b4d0c9bb68b7e14c2efbf5d642907ae Mon Sep 17 00:00:00 2001 From: ahueck Date: Sat, 17 Oct 2015 13:01:52 +0200 Subject: [PATCH 16/21] GlobalScope should find function-calls with only ::-specifier now. --- src/modules/GlobalScope.cpp | 10 +++++++--- test/collection/globalscope.cpp | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 test/collection/globalscope.cpp diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index b371c89..73355d1 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -26,13 +26,17 @@ void GlobalScope::setupOnce(const Configuration* config) { } void GlobalScope::setupMatcher() { - auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), - has(nestedNameSpecifier()))).bind("global"); + //auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), + // has(nestedNameSpecifier()))).bind("global"); + auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s)))))/*.bind("decl")*/ + ,has(nestedNameSpecifier(isGlobalNamespace())) + ).bind("global"); this->addMatcher(declref_matcher); } void GlobalScope::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const Expr* call = result.Nodes.getNodeAs("global"); + const DeclRefExpr* call = result.Nodes.getNodeAs("global"); + //const FunctionDecl* func_decl = result.Nodes.getNodeAs("decl"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(call, moduleName(), moduleDescription()); diff --git a/test/collection/globalscope.cpp b/test/collection/globalscope.cpp new file mode 100644 index 0000000..ebce84c --- /dev/null +++ b/test/collection/globalscope.cpp @@ -0,0 +1,35 @@ +#pragma clang diagnostic ignored "-Wunused-value" +typedef double scalar; + +class X; +int a = 1; +scalar b = 1.0; +double c = 2.0; + +void f(const X& x); + +class X { +public: + friend void f(const X& x) { } +}; + +void g(scalar a); +void h(double b); + +namespace ns { +void f(const X& x); +void g(scalar z); +void f(); +void call() { + ::f(X()); + ns::f(X()); + f(); + ::a; + ::b; + ::g(scalar(1.0)); + ::g(::b); + ::g(::b*scalar(1)); + g(scalar(1.0)); + ns::g(scalar(1.0)); +} +} From 8879ce0d192fd6839c8c50b74593c401e37fbf9e Mon Sep 17 00:00:00 2001 From: ahueck Date: Sat, 17 Oct 2015 13:52:28 +0200 Subject: [PATCH 17/21] Backwards compatibility for a specific matcher, clean up for unionmatcher: no need for declcollector anymore. --- CMakeLists.txt | 2 +- include/core/utility/ClangMatcherExt.h | 8 +++--- include/modules/FieldDeclCollector.h | 34 ------------------------ include/modules/UnionMatcher.h | 5 ---- src/modules/FieldDeclCollector.cpp | 36 -------------------------- src/modules/GlobalScope.cpp | 4 +-- src/modules/IfElseAssign.cpp | 4 +-- src/modules/ImplicitConversion.cpp | 6 +---- src/modules/UnionMatcher.cpp | 16 +++++++++--- 9 files changed, 22 insertions(+), 93 deletions(-) delete mode 100644 include/modules/FieldDeclCollector.h delete mode 100644 src/modules/FieldDeclCollector.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e03bff..d4e557d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,7 +31,7 @@ set(PSOURCES src/AnalyzerFactory.cpp src/ModuleConsumer.cpp src/modules/ImplicitConditionMatcher.cpp -src/modules/FieldDeclCollector.cpp +#src/modules/FieldDeclCollector.cpp src/modules/UnionMatcher.cpp src/modules/ExplicitCast.cpp src/modules/ConditionalAssgnMatcher.cpp diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index 3c0ab25..bfb7b4f 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -43,19 +43,19 @@ const internal::VariadicDynCastAllOfMatcher< #define ancestor_or_self(NODE) \ anyOf(NODE, hasAncestor(NODE)) -/* -AST_MATCHER_P(IfStmt, hasThen, internal::Matcher, InnerMatcher) { +// TODO remove this code duplication (hasThen) once backwards compatibility is not necessary +AST_MATCHER_P(IfStmt, hasThenStmt, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 const Stmt *const Then = Node.getThen(); return (Then != nullptr && InnerMatcher.matches(*Then, Finder, Builder)); } -AST_MATCHER_P(IfStmt, hasElse, internal::Matcher, InnerMatcher) { +// TODO remove this code duplication (hasElse) once backwards compatibility is not necessary +AST_MATCHER_P(IfStmt, hasElseStmt, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2934 const Stmt *const Else = Node.getElse(); return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); } -*/ AST_MATCHER(TagDecl, isUnion) { return Node.isUnion(); diff --git a/include/modules/FieldDeclCollector.h b/include/modules/FieldDeclCollector.h deleted file mode 100644 index 3cc6999..0000000 --- a/include/modules/FieldDeclCollector.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * FieldDeclCollector.h - * - * Created on: Jun 9, 2014 - * Author: ahueck - */ - -#ifndef FIELDDECLCOLLECTOR_H_ -#define FIELDDECLCOLLECTOR_H_ - -#include - -#include -#include - -namespace opov { -namespace module { - -class FieldDeclCollector : public clang::RecursiveASTVisitor { - private: - std::vector violations; - const std::string& type; - - public: - FieldDeclCollector(const std::string& type = ""); - std::vector extractDecl(clang::CXXRecordDecl* unionDecl); - bool VisitFieldDecl(clang::FieldDecl* decl); - virtual ~FieldDeclCollector(); -}; - -} /* namespace module */ -} /* namespace opov */ - -#endif /* FIELDDECLCOLLECTOR_H_ */ diff --git a/include/modules/UnionMatcher.h b/include/modules/UnionMatcher.h index c343fc8..ef812ca 100644 --- a/include/modules/UnionMatcher.h +++ b/include/modules/UnionMatcher.h @@ -11,16 +11,11 @@ #include #include -#include namespace opov { namespace module { -class FieldDeclCollector; - class UnionMatcher : public opov::ASTMatcherModule { - private: - std::unique_ptr visitor; public: UnionMatcher(); diff --git a/src/modules/FieldDeclCollector.cpp b/src/modules/FieldDeclCollector.cpp deleted file mode 100644 index 4cc38ca..0000000 --- a/src/modules/FieldDeclCollector.cpp +++ /dev/null @@ -1,36 +0,0 @@ -/* - * FieldDeclCollector.cpp - * - * Created on: Jun 9, 2014 - * Author: ahueck - */ - -#include -#include - -namespace opov { -namespace module { - -FieldDeclCollector::FieldDeclCollector(const std::string& type) - : violations() - , type(type) { -} - -std::vector FieldDeclCollector::extractDecl(clang::CXXRecordDecl* unionDecl) { - violations.clear(); - TraverseDecl(unionDecl); - return violations; -} - -bool FieldDeclCollector::VisitFieldDecl(clang::FieldDecl* decl) { - if (clutil::typeOf(decl) == type) { - violations.push_back(decl); - } - return true; -} - -FieldDeclCollector::~FieldDeclCollector() { -} - -} /* namespace module */ -} /* namespace opov */ diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index 73355d1..c37b5b0 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -28,8 +28,8 @@ void GlobalScope::setupOnce(const Configuration* config) { void GlobalScope::setupMatcher() { //auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), // has(nestedNameSpecifier()))).bind("global"); - auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s)))))/*.bind("decl")*/ - ,has(nestedNameSpecifier(isGlobalNamespace())) + auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))) + , has(nestedNameSpecifier(isGlobalNamespace())) ).bind("global"); this->addMatcher(declref_matcher); } diff --git a/src/modules/IfElseAssign.cpp b/src/modules/IfElseAssign.cpp index c322da6..c25be50 100644 --- a/src/modules/IfElseAssign.cpp +++ b/src/modules/IfElseAssign.cpp @@ -43,8 +43,8 @@ void IfElseAssign::setupMatcher() { anyOf(compoundStmt(statementCountIs(1), has(assign_bind(BIND))), assign_bind(BIND)) auto conditional = ifStmt(anyOf( - allOf(hasThen(assign_expr("then")), hasElse(assign_expr("else"))) - , allOf(hasThen(assign_expr("then")), unless(hasElse(stmt()))))).bind("conditional"); + allOf(hasThenStmt(assign_expr("then")), hasElseStmt(assign_expr("else"))) + , allOf(hasThenStmt(assign_expr("then")), unless(hasElseStmt(stmt()))))).bind("conditional"); this->addMatcher(conditional); } diff --git a/src/modules/ImplicitConversion.cpp b/src/modules/ImplicitConversion.cpp index b5d0c39..0039c80 100644 --- a/src/modules/ImplicitConversion.cpp +++ b/src/modules/ImplicitConversion.cpp @@ -31,11 +31,7 @@ void ImplicitConversion::setupOnce(const Configuration* config) { void ImplicitConversion::setupMatcher() { StatementMatcher impl_conversion = materializeTemporaryExpr(hasTemporary(ignoringImpCasts( constructExpr(hasImplicitConversion(type_s), unless(temporaryObjectExpr())).bind("conversion")))); - /*constructExpr( - unless(hasParent(varDecl())) // TODO remove varDecl req.? - , - hasImplicitConversion(type_s)).bind("conversion"); - */ + this->addMatcher(impl_conversion); } diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index c40edc4..6713e7b 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -6,7 +6,6 @@ */ #include -#include #include #include #include @@ -16,6 +15,9 @@ #include #include +#include +#include + namespace opov { namespace module { @@ -26,7 +28,7 @@ UnionMatcher::UnionMatcher() { } void UnionMatcher::setupOnce(const Configuration* config) { - visitor = opov::util::make_unique(type_s); + } void UnionMatcher::setupMatcher() { @@ -38,14 +40,20 @@ void UnionMatcher::setupMatcher() { void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { const CXXRecordDecl* inv_union = result.Nodes.getNodeAs("union"); const bool is_anon = inv_union->isAnonymousStructOrUnion(); - auto fieldDecls = visitor->extractDecl(const_cast(inv_union)); + std::vector fieldDecls(inv_union->field_begin(), inv_union->field_end()); + auto filter_end = std::remove_if(fieldDecls.begin(), fieldDecls.end(), + [&](const FieldDecl* decl) { + return clutil::typeOf(decl) != type_s; + } + ); + fieldDecls.erase(filter_end, fieldDecls.end()); std::stringstream message; message << (is_anon ? "Anonymous union" : "Union") << " with typedef member. " << fieldDecls.size() << " fieldDecl violate the convention."; auto& ihandle = context->getIssueHandler(); - ihandle.addIssue(inv_union, moduleName(), moduleDescription(), message.str()); + ihandle.addIssue(inv_union, moduleName(), moduleDescription()); if(transform) { auto& thandle = context->getTransformationHandler(); From 2e266b31c5b271422be725a03f90fd82606a1dbd Mon Sep 17 00:00:00 2001 From: ahueck Date: Sat, 17 Oct 2015 16:26:33 +0200 Subject: [PATCH 18/21] Small changes to implicit condition matcher, added a small test file --- src/modules/ImplicitConditionMatcher.cpp | 6 +++++- test/collection/globalscope.cpp | 1 + test/collection/implicit_condition.cpp | 12 ++++++++++++ test/collection/implicit_conversion.cpp | 17 +++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 test/collection/implicit_condition.cpp create mode 100644 test/collection/implicit_conversion.cpp diff --git a/src/modules/ImplicitConditionMatcher.cpp b/src/modules/ImplicitConditionMatcher.cpp index 3ef4f67..0ad3df6 100644 --- a/src/modules/ImplicitConditionMatcher.cpp +++ b/src/modules/ImplicitConditionMatcher.cpp @@ -36,8 +36,12 @@ void ImplicitConditionMatcher::setupMatcher() { auto unaryMatch = unaryOperator(hasUnaryOperand(implicitCastExpr(isFloatingToBoolean(), hasSourceExpression(ofType(type_s))))) .bind("unary"); + + /*auto typedef_condition = + hasCondition(anyOf(unaryMatch, expr(hasDescendant(unaryMatch)), impl, expr(hasDescendant(impl))));*/ + auto typedef_condition = - hasCondition(anyOf(unaryMatch, expr(hasDescendant(unaryMatch)), impl, expr(hasDescendant(impl)))); + hasCondition(anyOf(descendant_or_self(unaryMatch), descendant_or_self(impl))); StatementMatcher all_cond = stmt(anyOf(ifStmt(typedef_condition), forStmt(typedef_condition), doStmt(typedef_condition), diff --git a/test/collection/globalscope.cpp b/test/collection/globalscope.cpp index ebce84c..a644250 100644 --- a/test/collection/globalscope.cpp +++ b/test/collection/globalscope.cpp @@ -29,6 +29,7 @@ void call() { ::g(scalar(1.0)); ::g(::b); ::g(::b*scalar(1)); + ::h(::b); g(scalar(1.0)); ns::g(scalar(1.0)); } diff --git a/test/collection/implicit_condition.cpp b/test/collection/implicit_condition.cpp new file mode 100644 index 0000000..f7ce30b --- /dev/null +++ b/test/collection/implicit_condition.cpp @@ -0,0 +1,12 @@ +typedef double scalar; + +void f() { + scalar a; + if(a) {} + if(!a) {} + if(!!a) {} +} + +int main() { + return 1; +} diff --git a/test/collection/implicit_conversion.cpp b/test/collection/implicit_conversion.cpp new file mode 100644 index 0000000..fffed68 --- /dev/null +++ b/test/collection/implicit_conversion.cpp @@ -0,0 +1,17 @@ +typedef double scalar; + +class X { +public: + X(scalar a); + bool operator==(const X& other); +}; + +void f() { + X a(1.0); + if(a == 1.0) {} + if(a == 2*2) {} +} + +int main() { + return 1; +} From 69c02feb2689bf9cbcf1f6b922f4911668f23d52 Mon Sep 17 00:00:00 2001 From: ahueck Date: Sat, 17 Oct 2015 19:46:21 +0200 Subject: [PATCH 19/21] Code formatting: preparation for merge. --- include/core/AbstractModuleConsumer.h | 2 +- include/core/issue/IssueHandler.h | 3 +- include/core/issue/IssueHandler.hpp | 3 +- include/core/utility/ClangMatcherExt.h | 16 ++-- include/core/utility/ClangUtil.h | 30 ++++---- include/core/utility/Util.h | 2 +- include/modules/ConditionalAssgnMatcher.h | 11 ++- include/modules/GlobalScope.h | 2 +- include/modules/IfElseAssign.h | 8 +- include/modules/ImplicitConditionMatcher.h | 2 +- include/modules/ImplicitConversion.h | 2 +- include/modules/UnionMatcher.h | 1 - src/AnalyzerFactory.cpp | 6 +- src/ModuleConsumer.cpp | 2 +- src/OpOvApp.cpp | 2 +- src/core/AbstractFactory.cpp | 2 +- src/core/Application.cpp | 5 +- src/core/reporting/CSVReporter.cpp | 28 +++---- .../transformation/TransformationHandler.cpp | 20 ++--- src/modules/ConditionalAssgnMatcher.cpp | 29 ++++---- src/modules/ExplicitCast.cpp | 2 +- src/modules/GlobalScope.cpp | 9 +-- src/modules/IfElseAssign.cpp | 73 +++++++++---------- src/modules/ImplicitConditionMatcher.cpp | 6 +- src/modules/ImplicitConversion.cpp | 2 +- src/modules/UnionMatcher.cpp | 33 ++++----- 26 files changed, 141 insertions(+), 160 deletions(-) diff --git a/include/core/AbstractModuleConsumer.h b/include/core/AbstractModuleConsumer.h index ddb0c69..fde1504 100644 --- a/include/core/AbstractModuleConsumer.h +++ b/include/core/AbstractModuleConsumer.h @@ -24,7 +24,7 @@ class AbstractModuleConsumer : public clang::ASTConsumer { public: AbstractModuleConsumer(Module* module, ModuleContext* mcontext); - virtual void Initialize(clang::ASTContext &Context) override; + virtual void Initialize(clang::ASTContext& Context) override; void HandleTranslationUnit(clang::ASTContext& Context) override; virtual ~AbstractModuleConsumer(); }; diff --git a/include/core/issue/IssueHandler.h b/include/core/issue/IssueHandler.h index 14622eb..042da76 100644 --- a/include/core/issue/IssueHandler.h +++ b/include/core/issue/IssueHandler.h @@ -32,8 +32,7 @@ class IssueHandler { void setSource(const std::string& source); void init(clang::ASTContext* ac); template - void addIssue(T node, const std::string& module, - const std::string& module_descr, std::string message = ""); + void addIssue(T node, const std::string& module, const std::string& module_descr, std::string message = ""); TUIssuesMap& getAllIssues(); void clear(); virtual ~IssueHandler(); diff --git a/include/core/issue/IssueHandler.hpp b/include/core/issue/IssueHandler.hpp index 3da1cc2..1f0e188 100644 --- a/include/core/issue/IssueHandler.hpp +++ b/include/core/issue/IssueHandler.hpp @@ -19,8 +19,7 @@ namespace opov { template -void IssueHandler::addIssue(T node, - const std::string& module, const std::string& module_descr, std::string message) { +void IssueHandler::addIssue(T node, const std::string& module, const std::string& module_descr, std::string message) { auto& sm = ac->getSourceManager(); std::shared_ptr issue = std::make_shared(); std::string issue_file = clutil::fileOriginOf(sm, node); diff --git a/include/core/utility/ClangMatcherExt.h b/include/core/utility/ClangMatcherExt.h index bfb7b4f..e4e0126 100644 --- a/include/core/utility/ClangMatcherExt.h +++ b/include/core/utility/ClangMatcherExt.h @@ -33,27 +33,23 @@ AST_MATCHER(Stmt, notABinaryExpr) { return !isa(Node); } -const internal::VariadicDynCastAllOfMatcher< - Stmt, - ParenExpr> parenExpr; +const internal::VariadicDynCastAllOfMatcher parenExpr; -#define descendant_or_self(NODE) \ - anyOf(NODE, hasDescendant(NODE)) +#define descendant_or_self(NODE) anyOf(NODE, hasDescendant(NODE)) -#define ancestor_or_self(NODE) \ - anyOf(NODE, hasAncestor(NODE)) +#define ancestor_or_self(NODE) anyOf(NODE, hasAncestor(NODE)) // TODO remove this code duplication (hasThen) once backwards compatibility is not necessary AST_MATCHER_P(IfStmt, hasThenStmt, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2922 - const Stmt *const Then = Node.getThen(); + const Stmt* const Then = Node.getThen(); return (Then != nullptr && InnerMatcher.matches(*Then, Finder, Builder)); } // TODO remove this code duplication (hasElse) once backwards compatibility is not necessary AST_MATCHER_P(IfStmt, hasElseStmt, internal::Matcher, InnerMatcher) { // Taken from the current version of Clangs ASTMatchers.h file: Line 2934 - const Stmt *const Else = Node.getElse(); + const Stmt* const Else = Node.getElse(); return (Else != nullptr && InnerMatcher.matches(*Else, Finder, Builder)); } @@ -64,7 +60,7 @@ AST_MATCHER(TagDecl, isUnion) { AST_MATCHER_P(Stmt, ofType, std::string, type) { opov::clutil::TypeDeducer deducer(type); const bool is_type = deducer.hasType(const_cast(&Node)); - //LOG_DEBUG("ofType: '" << type << "' : " << is_type); + // LOG_DEBUG("ofType: '" << type << "' : " << is_type); return is_type; } diff --git a/include/core/utility/ClangUtil.h b/include/core/utility/ClangUtil.h index 900669c..1dbe4ad 100644 --- a/include/core/utility/ClangUtil.h +++ b/include/core/utility/ClangUtil.h @@ -23,8 +23,8 @@ namespace opov { namespace clutil { -inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl); -inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl); +inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext& Ctx, bool IsDecl); +inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext& Ctx, bool IsDecl); template inline clang::SourceRange locOf(const clang::SourceManager& sm, T node, unsigned int offset = 0) { @@ -38,16 +38,16 @@ template inline clang::SourceRange locOf(clang::ASTContext& ac, T node, bool semicolon = false) { clang::SourceLocation start(node->getLocStart()); clang::SourceLocation end(node->getLocEnd()); - if(semicolon) { + if (semicolon) { clang::SourceLocation semi_end = findLocationAfterSemi(end, ac, llvm::isa(node)); - //LOG_MSG(semi_end.isValid() << ": " << semi_end.printToString(ac.getSourceManager()) << ", " << end.printToString(ac.getSourceManager())); + // LOG_MSG(semi_end.isValid() << ": " << semi_end.printToString(ac.getSourceManager()) << ", " << + // end.printToString(ac.getSourceManager())); return {start, semi_end.isValid() ? semi_end : end}; } else { return {start, end}; } } - template inline std::string typeOf(NODE node) { return node->getType().getUnqualifiedType().getAsString(); @@ -117,7 +117,8 @@ inline std::tuple colOf(const clang::SourceManager& sm, T no } template -inline std::tuple posOf(const clang::SourceManager& sm, T node, bool semicolon = false) { +inline std::tuple posOf(const clang::SourceManager& sm, T node, + bool semicolon = false) { return std::tuple_cat(rowOf(sm, node, semicolon), colOf(sm, node, semicolon)); } @@ -144,9 +145,8 @@ inline std::string node2str(const clang::ASTContext& ac, const clang::Decl* node return s.str(); } - // Both functions taken from File "Transform.cpp": lib/ARCMigrate/Transforms.cpp -inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext &Ctx, bool IsDecl) { +inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, clang::ASTContext& Ctx, bool IsDecl) { using namespace clang; SourceLocation SemiLoc = findSemiAfterLocation(loc, Ctx, IsDecl); if (SemiLoc.isInvalid()) { @@ -155,16 +155,14 @@ inline clang::SourceLocation findLocationAfterSemi(clang::SourceLocation loc, cl return SemiLoc.getLocWithOffset(1); } -inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, - clang::ASTContext &Ctx, - bool IsDecl) { +inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, clang::ASTContext& Ctx, bool IsDecl) { /// \brief \arg Loc is the end of a statement range. This returns the location /// of the semicolon following the statement. /// If no semicolon is found or the location is inside a macro, the returned /// source location will be invalid. using namespace clang; using namespace llvm; - SourceManager &SM = Ctx.getSourceManager(); + SourceManager& SM = Ctx.getSourceManager(); if (loc.isMacroID()) { if (!Lexer::isAtEndOfMacroExpansion(loc, SM, Ctx.getLangOpts(), &loc)) return SourceLocation(); @@ -180,12 +178,10 @@ inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, if (invalidTemp) return SourceLocation(); - const char *tokenBegin = file.data() + locInfo.second; + const char* tokenBegin = file.data() + locInfo.second; // Lex from the start of the given location. - Lexer lexer(SM.getLocForStartOfFile(locInfo.first), - Ctx.getLangOpts(), - file.begin(), tokenBegin, file.end()); + Lexer lexer(SM.getLocForStartOfFile(locInfo.first), Ctx.getLangOpts(), file.begin(), tokenBegin, file.end()); Token tok; lexer.LexFromRawLexer(tok); if (tok.isNot(tok::semi)) { @@ -193,7 +189,7 @@ inline clang::SourceLocation findSemiAfterLocation(clang::SourceLocation loc, return SourceLocation(); // Declaration may be followed with other tokens; such as an __attribute, // before ending with a semicolon. - return findSemiAfterLocation(tok.getLocation(), Ctx, /*IsDecl*/true); + return findSemiAfterLocation(tok.getLocation(), Ctx, /*IsDecl*/ true); } return tok.getLocation(); diff --git a/include/core/utility/Util.h b/include/core/utility/Util.h index fcd2239..ad42ba7 100644 --- a/include/core/utility/Util.h +++ b/include/core/utility/Util.h @@ -29,7 +29,7 @@ inline std::vector split(const std::string& input, char delimiter = return tokens; } -template +template std::string num2str(T val) { std::stringstream sstream; sstream << val; diff --git a/include/modules/ConditionalAssgnMatcher.h b/include/modules/ConditionalAssgnMatcher.h index f413d34..6b08c18 100644 --- a/include/modules/ConditionalAssgnMatcher.h +++ b/include/modules/ConditionalAssgnMatcher.h @@ -18,13 +18,11 @@ namespace module { // class ConditionalAssgnVisitor; class ConditionalAssgnMatcher : public opov::ASTMatcherModule { -private: + private: unsigned var_counter; - typedef struct { - std::string type, variable, lhs, rhs, replacement; - } conditional_data; + typedef struct { std::string type, variable, lhs, rhs, replacement; } conditional_data; -public: + public: ConditionalAssgnMatcher(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; @@ -32,7 +30,8 @@ class ConditionalAssgnMatcher : public opov::ASTMatcherModule { virtual std::string moduleName() override; virtual std::string moduleDescription() override; virtual ~ConditionalAssgnMatcher(); -private: + + private: void toString(clang::ASTContext& ac, const clang::Expr* e, conditional_data& d, int counter = 0); conditional_data buildReplacement(clang::ASTContext& ac, const clang::ConditionalOperator* e); }; diff --git a/include/modules/GlobalScope.h b/include/modules/GlobalScope.h index 6ae03e3..2318a45 100644 --- a/include/modules/GlobalScope.h +++ b/include/modules/GlobalScope.h @@ -16,7 +16,7 @@ namespace opov { namespace module { class GlobalScope : public opov::ASTMatcherModule { -public: + public: GlobalScope(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/IfElseAssign.h b/include/modules/IfElseAssign.h index 0690867..2f0f8b2 100644 --- a/include/modules/IfElseAssign.h +++ b/include/modules/IfElseAssign.h @@ -14,7 +14,7 @@ namespace opov { namespace module { class IfElseAssign : public opov::ASTMatcherModule { -public: + public: IfElseAssign(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; @@ -22,8 +22,10 @@ class IfElseAssign : public opov::ASTMatcherModule { virtual std::string moduleName() override; virtual std::string moduleDescription() override; virtual ~IfElseAssign(); -private: - std::string toString(clang::ASTContext& ac, const clang::IfStmt* stmt, const clang::BinaryOperator* then, const clang::BinaryOperator* else_e); + + private: + std::string toString(clang::ASTContext& ac, const clang::IfStmt* stmt, const clang::BinaryOperator* then, + const clang::BinaryOperator* else_e); }; } /* namespace module */ diff --git a/include/modules/ImplicitConditionMatcher.h b/include/modules/ImplicitConditionMatcher.h index 128b1ad..9c4818c 100644 --- a/include/modules/ImplicitConditionMatcher.h +++ b/include/modules/ImplicitConditionMatcher.h @@ -15,7 +15,7 @@ namespace opov { namespace module { class ImplicitConditionMatcher : public opov::ASTMatcherModule { -public: + public: ImplicitConditionMatcher(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/ImplicitConversion.h b/include/modules/ImplicitConversion.h index f894b24..caf1f76 100644 --- a/include/modules/ImplicitConversion.h +++ b/include/modules/ImplicitConversion.h @@ -17,7 +17,7 @@ namespace opov { namespace module { class ImplicitConversion : public opov::ASTMatcherModule { -public: + public: ImplicitConversion(); virtual void setupOnce(const Configuration* config) override; virtual void setupMatcher() override; diff --git a/include/modules/UnionMatcher.h b/include/modules/UnionMatcher.h index ef812ca..b7d63bc 100644 --- a/include/modules/UnionMatcher.h +++ b/include/modules/UnionMatcher.h @@ -16,7 +16,6 @@ namespace opov { namespace module { class UnionMatcher : public opov::ASTMatcherModule { - public: UnionMatcher(); virtual void setupOnce(const Configuration* config) override; diff --git a/src/AnalyzerFactory.cpp b/src/AnalyzerFactory.cpp index 9c85935..8b6167a 100644 --- a/src/AnalyzerFactory.cpp +++ b/src/AnalyzerFactory.cpp @@ -17,11 +17,11 @@ namespace opov { -AnalyzerFactory::AnalyzerFactory(Configuration* config, IssueHandler* ihandler, TransformationHandler* thandler) +AnalyzerFactory::AnalyzerFactory(Configuration *config, IssueHandler *ihandler, TransformationHandler *thandler) : AbstractFactory(config, ihandler, thandler) { } -bool AnalyzerFactory::handleBeginSource(clang::CompilerInstance& CI, clang::StringRef Filename) { +bool AnalyzerFactory::handleBeginSource(clang::CompilerInstance &CI, clang::StringRef Filename) { AbstractFactory::handleBeginSource(CI, Filename); LOG_DEBUG("Called AnalyzerFactory beginsource: " << CI.hasASTConsumer()); return true; @@ -32,7 +32,7 @@ void AnalyzerFactory::handleEndSource() { LOG_DEBUG("Called AnalyzerFactory endsource."); } -clang::ASTConsumer* AnalyzerFactory::newASTConsumer() { +clang::ASTConsumer *AnalyzerFactory::newASTConsumer() { LOG_DEBUG("Called AnalyzerFactory: " << this); return new ModuleConsumer(module, context.get()); } diff --git a/src/ModuleConsumer.cpp b/src/ModuleConsumer.cpp index cb7ee34..d0be3a7 100644 --- a/src/ModuleConsumer.cpp +++ b/src/ModuleConsumer.cpp @@ -10,7 +10,7 @@ namespace opov { -ModuleConsumer::ModuleConsumer(Module* module, ModuleContext* mcontext) +ModuleConsumer::ModuleConsumer(Module *module, ModuleContext *mcontext) : AbstractModuleConsumer(module, mcontext) { } diff --git a/src/OpOvApp.cpp b/src/OpOvApp.cpp index 350e438..0aaa33d 100644 --- a/src/OpOvApp.cpp +++ b/src/OpOvApp.cpp @@ -24,7 +24,7 @@ #include #include -OpOvApp::OpOvApp(const std::string& config_path) +OpOvApp::OpOvApp(const std::string &config_path) : config_path(config_path) { } diff --git a/src/core/AbstractFactory.cpp b/src/core/AbstractFactory.cpp index 85c1d72..e39f83b 100644 --- a/src/core/AbstractFactory.cpp +++ b/src/core/AbstractFactory.cpp @@ -38,7 +38,7 @@ bool AbstractFactory::handleBeginSource(clang::CompilerInstance& CI, llvm::Strin } void AbstractFactory::handleEndSource() { - if(thandler->getRewriter().overwriteChangedFiles()) { + if (thandler->getRewriter().overwriteChangedFiles()) { LOG_ERROR("Error while writing source transformations to disk."); } } diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 66c7bc5..d55d144 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -32,8 +32,7 @@ namespace opov { -Application::Application() { - +Application::Application() { } void Application::init() { @@ -99,7 +98,7 @@ void Application::report() { config->getValue("global:filter", do_filter, false); TUIssuesMap& issuesMap = ihandler->getAllIssues(); - if(do_filter) { + if (do_filter) { auto filteredSet = filter->apply(issuesMap); reporter->addIssues(filteredSet); } else { diff --git a/src/core/reporting/CSVReporter.cpp b/src/core/reporting/CSVReporter.cpp index 6b495b5..fd2b3e5 100644 --- a/src/core/reporting/CSVReporter.cpp +++ b/src/core/reporting/CSVReporter.cpp @@ -28,21 +28,21 @@ void CSVReporter::addIssues(const TUIssuesMap& issues) { void CSVReporter::addIssues(const filter::IssueSet& set) { std::stringstream csv; - csv << "Module Name;File Path;Code;Line Start;Line End;Column Start;Column End;Files\n"; - for (auto& ifiltered : set) { - auto i = ifiltered.second.issue; - csv << i->getModuleName() << ";"; - csv << i->getFile() << ";"; - csv << i->getCode() << ";"; - csv << i->getLineStart() << ";" << i->getLineEnd() << ";" << i->getColumnStart() << ";" << i->getColumnEnd() << ";"; - csv << "+"; - for (auto& tunit : ifiltered.second.tunits) { - csv << tunit << "+"; - } - LOG_MSG(csv.str()); - csv.str(""); - csv.clear(); + csv << "Module Name;File Path;Code;Line Start;Line End;Column Start;Column End;Files\n"; + for (auto& ifiltered : set) { + auto i = ifiltered.second.issue; + csv << i->getModuleName() << ";"; + csv << i->getFile() << ";"; + csv << i->getCode() << ";"; + csv << i->getLineStart() << ";" << i->getLineEnd() << ";" << i->getColumnStart() << ";" << i->getColumnEnd() << ";"; + csv << "+"; + for (auto& tunit : ifiltered.second.tunits) { + csv << tunit << "+"; } + LOG_MSG(csv.str()); + csv.str(""); + csv.clear(); + } } void CSVReporter::print(const TranslationUnitIssues& issue) { diff --git a/src/core/transformation/TransformationHandler.cpp b/src/core/transformation/TransformationHandler.cpp index b95fca2..0aee432 100644 --- a/src/core/transformation/TransformationHandler.cpp +++ b/src/core/transformation/TransformationHandler.cpp @@ -46,20 +46,22 @@ void TransformationHandler::addReplacements(const clang::FixItHint& Hint) { if (Hint.CodeToInsert.empty()) { if (!Hint.InsertFromRange.isValid()) { rewriter.RemoveText(Hint.RemoveRange, nl_opt); - //rewriter.ReplaceText(Hint.RemoveRange.getAsRange(), ""); + // rewriter.ReplaceText(Hint.RemoveRange.getAsRange(), ""); } - //if (Hint.InsertFromRange.isValid()) - // commit.insertFromRange(Hint.RemoveRange.getBegin(), Hint.InsertFromRange, /*afterToken=*/false, Hint.BeforePreviousInsertions); - //else + // if (Hint.InsertFromRange.isValid()) + // commit.insertFromRange(Hint.RemoveRange.getBegin(), Hint.InsertFromRange, /*afterToken=*/false, + // Hint.BeforePreviousInsertions); + // else // commit.remove(Hint.RemoveRange); } else { - if (Hint.RemoveRange.isTokenRange() ||Hint.RemoveRange.getBegin() != Hint.RemoveRange.getEnd()) { - //commit.replace(Hint.RemoveRange, Hint.CodeToInsert); + if (Hint.RemoveRange.isTokenRange() || Hint.RemoveRange.getBegin() != Hint.RemoveRange.getEnd()) { + // commit.replace(Hint.RemoveRange, Hint.CodeToInsert); rewriter.ReplaceText(Hint.RemoveRange.getAsRange(), Hint.CodeToInsert); } else { - //commit.insert(Hint.RemoveRange.getBegin(), Hint.CodeToInsert,/*afterToken=*/false, Hint.BeforePreviousInsertions); - rewriter.InsertText(Hint.RemoveRange.getBegin(),Hint.CodeToInsert, Hint.BeforePreviousInsertions, true); - //rewriter.ReplaceText(Hint.RemoveRange.getBegin(),0, Hint.CodeToInsert); + // commit.insert(Hint.RemoveRange.getBegin(), Hint.CodeToInsert,/*afterToken=*/false, + // Hint.BeforePreviousInsertions); + rewriter.InsertText(Hint.RemoveRange.getBegin(), Hint.CodeToInsert, Hint.BeforePreviousInsertions, true); + // rewriter.ReplaceText(Hint.RemoveRange.getBegin(),0, Hint.CodeToInsert); } } /* diff --git a/src/modules/ConditionalAssgnMatcher.cpp b/src/modules/ConditionalAssgnMatcher.cpp index 3edc461..9e8349a 100644 --- a/src/modules/ConditionalAssgnMatcher.cpp +++ b/src/modules/ConditionalAssgnMatcher.cpp @@ -23,12 +23,11 @@ using namespace clang; using namespace clang::ast_matchers; ConditionalAssgnMatcher::ConditionalAssgnMatcher() -: var_counter(0) { - + : var_counter(0) { } void ConditionalAssgnMatcher::setupOnce(const Configuration* config) { - //transformer = util::make_unique(type_s); + // transformer = util::make_unique(type_s); } void ConditionalAssgnMatcher::setupMatcher() { @@ -42,22 +41,21 @@ void ConditionalAssgnMatcher::setupMatcher() { this->addMatcher(conditional); } - void ConditionalAssgnMatcher::toString(clang::ASTContext& ac, const Expr* e, conditional_data& d, int counter) { auto cond = dyn_cast(e->IgnoreParenImpCasts()); - if(cond) { + if (cond) { d.type = clutil::typeOf(e); d.variable = "_oolint_t_" + util::num2str(cond); - d.replacement = d.type + " " + d.variable + ";\n" + "condassign(" + d.variable - + ", " + clutil::node2str(ac, cond->getCond()) - + ", " + (d.lhs == "" ? clutil::node2str(ac, cond->getLHS()) : d.lhs) - + ", " + (d.rhs == "" ? clutil::node2str(ac, cond->getRHS()) : d.rhs) + ");"; + d.replacement = d.type + " " + d.variable + ";\n" + "condassign(" + d.variable + ", " + + clutil::node2str(ac, cond->getCond()) + ", " + + (d.lhs == "" ? clutil::node2str(ac, cond->getLHS()) : d.lhs) + ", " + + (d.rhs == "" ? clutil::node2str(ac, cond->getRHS()) : d.rhs) + ");"; } } -ConditionalAssgnMatcher::conditional_data ConditionalAssgnMatcher::buildReplacement(clang::ASTContext& ac, const ConditionalOperator* conditional) { -#define nl_str(STRUCT) \ - (STRUCT.replacement == "" ? "" : (STRUCT.replacement + "\n")) +ConditionalAssgnMatcher::conditional_data ConditionalAssgnMatcher::buildReplacement( + clang::ASTContext& ac, const ConditionalOperator* conditional) { +#define nl_str(STRUCT) (STRUCT.replacement == "" ? "" : (STRUCT.replacement + "\n")) conditional_data lhs_dat, rhs_dat, cond_dat; toString(ac, conditional->getRHS(), rhs_dat, ++var_counter); @@ -73,19 +71,20 @@ void ConditionalAssgnMatcher::run(const clang::ast_matchers::MatchFinder::MatchR auto* root = result.Nodes.getNodeAs("conditional_root"); auto* conditional = result.Nodes.getNodeAs("conditional"); - if(root == nullptr) { + if (root == nullptr) { // We report every conditionalOperator (even nested ones) auto& ihandle = context->getIssueHandler(); ihandle.addIssue(conditional, moduleName(), moduleDescription()); } - if(transform && root && conditional) { + if (transform && root && conditional) { // We transform starting from root down to the last conditionalOperator auto& thandle = context->getTransformationHandler(); auto& ac = context->getASTContext(); conditional_data cond_dat = buildReplacement(ac, conditional); // Delete if for some reason ?: has no assignement (root is equal to conditional pointer) - thandle.addReplacements(clang::tooling::Replacement(ac.getSourceManager(), conditional, root == conditional ? "" : cond_dat.variable)); + thandle.addReplacements( + clang::tooling::Replacement(ac.getSourceManager(), conditional, root == conditional ? "" : cond_dat.variable)); thandle.addReplacements(trutil::insertString(ac, cond_dat.replacement, root, true, "")); } } diff --git a/src/modules/ExplicitCast.cpp b/src/modules/ExplicitCast.cpp index a77dc47..f38bfae 100644 --- a/src/modules/ExplicitCast.cpp +++ b/src/modules/ExplicitCast.cpp @@ -51,7 +51,7 @@ void ExplicitCast::run(const clang::ast_matchers::MatchFinder::MatchResult& resu auto& ihandle = context->getIssueHandler(); ihandle.addIssue(ecast, moduleName(), moduleDescription()); - if(transform) { + if (transform) { auto& thandle = context->getTransformationHandler(); auto replace = trutil::reCast(context->getASTContext(), ecast, type_s, stmt_cast); thandle.addHeader(header_cast, clutil::locOf(context->getSourceManager(), ecast).getBegin()); diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index c37b5b0..ab09c92 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -26,17 +26,16 @@ void GlobalScope::setupOnce(const Configuration* config) { } void GlobalScope::setupMatcher() { - //auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), + // auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), // has(nestedNameSpecifier()))).bind("global"); - auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))) - , has(nestedNameSpecifier(isGlobalNamespace())) - ).bind("global"); + auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), + has(nestedNameSpecifier(isGlobalNamespace()))).bind("global"); this->addMatcher(declref_matcher); } void GlobalScope::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { const DeclRefExpr* call = result.Nodes.getNodeAs("global"); - //const FunctionDecl* func_decl = result.Nodes.getNodeAs("decl"); + // const FunctionDecl* func_decl = result.Nodes.getNodeAs("decl"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(call, moduleName(), moduleDescription()); diff --git a/src/modules/IfElseAssign.cpp b/src/modules/IfElseAssign.cpp index c25be50..d2d0810 100644 --- a/src/modules/IfElseAssign.cpp +++ b/src/modules/IfElseAssign.cpp @@ -23,56 +23,52 @@ using namespace clang; using namespace clang::ast_matchers; IfElseAssign::IfElseAssign() { - - } void IfElseAssign::setupOnce(const Configuration* config) { - } void IfElseAssign::setupMatcher() { - // ADOL-C speaks about all types being 'active', we should warn whenever an active type is involved (scalar). - // Caveat: We limit ourselves to conditionaloperator-like structures (as shown in ADOL-C tech paper) - // That is if or if-else with each having exactly one assignment with an active type involved - //auto assign = binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND); - //auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); +// ADOL-C speaks about all types being 'active', we should warn whenever an active type is involved (scalar). +// Caveat: We limit ourselves to conditionaloperator-like structures (as shown in ADOL-C tech paper) +// That is if or if-else with each having exactly one assignment with an active type involved +// auto assign = binaryOperator(hasOperatorName("="), hasDescendant(expr(isTypedef(type_s)))).bind(BIND); +// auto single_expr = anyOf(compoundStmt(statementCountIs(1), has(assign)), assign); #define assign_bind(BIND) \ binaryOperator(hasOperatorName("="), hasLHS(declRefExpr()), hasDescendant(expr(isTypedef(type_s)))).bind(BIND) -#define assign_expr(BIND) \ - anyOf(compoundStmt(statementCountIs(1), has(assign_bind(BIND))), assign_bind(BIND)) +#define assign_expr(BIND) anyOf(compoundStmt(statementCountIs(1), has(assign_bind(BIND))), assign_bind(BIND)) - auto conditional = ifStmt(anyOf( - allOf(hasThenStmt(assign_expr("then")), hasElseStmt(assign_expr("else"))) - , allOf(hasThenStmt(assign_expr("then")), unless(hasElseStmt(stmt()))))).bind("conditional"); + auto conditional = + ifStmt(anyOf(allOf(hasThenStmt(assign_expr("then")), hasElseStmt(assign_expr("else"))), + allOf(hasThenStmt(assign_expr("then")), unless(hasElseStmt(stmt()))))).bind("conditional"); this->addMatcher(conditional); } -std::string IfElseAssign::toString(clang::ASTContext& ac, const IfStmt* stmt, const BinaryOperator* then, const BinaryOperator* else_e) { - auto then_ref = clutil::nameOf(dyn_cast(then->getLHS()->IgnoreImpCasts())); +std::string IfElseAssign::toString(clang::ASTContext& ac, const IfStmt* stmt, const BinaryOperator* then, + const BinaryOperator* else_e) { + auto then_ref = clutil::nameOf(dyn_cast(then->getLHS()->IgnoreImpCasts())); - if(else_e && then_ref != clutil::nameOf(dyn_cast(else_e->getLHS()->IgnoreImpCasts()))) { - // only transform when in both blocks the same variable is assigned to! - return ""; - } + if (else_e && then_ref != clutil::nameOf(dyn_cast(else_e->getLHS()->IgnoreImpCasts()))) { + // only transform when in both blocks the same variable is assigned to! + return ""; + } - auto replacement = "condassign(" + then_ref - + ", " + clutil::node2str(ac, stmt->getCond()) - + ", " +clutil::node2str(ac, then->getRHS()); - if(else_e) { - replacement += ", " + clutil::node2str(ac, else_e->getRHS()); - } - replacement += ")"; - if((else_e && isa(stmt->getElse())) || isa(stmt->getThen())) { - /* - * For structures as: - * if(...) - * a = b; - * the later Replacement does not remove the semicolon of "a = b;" - */ - replacement += ";"; - } - return replacement; + auto replacement = "condassign(" + then_ref + ", " + clutil::node2str(ac, stmt->getCond()) + ", " + + clutil::node2str(ac, then->getRHS()); + if (else_e) { + replacement += ", " + clutil::node2str(ac, else_e->getRHS()); + } + replacement += ")"; + if ((else_e && isa(stmt->getElse())) || isa(stmt->getThen())) { + /* + * For structures as: + * if(...) + * a = b; + * the later Replacement does not remove the semicolon of "a = b;" + */ + replacement += ";"; + } + return replacement; } void IfElseAssign::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { @@ -83,10 +79,10 @@ void IfElseAssign::run(const clang::ast_matchers::MatchFinder::MatchResult& resu auto& ihandle = context->getIssueHandler(); ihandle.addIssue(conditional, moduleName(), moduleDescription()); - if(transform) { + if (transform) { auto& ac = context->getASTContext(); auto replacement = toString(ac, conditional, then_expr, else_expr); - if(replacement != "") { + if (replacement != "") { auto& thandle = context->getTransformationHandler(); thandle.addReplacements(clang::tooling::Replacement(context->getSourceManager(), conditional, replacement)); } @@ -102,7 +98,6 @@ std::string IfElseAssign::moduleDescription() { } IfElseAssign::~IfElseAssign() { - } } /* namespace module */ diff --git a/src/modules/ImplicitConditionMatcher.cpp b/src/modules/ImplicitConditionMatcher.cpp index 0ad3df6..80c6eb4 100644 --- a/src/modules/ImplicitConditionMatcher.cpp +++ b/src/modules/ImplicitConditionMatcher.cpp @@ -27,7 +27,6 @@ ImplicitConditionMatcher::ImplicitConditionMatcher() { } void ImplicitConditionMatcher::setupOnce(const Configuration* config) { - } void ImplicitConditionMatcher::setupMatcher() { @@ -40,8 +39,7 @@ void ImplicitConditionMatcher::setupMatcher() { /*auto typedef_condition = hasCondition(anyOf(unaryMatch, expr(hasDescendant(unaryMatch)), impl, expr(hasDescendant(impl))));*/ - auto typedef_condition = - hasCondition(anyOf(descendant_or_self(unaryMatch), descendant_or_self(impl))); + auto typedef_condition = hasCondition(anyOf(descendant_or_self(unaryMatch), descendant_or_self(impl))); StatementMatcher all_cond = stmt(anyOf(ifStmt(typedef_condition), forStmt(typedef_condition), doStmt(typedef_condition), @@ -60,7 +58,7 @@ void ImplicitConditionMatcher::run(const clang::ast_matchers::MatchFinder::Match auto& ihandle = context->getIssueHandler(); ihandle.addIssue(invalid, moduleName(), moduleDescription()); - if(transform) { + if (transform) { auto& thandle = context->getTransformationHandler(); thandle.addReplacements(trutil::addExplicitCompare(context->getASTContext(), invalid, type_s)); } diff --git a/src/modules/ImplicitConversion.cpp b/src/modules/ImplicitConversion.cpp index 0039c80..7db5cc1 100644 --- a/src/modules/ImplicitConversion.cpp +++ b/src/modules/ImplicitConversion.cpp @@ -41,7 +41,7 @@ void ImplicitConversion::run(const clang::ast_matchers::MatchFinder::MatchResult auto& ihandle = context->getIssueHandler(); ihandle.addIssue(expr, moduleName(), moduleDescription()); //, message.str()); - if(transform) { + if (transform) { auto& thandle = context->getTransformationHandler(); thandle.addReplacements(trutil::castTheExpr(context->getASTContext(), expr, type_s)); } diff --git a/src/modules/UnionMatcher.cpp b/src/modules/UnionMatcher.cpp index 6713e7b..fc1c356 100644 --- a/src/modules/UnionMatcher.cpp +++ b/src/modules/UnionMatcher.cpp @@ -28,7 +28,6 @@ UnionMatcher::UnionMatcher() { } void UnionMatcher::setupOnce(const Configuration* config) { - } void UnionMatcher::setupMatcher() { @@ -42,10 +41,7 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu const bool is_anon = inv_union->isAnonymousStructOrUnion(); std::vector fieldDecls(inv_union->field_begin(), inv_union->field_end()); auto filter_end = std::remove_if(fieldDecls.begin(), fieldDecls.end(), - [&](const FieldDecl* decl) { - return clutil::typeOf(decl) != type_s; - } - ); + [&](const FieldDecl* decl) { return clutil::typeOf(decl) != type_s; }); fieldDecls.erase(filter_end, fieldDecls.end()); std::stringstream message; @@ -55,29 +51,32 @@ void UnionMatcher::run(const clang::ast_matchers::MatchFinder::MatchResult& resu auto& ihandle = context->getIssueHandler(); ihandle.addIssue(inv_union, moduleName(), moduleDescription()); - if(transform) { + if (transform) { auto& thandle = context->getTransformationHandler(); auto& ast_ctx = context->getASTContext(); - if(is_anon) { + if (is_anon) { const int scalar_count = std::distance(fieldDecls.begin(), fieldDecls.end()); const int decl_count = clutil::declCount(inv_union); - if(decl_count == 1) { - thandle.addReplacements(FixItHint::CreateReplacement(clutil::locOf(ast_ctx, inv_union), clutil::node2str(ast_ctx, fieldDecls.front()))); + if (decl_count == 1) { + thandle.addReplacements(FixItHint::CreateReplacement(clutil::locOf(ast_ctx, inv_union), + clutil::node2str(ast_ctx, fieldDecls.front()))); } else { const auto union_end = clutil::locOf(ast_ctx, inv_union, true).getEnd(); - if((decl_count - scalar_count) < 2) { - thandle.addReplacements(FixItHint::CreateReplacement(clutil::locOf(ast_ctx, inv_union), clutil::node2str(ast_ctx, *inv_union->field_begin()))); - for(auto it = inv_union->field_begin(); it != inv_union->field_end(); ++it) { - if(it == inv_union->field_begin()) { + if ((decl_count - scalar_count) < 2) { + thandle.addReplacements(FixItHint::CreateReplacement(clutil::locOf(ast_ctx, inv_union), + clutil::node2str(ast_ctx, *inv_union->field_begin()))); + for (auto it = inv_union->field_begin(); it != inv_union->field_end(); ++it) { + if (it == inv_union->field_begin()) { continue; } - thandle.addReplacements(FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, *it) + ";", true) ); + thandle.addReplacements( + FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, *it) + ";", true)); } } else { - - for(auto fd : fieldDecls) { + for (auto fd : fieldDecls) { thandle.addReplacements(FixItHint::CreateRemoval(clutil::locOf(ast_ctx, fd, true))); - thandle.addReplacements(FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, fd) + ";", true) ); + thandle.addReplacements( + FixItHint::CreateInsertion(union_end, "\n" + clutil::node2str(ast_ctx, fd) + ";", true)); } } } From 6f7e85399734bb9447a14c399d776bc145b217bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Mon, 19 Oct 2015 09:41:09 +0200 Subject: [PATCH 20/21] Moidified the globalscope matcher to return the callexpr not the declrefexpr. --- src/modules/GlobalScope.cpp | 11 +++++------ test/collection/globalscope.cpp | 2 ++ test/src/test_globalscope.cpp | 27 ++++++++++++++++----------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/modules/GlobalScope.cpp b/src/modules/GlobalScope.cpp index ab09c92..2ce0c3f 100644 --- a/src/modules/GlobalScope.cpp +++ b/src/modules/GlobalScope.cpp @@ -26,16 +26,15 @@ void GlobalScope::setupOnce(const Configuration* config) { } void GlobalScope::setupMatcher() { - // auto declref_matcher = declRefExpr(allOf(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), - // has(nestedNameSpecifier()))).bind("global"); auto declref_matcher = declRefExpr(hasDeclaration(functionDecl(hasAnyParameter(varDecl(isTypedef(type_s))))), - has(nestedNameSpecifier(isGlobalNamespace()))).bind("global"); - this->addMatcher(declref_matcher); + has(nestedNameSpecifier(isGlobalNamespace()))); /*.bind("global_ref");*/ + auto call_matcher = callExpr(callee(expr(ignoringImpCasts(declref_matcher)))).bind("call"); + this->addMatcher(call_matcher); } void GlobalScope::run(const clang::ast_matchers::MatchFinder::MatchResult& result) { - const DeclRefExpr* call = result.Nodes.getNodeAs("global"); - // const FunctionDecl* func_decl = result.Nodes.getNodeAs("decl"); + //auto ref = result.Nodes.getNodeAs("global_ref"); + auto call = result.Nodes.getNodeAs("call"); auto& ihandle = context->getIssueHandler(); ihandle.addIssue(call, moduleName(), moduleDescription()); diff --git a/test/collection/globalscope.cpp b/test/collection/globalscope.cpp index a644250..41be206 100644 --- a/test/collection/globalscope.cpp +++ b/test/collection/globalscope.cpp @@ -30,7 +30,9 @@ void call() { ::g(::b); ::g(::b*scalar(1)); ::h(::b); + ::g(b); g(scalar(1.0)); ns::g(scalar(1.0)); + ::ns::g(scalar(1.0)); } } diff --git a/test/src/test_globalscope.cpp b/test/src/test_globalscope.cpp index ecf3ec9..74a618b 100644 --- a/test/src/test_globalscope.cpp +++ b/test/src/test_globalscope.cpp @@ -43,17 +43,17 @@ void h(double b);\n" ::a;\n\ ::b;\n" +#define NS_END \ + "}\n\ +}" + #define MAKE_CODE(stmt) PRAGMAS HEADER GLOBAL NS stmt NS_END #define CALL(ARG) MAKE_CODE("::g(" ARG ");\n") +#define CALLR(ARG) "::g(" ARG ")" #define CALLH(ARG) MAKE_CODE("::h(" ARG ");\n") -#define NS_END \ - "}\n\ -}" - - -KICKOFF_TEST(opov::module::GlobalScope, CALL("b"), "::g") +KICKOFF_TEST(opov::module::GlobalScope, CALL("b"), CALLR("b")) SCENARIO("Global scope function calls with scalars. Module looks for type scalar and one match is produced.", "[scalar_match]") { @@ -63,14 +63,19 @@ SCENARIO("Global scope function calls with scalars. Module looks for type scalar app.addModule(new opov::module::GlobalScope()); SIMPLE_TEST1("A call with scalar argument." - , CALL("b"), "::g"); + , CALL("b"), CALLR("b")); SIMPLE_TEST1("A call with binary expression." - , CALL("b*b"), "::g"); + , CALL("b*b"), CALLR("b*b")); SIMPLE_TEST1("A simple call with int argument." - , CALL("a"), "::g"); + , CALL("a"), CALLR("a")); SIMPLE_TEST1("A simple call with double argument." - , CALL("c"), "::g"); - + , CALL("c"), CALLR("c")); + SIMPLE_TEST1("A simple call with a (cast) scalar argument." + , CALL("scalar(1)"), CALLR("scalar(1)")); + SIMPLE_TEST1("A call with scalar argument using namespace qualifier." + , CALL("::b"), CALLR("::b")); + SIMPLE_TEST1("A call with double argument using namespace qualifier." + , CALL("::a"), CALLR("::a")); } } From c837ed6c99311a4ae62dd7084f4fe2a22142557b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCck?= Date: Mon, 19 Oct 2015 10:06:29 +0200 Subject: [PATCH 21/21] Updated readme. --- README.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index dc31168..be42c7c 100644 --- a/README.md +++ b/README.md @@ -16,37 +16,38 @@ The status of this software is alpha level. License ------------ -Distributed under the MIT License. For details refer to the [LICENSE file](LICENSE) +Distributed under the MIT License. For details refer to the [LICENSE](LICENSE) Motivation ------------ -Operator Overloading allows for the semantic augmentation of existing codes. +Operator overloading allows for the semantic augmentation of existing codes. The basic arithmetic type in a code is replaced by a user-defined type. *Type change* (in theory this just works): - typedef **double** scalar; -> typedef **userdef_double** scalar; However, several coding patterns are not compatible with user-defined classes -and result in compile time errors +and result in compile time errors. - Implicit Conversions - “At most one user-defined conversion [...] is implicitly applied to a single value.” – [§12.3-4, C++03 Standard] - Subset: - - Boolean Conversions: i.e., conditional statements + - Boolean Conversions, i.e., conditional statements - Unions - Incompatible with complex data classes [§9.5-1, C++03 Standard] - Explicit Conversions - Cast operation from a user-defined type to a built-in is often not possible -- etc. +- Friend functions and the Scope Resolution Operator (**::**) + - A combination of these two C++ features can cause problems in certain circumstances Goal ------------ Provide developers of (numerical) codes with static code analysis -to avoid problematic coding patterns +to avoid problematic coding patterns. For legacy numerical codes: - Flag potential problematic code locations @@ -57,15 +58,15 @@ Installation ------------ The tool is developed and tested on Linux. -For Ubuntu/Dabian, refer to the [Travis CI file](.travis.yml) for guidance. +For Ubuntu/Debian, refer to the [Travis CI file](.travis.yml) for guidance. ### Prerequisites -1. C++ Compiler with C++11 support (gcc version >= 4.8) -2. Cmake (version >=2.8) +1. C++ Compiler with C++11 support (GCC version >= 4.8) +2. cmake (version >=2.8) 3. Clang/LLVM in Version 3.5.0. Newer versions might not work due to the changing API. The build system relies on the presence of the **llvm-config**(-3.5 -3.6 -3.7) binary. - If it can't be found, set the cmake variable ${LLVM_ROOT_DIR} to point to the + If it can not be found, set the cmake variable ${LLVM_ROOT_DIR} to point to the respective bin/ folder where llvm-config is located. ### Build the OO-Lint tool @@ -73,7 +74,7 @@ For Ubuntu/Dabian, refer to the [Travis CI file](.travis.yml) for guidance. In the root folder of the project: mkdir build && cd build/ - cmake -DCMAKE_BUILD_TYPE=Release .. + cmake -DCMAKE_BUILD_TYPE=Release -DMAKE_TEST=FALSE .. make -The binary should be created in the project bin/ folder. +The binary should be created in the project folder *bin/*.