Merge pull request #435 from borglab/fix/clang-warnings

Fix clang warnings

cmake/GtsamBuildTypes.cmake

@@ -104,7 +104,14 @@ if(MSVC)
   set(GTSAM_COMPILE_OPTIONS_PRIVATE_TIMING          /MD /O2  CACHE STRING "(User editable) Private compiler flags for Timing configuration.")
 else()
   # Common to all configurations, next for each configuration:
-  
+
+  if (
+      ((CMAKE_CXX_COMPILER_ID MATCHES "Clang") AND (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 12.0.0)) OR
+      (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+     )
+    set(flag_override_ -Wsuggest-override) # -Werror=suggest-override: Add again someday
+  endif()
+
   set(GTSAM_COMPILE_OPTIONS_PRIVATE_COMMON
     -Wall                                          # Enable common warnings
     -fPIC                                          # ensure proper code generation for shared libraries
@@ -112,7 +119,7 @@ else()
     $<$<CXX_COMPILER_ID:Clang>:-Wreturn-stack-address   -Werror=return-stack-address>  # Error: return local address
     -Wreturn-type  -Werror=return-type             # Error on missing return()
     -Wformat -Werror=format-security               # Error on wrong printf() arguments
-    $<$<COMPILE_LANGUAGE:CXX>:-Wsuggest-override -Werror=suggest-override>      # Enforce the use of the override keyword
+    $<$<COMPILE_LANGUAGE:CXX>:${flag_override_}>   # Enforce the use of the override keyword
     #
     CACHE STRING "(User editable) Private compiler flags for all configurations.")
   set(GTSAM_COMPILE_OPTIONS_PRIVATE_DEBUG           -g -fno-inline  CACHE STRING "(User editable) Private compiler flags for Debug configuration.")

gtsam/base/timing.cpp

@@ -89,7 +89,7 @@ void TimingOutline::print(const std::string& outline) const {
     childOrder[child.second->myOrder_] = child.second;
   }
   // Print children
-  for(const ChildOrder::value_type order_child: childOrder) {
+  for(const ChildOrder::value_type& order_child: childOrder) {
     std::string childOutline(outline);
     childOutline += "|   ";
     order_child.second->print(childOutline);

gtsam/discrete/DecisionTreeFactor.cpp

@@ -69,7 +69,7 @@ namespace gtsam {
     for(Key j: f.keys()) cs[j] = f.cardinality(j);
     // Convert map into keys
     DiscreteKeys keys;
-    for(const DiscreteKey& key: cs)
+    for(const std::pair<const Key,size_t>& key: cs)
       keys.push_back(key);
     // apply operand
     ADT result = ADT::apply(f, op);

gtsam/discrete/DiscreteBayesTree.h

@@ -45,6 +45,7 @@ class GTSAM_EXPORT DiscreteBayesTreeClique
   typedef boost::shared_ptr<This> shared_ptr;
   typedef boost::weak_ptr<This> weak_ptr;
   DiscreteBayesTreeClique() {}
+  virtual ~DiscreteBayesTreeClique() {}
   DiscreteBayesTreeClique(
       const boost::shared_ptr<DiscreteConditional>& conditional)
       : Base(conditional) {}

gtsam/discrete/Potentials.cpp

@@ -56,7 +56,7 @@ bool Potentials::equals(const Potentials& other, double tol) const {
 /* ************************************************************************* */
 void Potentials::print(const string& s, const KeyFormatter& formatter) const {
   cout << s << "\n  Cardinalities: {";
-  for (const DiscreteKey& key : cardinalities_)
+  for (const std::pair<const Key,size_t>& key : cardinalities_)
     cout << formatter(key.first) << ":" << key.second << ", ";
   cout << "}" << endl;
   ADT::print(" ");

gtsam/linear/GaussianBayesTree.h

@@ -41,6 +41,7 @@ namespace gtsam {
     typedef boost::shared_ptr<This> shared_ptr;
     typedef boost::weak_ptr<This> weak_ptr;
     GaussianBayesTreeClique() {}
+    virtual ~GaussianBayesTreeClique() {}
     GaussianBayesTreeClique(const boost::shared_ptr<GaussianConditional>& conditional) : Base(conditional) {}
   };
 

gtsam/linear/Preconditioner.cpp

@@ -145,7 +145,7 @@ void BlockJacobiPreconditioner::build(
 
   /* getting the block diagonals over the factors */
   std::map<Key, Matrix> hessianMap =gfg.hessianBlockDiagonal();
-  for ( const Matrix hessian: hessianMap | boost::adaptors::map_values)
+  for (const Matrix& hessian: hessianMap | boost::adaptors::map_values)
     blocks.push_back(hessian);
 
   /* if necessary, allocating the memory for cacheing the factorization results */

gtsam/linear/VectorValues.cpp

@@ -45,7 +45,7 @@ namespace gtsam {
 
   /* ************************************************************************* */
   VectorValues::VectorValues(const Vector& x, const Dims& dims) {
-    typedef pair<Key, size_t> Pair;
+    using Pair = pair<const Key, size_t>;
     size_t j = 0;
     for (const Pair& v : dims) {
       Key key;

gtsam/nonlinear/ISAM2Clique.h

@@ -51,6 +51,7 @@ class GTSAM_EXPORT ISAM2Clique
 
   /// Default constructor
   ISAM2Clique() : Base() {}
+  virtual ~ISAM2Clique() = default;
 
   /// Copy constructor, does *not* copy solution pointers as these are invalid
   /// in different trees.

gtsam/slam/dataset.cpp

@@ -73,8 +73,8 @@ string findExampleDataFile(const string& name) {
   namesToSearch.push_back(name + ".xml");
 
   // Find first name that exists
-  for(const fs::path& root: rootsToSearch) {
-    for(const fs::path& name: namesToSearch) {
+  for(const fs::path root: rootsToSearch) {
+    for(const fs::path name: namesToSearch) {
       if (fs::is_regular_file(root / name))
         return (root / name).string();
     }

gtsam/symbolic/SymbolicBayesTree.h

@@ -39,6 +39,7 @@ namespace gtsam {
     typedef boost::shared_ptr<This> shared_ptr;
     typedef boost::weak_ptr<This> weak_ptr;
     SymbolicBayesTreeClique() {}
+    virtual ~SymbolicBayesTreeClique() {}
     SymbolicBayesTreeClique(const boost::shared_ptr<SymbolicConditional>& conditional) : Base(conditional) {}
   };
 

wrap/Module.cpp

@@ -248,7 +248,7 @@ void Module::parseMarkup(const std::string& data) {
   // Create type attributes table and check validity
   typeAttributes.addClasses(expandedClasses);
   typeAttributes.addForwardDeclarations(forward_declarations);
-  for (const TypedefPair p: typedefs)
+  for (const TypedefPair& p: typedefs)
     typeAttributes.addType(p.newType);
   // add Eigen types as template arguments are also checked ?
   vector<ForwardDeclaration> eigen;