Fix warnings and improve ccache performance. (#723)

* Tweak NVHPC warning suppressions. This makes them work properly in standalone builds, when NMODL is not a submodule of CoreNEURON. * Add const version of PyAst::get_shared_ptr. Fixes NVHPC compiler warning: warning #612-D: overloaded virtual function "nmodl::ast::Ast::get_shared_ptr" is only partially overridden in class "PyAst". * Remove unused variable. Fixes compiler warning. * Tweak code so clang-format-{11,12} agree. Otherwise clang-format-12 added whitespace that clang-format-11 did not. * Call flex and bison with relative paths. This avoids #line directives and asserts containing the absolute path of the build directory, which should in turn improve ccache performance in the CI. * Fix comment and add note about ccache misses.
BlueBrain · Aug 26, 2021 · e98f281 · e98f281
1 parent cf56a24
commit e98f281
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 54 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -241,7 +241,6 @@ message(STATUS "You can now build NMODL using:")
 message(STATUS "  cmake --build . --parallel 8 [--target TARGET]")
 message(STATUS "You might want to adjust the number of parallel build jobs for your system.")
 message(STATUS "Some non-default targets you might want to build:")
-message(STATUS "Some things you can do now:")
 message(STATUS "--------------------+--------------------------------------------------------")
 message(STATUS " Target             |   Description")
 message(STATUS "--------------------+--------------------------------------------------------")

diff --git a/cmake/CompilerHelper.cmake b/cmake/CompilerHelper.cmake
@@ -39,5 +39,10 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "PGI" OR CMAKE_CXX_COMPILER_ID MATCHES "NVHPC")
     # "nvc++IkWUbMugiSgNH.s: Warning: stand-alone `data16' prefix
     # ~~~
     set(NMODL_COMPILER_WARNING_SUPPRESSIONS --diag_suppress=1,111,186,998,1098,1626)
+    # There are a few more warnings produced by the unit test infrastructure.
+    # ~~~
+    # "test/unit/visitor/constant_folder.cpp", warning #177-D: variable "..." was declared but never referenced
+    # ~~~
+    set(NMODL_TESTS_COMPILER_WARNING_SUPPRESSIONS --diag_suppress=177)
   endif()
 endif()
diff --git a/src/codegen/fast_math.hpp b/src/codegen/fast_math.hpp
@@ -133,7 +133,7 @@ static inline double vexpm1(double initial_x) {
     double px = std::floor(LOG2E * x + 0.5);
     const int32_t n = px;
 
-    const uint64_t twopnm1 = (((uint64_t)(n - 1)) + 1023) << 52;
+    const uint64_t twopnm1 = (static_cast<uint64_t>(n - 1) + 1023) << 52;
     x = 2 * (uint642dp(twopnm1) * egm1(x, px) + uint642dp(twopnm1)) - 1;
 
     if (initial_x > EXP_LIMIT)

diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in
@@ -17,8 +17,9 @@ const std::string nmodl::Version::NMODL_VERSION = "@PROJECT_VERSION@";
  * \brief Path of nrnutils.lib file
  *
  * nrnunits.lib need to be loaded at runtime. Before project is
- * installed it needs to be read from NMODL_PROJECT_SOURCE_DIR and later
- * from CMAKE_INSTALL_PREFIX.
+ * installed it needs to be read from NMODL_PROJECT_BINARY_DIR and later
+ * from CMAKE_INSTALL_PREFIX. Note that this use of NMODL_PROJECT_BINARY_DIR
+ * will cause ccache misses when the build prefix is changed.
  */
 std::vector<std::string> nmodl::NrnUnitsLib::NRNUNITSLIB_PATH =
     {"@CMAKE_INSTALL_PREFIX@/share/nmodl/nrnunits.lib", "@NMODL_PROJECT_BINARY_DIR@/share/nmodl/nrnunits.lib"};
diff --git a/src/language/templates/pybind/pyast.hpp b/src/language/templates/pybind/pyast.hpp
@@ -102,6 +102,10 @@ struct PyAst: public Ast {
         PYBIND11_OVERLOAD(std::shared_ptr<Ast>, Ast, get_shared_ptr, );
     }
 
+    std::shared_ptr<const Ast> get_shared_ptr() const override {
+        PYBIND11_OVERLOAD(std::shared_ptr<const Ast>, Ast, get_shared_ptr, );
+    }
+
     const ModToken* get_token() const override {
         PYBIND11_OVERLOAD(const ModToken*, Ast, get_token, );
     }

diff --git a/src/lexer/CMakeLists.txt b/src/lexer/CMakeLists.txt
@@ -74,101 +74,124 @@ file(MAKE_DIRECTORY ${PROJECT_BINARY_DIR}/src/parser/nmodl ${PROJECT_BINARY_DIR}
 # =============================================================================
 # Lexer & Parser commands
 # =============================================================================
-# command to generate nmodl parser
+# Command to generate nmodl parser Construct a relative path to avoid baking any absolute paths into
+# the output .cpp and .hpp files (via __FILE__ and so on), which cause ccache misses when the build
+# prefix changes (e.g. GitLab CI).
+set(NMODL_PARSER_SOURCE_DIR "${NMODL_PROJECT_SOURCE_DIR}/src/parser")
+set(NMODL_PARSER_BINARY_DIR "${NMODL_PROJECT_BINARY_DIR}/src/parser")
+file(RELATIVE_PATH NMODL_YY_FROM_PARSER_BINARY_DIR "${NMODL_PARSER_BINARY_DIR}"
+     "${NMODL_PARSER_SOURCE_DIR}/nmodl.yy")
 add_custom_command(
-  OUTPUT ${PROJECT_BINARY_DIR}/src/parser/nmodl/location.hh
-         ${PROJECT_BINARY_DIR}/src/parser/nmodl/nmodl_parser.cpp
-         ${PROJECT_BINARY_DIR}/src/parser/nmodl/nmodl_parser.hpp
-         ${PROJECT_BINARY_DIR}/src/parser/nmodl/position.hh
-         ${PROJECT_BINARY_DIR}/src/parser/nmodl/stack.hh
-  COMMAND ${BISON_EXECUTABLE} ARGS -d -o ${PROJECT_BINARY_DIR}/src/parser/nmodl/nmodl_parser.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/nmodl.yy
-  DEPENDS ${NMODL_PROJECT_SOURCE_DIR}/src/parser/nmodl.yy pyastgen
+  OUTPUT "${NMODL_PARSER_BINARY_DIR}/nmodl/location.hh"
+         "${NMODL_PARSER_BINARY_DIR}/nmodl/nmodl_parser.cpp"
+         "${NMODL_PARSER_BINARY_DIR}/nmodl/nmodl_parser.hpp"
+         "${NMODL_PARSER_BINARY_DIR}/nmodl/position.hh"
+         "${NMODL_PARSER_BINARY_DIR}/nmodl/stack.hh"
+  WORKING_DIRECTORY "${NMODL_PARSER_BINARY_DIR}"
+  COMMAND ${BISON_EXECUTABLE} ARGS -d -o nmodl/nmodl_parser.cpp "${NMODL_YY_FROM_PARSER_BINARY_DIR}"
+  DEPENDS "${NMODL_PARSER_SOURCE_DIR}/nmodl.yy" pyastgen
   COMMENT "-- NMODL : GENERATING NMODL_CORE PARSER WITH BISON! --")
 
-# command to generate verbatim parser
+# Command to generate verbatim parser. See comment above about absolute paths.
+file(RELATIVE_PATH VERBATIM_YY_FROM_PARSER_BINARY_DIR "${NMODL_PARSER_BINARY_DIR}"
+     "${NMODL_PARSER_SOURCE_DIR}/verbatim.yy")
 add_custom_command(
-  OUTPUT ${PROJECT_BINARY_DIR}/src/parser/verbatim_parser.cpp
-         ${PROJECT_BINARY_DIR}/src/parser/verbatim_parser.hpp
-  COMMAND ${BISON_EXECUTABLE} ARGS -d -o ${PROJECT_BINARY_DIR}/src/parser/verbatim_parser.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/verbatim.yy
-  DEPENDS ${NMODL_PROJECT_SOURCE_DIR}/src/parser/verbatim.yy
+  OUTPUT "${NMODL_PARSER_BINARY_DIR}/verbatim_parser.cpp"
+         "${NMODL_PARSER_BINARY_DIR}/verbatim_parser.hpp"
+  WORKING_DIRECTORY "${NMODL_PARSER_BINARY_DIR}"
+  COMMAND ${BISON_EXECUTABLE} ARGS -d -o verbatim_parser.cpp "${VERBATIM_YY_FROM_PARSER_BINARY_DIR}"
+  DEPENDS "${NMODL_PARSER_SOURCE_DIR}/verbatim.yy"
   COMMENT "-- NMODL : GENERATING VERBATIM PARSER WITH BISON! --")
 
-# command to generate differential equation parser
+# Command to generate differential equation parser. See comment above about absolute paths.
+file(RELATIVE_PATH DIFFEQ_YY_FROM_PARSER_BINARY_DIR "${NMODL_PARSER_BINARY_DIR}"
+     "${NMODL_PARSER_SOURCE_DIR}/diffeq.yy")
 add_custom_command(
-  OUTPUT ${PROJECT_BINARY_DIR}/src/parser/diffeq/diffeq_parser.cpp
-         ${PROJECT_BINARY_DIR}/src/parser/diffeq/diffeq_parser.hpp
-         ${PROJECT_BINARY_DIR}/src/parser/diffeq/stack.hh
-  COMMAND ${BISON_EXECUTABLE} ARGS -d -o ${PROJECT_BINARY_DIR}/src/parser/diffeq/diffeq_parser.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/diffeq.yy
-  DEPENDS ${NMODL_PROJECT_SOURCE_DIR}/src/parser/diffeq.yy
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/diffeq_context.hpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/diffeq_context.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/diffeq_helper.hpp
+  OUTPUT "${NMODL_PARSER_BINARY_DIR}/diffeq/diffeq_parser.cpp"
+         "${NMODL_PARSER_BINARY_DIR}/diffeq/diffeq_parser.hpp"
+         "${NMODL_PARSER_BINARY_DIR}/diffeq/stack.hh"
+  WORKING_DIRECTORY "${NMODL_PARSER_BINARY_DIR}"
+  COMMAND ${BISON_EXECUTABLE} ARGS -d -o diffeq/diffeq_parser.cpp
+          "${DIFFEQ_YY_FROM_PARSER_BINARY_DIR}"
+  DEPENDS "${NMODL_PARSER_SOURCE_DIR}/diffeq.yy" "${NMODL_PARSER_SOURCE_DIR}/diffeq_context.hpp"
+          "${NMODL_PARSER_SOURCE_DIR}/diffeq_context.cpp"
+          "${NMODL_PARSER_SOURCE_DIR}/diffeq_helper.hpp"
   COMMENT "-- NMODL : GENERATING DIFFERENTIAL EQUATION PARSER WITH BISON! --")
 
-# command to generate C (11) parser
+# Command to generate C (11) parser. See comment above about absolute paths.
+file(RELATIVE_PATH C11_YY_FROM_PARSER_BINARY_DIR "${NMODL_PARSER_BINARY_DIR}"
+     "${NMODL_PARSER_SOURCE_DIR}/c11.yy")
 add_custom_command(
-  OUTPUT ${PROJECT_BINARY_DIR}/src/parser/c/c11_parser.cpp
-         ${PROJECT_BINARY_DIR}/src/parser/c/c11_parser.hpp
-         ${PROJECT_BINARY_DIR}/src/parser/c/stack.hh
-  COMMAND ${BISON_EXECUTABLE} ARGS -d -o ${PROJECT_BINARY_DIR}/src/parser/c/c11_parser.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/c11.yy
-  DEPENDS ${NMODL_PROJECT_SOURCE_DIR}/src/parser/c11.yy
+  OUTPUT "${NMODL_PARSER_BINARY_DIR}/c/c11_parser.cpp" "${NMODL_PARSER_BINARY_DIR}/c/c11_parser.hpp"
+         "${NMODL_PARSER_BINARY_DIR}/c/stack.hh"
+  WORKING_DIRECTORY "${NMODL_PARSER_BINARY_DIR}"
+  COMMAND ${BISON_EXECUTABLE} ARGS -d -o c/c11_parser.cpp "${C11_YY_FROM_PARSER_BINARY_DIR}"
+  DEPENDS "${NMODL_PARSER_SOURCE_DIR}/c11.yy"
   COMMENT "-- NMODL : GENERATING C (11) PARSER WITH BISON! --")
 
-# command to generate Units parser
+# Command to generate Units parser. See comment above about absolute paths.
+file(RELATIVE_PATH UNIT_YY_FROM_PARSER_BINARY_DIR "${NMODL_PARSER_BINARY_DIR}"
+     "${NMODL_PARSER_SOURCE_DIR}/unit.yy")
 add_custom_command(
-  OUTPUT ${PROJECT_BINARY_DIR}/src/parser/unit/unit_parser.cpp
-         ${PROJECT_BINARY_DIR}/src/parser/unit/unit_parser.hpp
-         ${PROJECT_BINARY_DIR}/src/parser/unit/stack.hh
-  COMMAND ${BISON_EXECUTABLE} ARGS -d -o ${PROJECT_BINARY_DIR}/src/parser/unit/unit_parser.cpp
-          ${NMODL_PROJECT_SOURCE_DIR}/src/parser/unit.yy
-  DEPENDS ${NMODL_PROJECT_SOURCE_DIR}/src/parser/unit.yy
+  OUTPUT "${NMODL_PARSER_BINARY_DIR}/unit/unit_parser.cpp"
+         "${NMODL_PARSER_BINARY_DIR}/unit/unit_parser.hpp"
+         "${NMODL_PARSER_BINARY_DIR}/unit/stack.hh"
+  WORKING_DIRECTORY "${NMODL_PARSER_BINARY_DIR}"
+  COMMAND ${BISON_EXECUTABLE} ARGS -d -o unit/unit_parser.cpp "${UNIT_YY_FROM_PARSER_BINARY_DIR}"
+  DEPENDS "${NMODL_PARSER_SOURCE_DIR}/unit.yy"
   COMMENT "-- NMODL : GENERATING UNIT PARSER WITH BISON! --")
 
-# command to generate nmodl lexer
+# Command to generate nmodl lexer. See comment above about absolute paths.
+file(RELATIVE_PATH NMODL_LL_FROM_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}"
+     "${CMAKE_CURRENT_SOURCE_DIR}/nmodl.ll")
 add_custom_command(
   OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/nmodl_base_lexer.cpp
          ${CMAKE_CURRENT_BINARY_DIR}/nmodl_base_lexer.hpp
-  COMMAND ${FLEX_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/nmodl.ll
+  COMMAND ${FLEX_EXECUTABLE} ARGS "${NMODL_LL_FROM_BINARY_DIR}"
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/nmodl.ll ${CMAKE_CURRENT_SOURCE_DIR}/nmodl_utils.hpp
   COMMENT "-- NMODL : GENERATING NMODL LEXER WITH FLEX! --")
 
-# command to generate verbatim lexer
+# Command to generate verbatim lexer. See comment above about absolute paths.
+file(RELATIVE_PATH VERBATIM_LL_FROM_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}"
+     "${CMAKE_CURRENT_SOURCE_DIR}/verbatim.l")
 add_custom_command(
   OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/verbatim_lexer.cpp
          ${CMAKE_CURRENT_BINARY_DIR}/verbatim_lexer.hpp
-  COMMAND ${FLEX_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/verbatim.l
+  COMMAND ${FLEX_EXECUTABLE} ARGS "${VERBATIM_LL_FROM_BINARY_DIR}"
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/verbatim.l
   COMMENT "-- NMODL : GENERATING VERBATIM LEXER WITH FLEX! --")
 
-# command to generate differential equation lexer
+# Command to generate differential equation lexer. See comment above about absolute paths.
+file(RELATIVE_PATH DIFFEQ_LL_FROM_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}"
+     "${CMAKE_CURRENT_SOURCE_DIR}/diffeq.ll")
 add_custom_command(
   OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/diffeq_base_lexer.cpp
          ${CMAKE_CURRENT_BINARY_DIR}/diffeq_base_lexer.hpp
-  COMMAND ${FLEX_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/diffeq.ll
+  COMMAND ${FLEX_EXECUTABLE} ARGS "${DIFFEQ_LL_FROM_BINARY_DIR}"
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/diffeq.ll
   COMMENT "-- NMODL : GENERATING DIFFERENTIAL EQUATION LEXER WITH FLEX! --")
 
-# command to generate C (11) lexer
+# Command to generate C (11) lexer. See comment above about absolute paths.
+file(RELATIVE_PATH C11_LL_FROM_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}"
+     "${CMAKE_CURRENT_SOURCE_DIR}/c11.ll")
 add_custom_command(
   OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/c11_base_lexer.cpp
          ${CMAKE_CURRENT_BINARY_DIR}/c11_base_lexer.hpp
-  COMMAND ${FLEX_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/c11.ll
+  COMMAND ${FLEX_EXECUTABLE} ARGS "${C11_LL_FROM_BINARY_DIR}"
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/c11.ll
   COMMENT "-- NMODL : GENERATING C(11) LEXER WITH FLEX! --")
 
-# command to generate Units lexer
+# Command to generate Units lexer. See comment above about absolute paths.
+file(RELATIVE_PATH UNIT_LL_FROM_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}"
+     "${CMAKE_CURRENT_SOURCE_DIR}/unit.ll")
 add_custom_command(
   OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/unit_base_lexer.cpp
          ${CMAKE_CURRENT_BINARY_DIR}/unit_base_lexer.hpp
-  COMMAND ${FLEX_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/unit.ll
+  COMMAND ${FLEX_EXECUTABLE} ARGS "${UNIT_LL_FROM_BINARY_DIR}"
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/unit.ll
   COMMENT "-- NMODL : GENERATING UNIT LEXER WITH FLEX! --")

diff --git a/src/visitors/sympy_replace_solutions_visitor.cpp b/src/visitors/sympy_replace_solutions_visitor.cpp
@@ -179,7 +179,6 @@ void SympyReplaceSolutionsVisitor::try_replace_tagged_statement(
     case ReplacePolicy::VALUE: {
         const auto dependencies = statement_dependencies(get_lhs(node), get_rhs(node));
         const auto& key = dependencies.first;
-        const auto& vars = dependencies.second;
 
         if (solution_statements.is_var_assigned_here(key)) {
             logger->debug("SympyReplaceSolutionsVisitor :: marking for replacement {}",

diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
@@ -1,5 +1,7 @@
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin)
 
+add_compile_options(${NMODL_TESTS_COMPILER_WARNING_SUPPRESSIONS})
+
 include_directories(${PYBIND11_INCLUDE_DIR} ${PYTHON_INCLUDE_DIRS})
 include_directories(${NMODL_PROJECT_SOURCE_DIR}/ext/catch ${NMODL_PROJECT_SOURCE_DIR}/test)
 include_directories(${NMODL_PROJECT_SOURCE_DIR}/src/solver)