Run some code checks

[chapman39]

Running basic code checks to ensure quality

• cppcheck
• run-clang-tidy

cppcheck

didn't catch too much

clang-tidy

how i ran clang-tidy

module load clang/14.0.6
module load python
run-clang-tidy -p <build-dir>

[ignored] warning related to logger

• i think this can be safely ignored. from axom documentation:

Slic maintains ownership of all registered Log Stream instances and will deallocate them when slic::finalize() is called.

/usr/WS2/meemee/serac/repo/src/serac/infrastructure/logger.cpp:67:3: warning: Potential leak of memory pointed to by 'i_logstream' [clang-analyzer-cplusplus.NewDeleteLeaks]
   67 |   addStreamToMsgLevel(d_logstream, slic::message::Debug);
      |   ^

[solved] warning related to stateNames()

• Clang-tidy didn't like when virtual functions are called in the constructor. I moved the block that calls stateNames() into resetStates() instead (see PR diff, also simplified godbolt example showcasing warning https://godbolt.org/z/P391j9YP3)

[ignored] memory leak warning related to generateParFiniteElementSpace()

• This warning is about this function, generateParFiniteElementSpace, which creates a mfem::FiniteElementCollection (fec) and a mfem::ParFiniteElementSpace (fes). Fes takes a raw pointer of fec in its constructor.
• I think the warning is about the the case if fec gets deleted before fes, fes will have a dangling pointer. Or, clang-tidy is simply unsure whether the raw pointer fec will be properly deleted in MFEM.
• Similar to the logger warning, this can also probably be safely ignored assuming MFEM manages their raw pointers properly.
• We could use shared_ptr instead of unique_ptr, which would likely resolve the issue but could have greater implications.
• Adding // NOLINT is kind of unreasonable, since it would have to be added to every time generateParFiniteElementSpace is called (it cannot be added to functions - it can only be added to lines of code that triggers the warning).

/usr/WS2/meemee/serac/repo/src/serac/physics/boundary_conditions/tests/boundary_cond.cpp:109:61: warning: Potential leak of memory pointed to by '.first._M_t._M_t._M_head_impl' [clang-analyzer-cplusplus.NewDeleteLeaks]
  serac::mfem_ext::GetEssentialTrueDofsFromElementAttribute(*l2_fes, elem_attr_is_ess, ess_tdof_list);
                                                            ^
1m/usr/WS2/meemee/serac/repo/src/serac/physics/boundary_conditions/tests/boundary_cond.cpp:108:27: note: Calling 'generateParFiniteElementSpace<serac::L2<0, 1>>'
  auto [l2_fes, l2_fec] = serac::generateParFiniteElementSpace<L2<0>>(&pmesh);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[chapman39]

Clang-tidy complained this was returning -1, because it figured out the return value was being used as an index. I can see we'd still want to fail in this case, so I decided to explicitly print an error instead.

[chapman39]

Removes virtual function call in constructor. Warning was the following:

Call to virtual method 'Thermal::stateNames' during construction bypasses virtual dispatch [clang-analyzer-optin.cplusplus.VirtualCall]

[chapman39]

Added resetting of checkpoint states to completeSetup() to fix thermomech_finite_diff segfault. I think this was supposed to be here anyway.

[white238]

/style

[chapman39]

@tupek2 let me know if removing this var was the right call