Used clang-tidy to clean up parts of the sst codebase.

sstsimulator · Jan 13, 2020 · 15588df · 15588df
1 parent 7e38f87
commit 15588df
Show file tree

Hide file tree

Showing 34 changed files with 144 additions and 8,917 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,16 @@
+Checks: '-*,clang-diagnostic-*,misc-*,-misc-non-private-member-variables-in-classes,-misc-unused-parameters,readability-identifier-naming,readability-braces-around-statements,readability-redundant-string-cstr,readability-redundant-string-init,readability-misleading-indentation,readability-inconsistent-declaration-parameter-name,performance-*,modernize-use-override,modernize-use-nullptr,bugprone-*'
+CheckOptions:
+  - key:             readability-identifier-naming.ClassCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.ClassMethodCase
+    value:           camelBack
+  - key:             readability-identifier-naming.EnumCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.FunctionCase
+    value:           camelBack
+  - key:             readability-identifier-naming.MemberSuffix
+    value:           _
+  - key:             readability-identifier-naming.UnionCase
+    value:           CamelCase
+  - key:             readability-braces-around-statements
+    value:           0
diff --git a/Makefile.am b/Makefile.am
@@ -21,7 +21,7 @@ EXTRA_DIST = LICENSE bootstrap.sh \
   skeletons \
 	configurations
 
-SUBDIRS = $(subdirs) sumi-mpi sprockit sumi sstmac bin python tests configurations 
+SUBDIRS = $(subdirs) include sumi-mpi sprockit sumi sstmac bin python tests configurations 
 
 dist_bin_SCRIPTS = sstmacro-libtool
 

diff --git a/Makefile.common b/Makefile.common
@@ -1,4 +1,4 @@
-AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_srcdir)/sprockit -I$(top_builddir)/sprockit
+AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_srcdir)/include -I$(top_srcdir)/sprockit -I$(top_builddir)/sprockit
 AM_LDFLAGS = 
 AM_CXXFLAGS = $(STD_CXXFLAGS)
 AM_CFLAGS =

diff --git a/bin/clang/astVisitor.cc b/bin/clang/astVisitor.cc
@@ -588,21 +588,12 @@ SkeletonASTVisitor::replaceNullVariableConnectedContext(Expr* expr, const std::s
     auto& pair = binOps_[activeBinOpIdx_];
     BinaryOperator* binOp = pair.first;
     BinOpSide side = pair.second;
-    Expr* toDel = nullptr;
-    if (binOp->getOpcode() == BO_Assign){
-      if (side == LHS){
-        //replace the whole thing - don't ever assign to null value
-        toDel = binOp;
-      } else if (!repl.empty()){
-        //it is the right hand side that needs replacing and I have a repl
-        //this should have happened already - propagateNullness(binOp, nd);
+
+    Expr* toDel = binOp;
+    if(binOp->getOpcode() == BO_Assign && side != LHS && !repl.empty()){
         toDel = getUnderlyingExpr(binOp->getRHS());
-      } else {
-        toDel = binOp;
-      }
-    } else {
-      toDel = binOp;
     }
+
     ::replace(toDel, rewriter_, repl, *ci_);
     //pass the delete up to the owner
     throw StmtDeleteException(toDel);
@@ -699,7 +690,6 @@ SkeletonASTVisitor::visitNullVariable(Expr* expr, NamedDecl* nd)
     nullDereferenceError(expr, nd->getNameAsString());
   }
 
-  bool nullSafeFunctionCall = false;
   if (!nullVarPrg->deleteAll() && haveActiveFxnParam()){
     //we have "IPA" to deal with here
     //a null variable MUST map into a null variable - otherwise this is an error
@@ -727,7 +717,6 @@ SkeletonASTVisitor::visitNullVariable(Expr* expr, NamedDecl* nd)
         errorAbort(expr, *ci_, sstr.str());
       }
     }
-    nullSafeFunctionCall = true;
   }
 
   bool hasRepl = nullVarPrg->hasReplacement();
@@ -1503,7 +1492,6 @@ SkeletonASTVisitor::TraverseLambdaExpr(LambdaExpr* expr)
 bool
 SkeletonASTVisitor::doTraverseLambda(LambdaExpr* expr)
 {
-  LambdaCaptureDefault def = expr->getCaptureDefault();
   switch (expr->getCaptureDefault()){
     case LCD_None: {
       EmplaceGuard<std::set<const clang::Decl*>> eg(globalsTouched_);
@@ -1650,10 +1638,6 @@ SkeletonASTVisitor::deleteStmt(Stmt *s)
   ::replace(s, rewriter_, "", *ci_);
 }
 
-static bool isCombinedDecl(VarDecl* vD, RecordDecl* rD)
-{
-  return getStart(vD) <= getStart(rD) && getEnd(rD) <= getEnd(vD);
-}
 
 NamespaceDecl*
 SkeletonASTVisitor::getOuterNamespace(Decl *D)
@@ -1741,12 +1725,12 @@ std::string
 SkeletonASTVisitor::getCleanName(const std::string& name)
 {
   auto pos = name.find("struct ");
-  if (pos != std::string::npos && pos == 0){
+  if (pos == 0){
     return name.substr(7);
   }
 
   pos = name.find("class ");
-  if (pos != std::string::npos && pos == 0){
+  if (pos == 0){
     return name.substr(6);
   }
 
@@ -2167,9 +2151,8 @@ SkeletonASTVisitor::TraverseFunctionDecl(clang::FunctionDecl* D)
 {
   if (D->isMain() && opts_.refactorMain){
     replaceMain(D);
-  } else if (D->isTemplateInstantiation()){
-    return true; //do not visit implicitly instantiated template stuff
-  } else if (!D->isThisDeclarationADefinition()){
+  } else if (D->isTemplateInstantiation()   
+           || !D->isThisDeclarationADefinition()){
     return true;
   }
 

diff --git a/bin/clang/astVisitor.h b/bin/clang/astVisitor.h
@@ -1189,8 +1189,7 @@ class GlobalVariableVisitor : public clang::RecursiveASTVisitor<GlobalVariableVi
  public:
   GlobalVariableVisitor(clang::VarDecl* D, SkeletonASTVisitor* parent) :
     visitedGlobals_(false),
-    parent_(parent), 
-    decl_(D) 
+    parent_(parent)
   {
   }
 
@@ -1205,7 +1204,6 @@ class GlobalVariableVisitor : public clang::RecursiveASTVisitor<GlobalVariableVi
  private:
   bool visitedGlobals_;
   SkeletonASTVisitor* parent_;
-  clang::VarDecl* decl_;
 };
 
 

diff --git a/bin/clang/astVisitorGlobalVars.cc b/bin/clang/astVisitorGlobalVars.cc
@@ -24,7 +24,7 @@ static std::string
 getFxnTypedef(clang::SourceLocation loc, const Type* ty, const std::string& name, clang::CompilerInstance* ci){
   std::string typeName = GetAsString(ty);
   //this is horrible... but the only way I know
-  auto pos = typeName.find(")");
+  auto pos = typeName.find(')');
   if (pos == std::string::npos){
     internalError(loc, *ci, "failed typedef on " + typeName);
   }
@@ -256,7 +256,6 @@ SkeletonASTVisitor::setupClassStaticVarDecl(VarDecl* D)
 
     registerGlobalReplacement(D, &repl);
   } else {
-    CXXRecordDecl* outerCls = classContexts_.front();
     std::stringstream varname_scope_sstr; varname_scope_sstr << "_";
     std::stringstream cls_scope_sstr;
     for (CXXRecordDecl* decl : classContexts_){

diff --git a/bin/clang/clangHeaders.h b/bin/clang/clangHeaders.h
@@ -115,6 +115,9 @@ static inline std::string GetAsString(clang::QualType qty){
 }
 
 static inline clang::SourceLocation getStart(const clang::Stmt* s){
+  if(s == nullptr){
+    return clang::SourceLocation();
+  }
 #if CLANG_VERSION_MAJOR >= 8
   return s->getBeginLoc();
 #else

diff --git a/bin/clang/computeVisitor.h b/bin/clang/computeVisitor.h
@@ -59,7 +59,6 @@ class ComputeVisitor  {
     currentGeneration(1), 
     CI(c), 
     pragmas(plist), 
-    parent(par),
     context(ctxt)
   {}
 
@@ -88,7 +87,6 @@ class ComputeVisitor  {
   SSTPragmaList& pragmas;
   Replacements repls;
   clang::SourceLocation scopeStartLine;
-  ComputeVisitor* parent;
   SkeletonASTVisitor* context;
 
   Variable& getVariable(clang::NamedDecl* decl){

diff --git a/bin/clang/frontendActions.cc b/bin/clang/frontendActions.cc
@@ -182,7 +182,7 @@ ReplaceAction::EndSourceFileAction()
   SourceManager &SM = rewriter_.getSourceMgr();
   std::string sourceFile = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
   std::string sstSourceFile, sstGlobalFile;
-  std::size_t lastSlashPos = sourceFile.find_last_of("/");
+  std::size_t lastSlashPos = sourceFile.find_last_of('/');
   if (lastSlashPos == std::string::npos){
     sstSourceFile = "sst." + sourceFile;
     sstGlobalFile = "sstGlobals." + sourceFile + ".cpp";

diff --git a/bin/clang/pragmas.cc b/bin/clang/pragmas.cc
@@ -59,34 +59,7 @@ using namespace pragmas;
 SourceLocation SSTPragmaHandler::pragmaDirectiveLoc;
 std::map<std::string, SSTPragmaNamespace*>* PragmaRegisterMap::namespaces_ = nullptr;
 
-static std::string getStringToken(const Token& next, clang::CompilerInstance& ci)
-{
-  //the next token should be a string naming the argument
-  std::string argName;
-  switch (next.getKind()){
-    case tok::string_literal:
-      argName = next.getLiteralData();
-      break;
-    case tok::identifier:
-      argName = next.getIdentifierInfo()->getName().str();
-      break;
-    default: {
-      std::string error = std::string("invalid pragma token of type ") + next.getName()
-         + " - expected string literal argument name";
-      errorAbort(next.getLocation(), ci, error);
-    }
-  }
-  return argName;
-}
 
-static void assertToken(const Token& tok, tok::TokenKind kind, clang::CompilerInstance& ci)
-{
-  if (tok.getKind() != kind){
-    std::string error = std::string("invalid pragma token of type ") + tok.getName()
-       + " - expected " + tok::getTokenName(kind);
-    errorAbort(tok.getLocation(), ci, error);
-  }
-}
 
 void getLiteralDataAsString(const Token &tok, std::ostream &os)
 {
@@ -333,6 +306,7 @@ SSTPragma::getMap(SourceLocation loc, CompilerInstance& CI, const std::list<clan
         argList.push_back(sstr.str());
         sstr.str("");
         allArgs[argName] = std::move(argList);
+        argList = {};
       } else {
         tokenToString(t, sstr, CI);
       }

diff --git a/bin/clang/pragmas.h b/bin/clang/pragmas.h
@@ -458,7 +458,7 @@ struct PragmaRegisterMap {
 
 struct SSTPragmaNamespace {
 
-  SSTPragmaNamespace(const std::string name) :
+  SSTPragmaNamespace(const std::string &name) :
     name_(name)
   {
   }
@@ -501,8 +501,8 @@ struct PragmaRegister {
  * Use as fill-in for pragmas which should not be activated in a given mode
  */
 struct SSTDoNothingPragma : public SSTPragma {
-  void activate(clang::Stmt* s, clang::Rewriter& r, PragmaConfig& cfg){}
-  void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg){}
+  void activate(clang::Stmt* s, clang::Rewriter& r, PragmaConfig& cfg) override {}
+  void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg) override {}
 };
 
 class SSTReturnPragma : public SSTPragma {
@@ -633,7 +633,7 @@ class SSTNullVariablePragma : public SSTPragma {
 
   void doActivate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg);
 
-  virtual void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg) override;
+  void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg) override;
   void activate(clang::Stmt* s, clang::Rewriter& r, PragmaConfig& cfg) override;
 
   clang::NamedDecl* declAppliedTo_;
@@ -736,9 +736,9 @@ class SSTKeepPragma : public SSTPragma {
  public:
   SSTKeepPragma(){}
  protected:
-  virtual void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg) override;
+  void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg) override;
 
-  virtual void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg) override;
+  void activate(clang::Decl* d, clang::Rewriter& r, PragmaConfig& cfg) override;
 
   void deactivate(PragmaConfig &cfg) override {
     cfg.makeNoChanges = false;
@@ -751,7 +751,7 @@ class SSTKeepIfPragma : public SSTPragma {
     : ifCond_(ifCond)
   {}
  private:
-  void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg);
+  void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg) override;
   std::string ifCond_;
 };
 
@@ -789,7 +789,7 @@ class SSTAdvanceTimePragma : public SSTPragma {
                        const std::list<clang::Token>& tokens);
 
  private:
-  void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg);
+  void activate(clang::Stmt *s, clang::Rewriter &r, PragmaConfig &cfg) override;
   std::string units_;
   std::string amount_;
 };

diff --git a/bin/clang/recurseAll.h b/bin/clang/recurseAll.h
@@ -70,31 +70,6 @@ enum class ExprRole {
   ReturnValue
 };
 
-static const char* tostr(ExprRole role)
-{
-#define enum_case(x) case ExprRole::x: return #x
-  switch(role){
-  enum_case(ArrayBase);
-  enum_case(ArrayIdx);
-  enum_case(CallFxn);
-  enum_case(CallArg);
-  enum_case(BinOpLHS);
-  enum_case(BinOpRHS);
-  enum_case(ForInit);
-  enum_case(ForCond);
-  enum_case(ForInc);
-  enum_case(ForBody);
-  enum_case(SubExpr);
-  enum_case(IfCond);
-  enum_case(IfBody);
-  enum_case(IfElse);
-  enum_case(IfInit);
-  enum_case(Standalone);
-  enum_case(ThisPtr);
-  enum_case(ReturnValue);
-  }
-#undef enum_case
-}
 
 namespace pvt {
 

diff --git a/bin/clang/util.h b/bin/clang/util.h
@@ -46,6 +46,8 @@ Questions? Contact [email protected]
 #define bin_clang_util_h
 
 #include "clangHeaders.h"
+#include <clangtidymacros.h>
+
 #include <iostream>
 
 struct PrettyPrinter
@@ -95,19 +97,27 @@ struct PrettyPrinter
 bool isCxx(const std::string& filename);
 bool isValidSrc(const std::string& filename);
 
+CLANG_ANALYZER_NO_RETURN
 void internalError(const std::string& error);
 
+CLANG_ANALYZER_NO_RETURN
 void internalError(clang::SourceLocation loc, clang::CompilerInstance& CI, const std::string& error);
+CLANG_ANALYZER_NO_RETURN
 void errorAbort(clang::SourceLocation loc, clang::CompilerInstance& CI, const std::string& error);
+
 void warn(clang::SourceLocation loc, clang::CompilerInstance& CI, const std::string& warning);
 
 void warn(const clang::Stmt* s, clang::CompilerInstance& CI, const std::string& error);
 void warn(const clang::Decl* decl, clang::CompilerInstance& CI, const std::string& error);
 
+CLANG_ANALYZER_NO_RETURN
 void errorAbort(const clang::Stmt* s, clang::CompilerInstance& CI, const std::string& error);
+CLANG_ANALYZER_NO_RETURN
 void errorAbort(const clang::Decl* decl, clang::CompilerInstance& CI, const std::string& error);
 
+CLANG_ANALYZER_NO_RETURN
 void internalError(const clang::Stmt* s, clang::CompilerInstance& CI, const std::string& error);
+CLANG_ANALYZER_NO_RETURN
 void internalError(const clang::Decl* decl, clang::CompilerInstance& CI, const std::string& error);
 
 void insertBefore(const clang::Stmt* s, clang::Rewriter& r, const std::string& text);

diff --git a/configure.ac b/configure.ac
@@ -249,6 +249,7 @@ AC_CONFIG_FILES([
  sprockit/sprockit/Makefile
  sprockit/test/Makefile
  bin/Makefile
+ include/Makefile
  python/Makefile
  configurations/Makefile
  tests/Makefile

diff --git a/include/Makefile.am b/include/Makefile.am
@@ -0,0 +1,13 @@
+#
+#   This file is part of SST/macroscale: 
+#                The macroscale architecture simulator from the SST suite.
+#   Copyright (c) 2009-2018 NTESS.
+#   This software is distributed under the BSD License.
+#   Under the terms of Contract DE-NA0003525 with NTESS,
+#   the U.S. Government retains certain rights in this software.
+#   For more information, see the LICENSE file in the top 
+#   SST/macroscale directory.
+#
+
+library_includedir=$(includedir)
+nobase_library_include_HEADERS = clangtidymacros.h