Add clang-tidy script; make the least-impact changes found by clang-tidy #1518

leekillough · 2025-12-29T09:02:05Z

This adds a clang-tidy.sh script which runs clang-tidy with a modest default set of checks, a more aggressive --picky set of checks, and a --checks option for specifying specific tests.

--fix can be used to invoke the clang-tidy automatic fixer, and it modifies files in the working tree similar to how test-includes.pl --fix automatically adds #includes.

This clang-tidy.sh is not intended for CI runs, but for periodic manual runs to detect potential issues and to modernize the SST code to modern C++ idioms.

I recommend running it with no arguments at first, inspecting the output file, and if one of the suggestions is acceptable, running it with the --fix --checks <check_name> with the check name listed in brackets near the suggestion ( [<check_name>]), running tests, and merging the changes if they are acceptable. Some of the automatic fixes might require massaging the code after it is edited, because it is not always perfect.

I found one va_arg leak (va_end() was not called), and I found a reversed name string comparison. I also saw an if ( strcmp(...) ) which clang-tidy warned about because the int result was implicitly converted to bool, but I only added != 0 since I am not sure whether the comparison was reversed or not.

A self-assignment check had to be added to decimal_fixedpoint. clang-tidy detected that self-assignment wasn't accounted for.

There were warnings about strcpy() being insecure, but in two such cases the strcpy() was preceded by a malloc() or new[] and both the allocation and strcpy() could be replaced with strdup().

I will add comments below each of these changes.

The changes are minimal, to avoid disruptions, but the script is robust enough to be used in the future for clang-tidy research into possible issues, as well as ideas for modernizing the code.

github-actions · 2025-12-29T09:02:27Z

CLANG-FORMAT TEST - FAILED (on last commit):
Run > ./scripts/clang-format-test.sh using clang-format v20 to check formatting

github-actions · 2025-12-29T09:06:11Z

CLANG-FORMAT TEST - PASSED

github-actions · 2025-12-29T09:08:53Z

CLANG-FORMAT TEST - FAILED (on last commit):
Run > ./scripts/clang-format-test.sh using clang-format v20 to check formatting

github-actions · 2025-12-29T09:08:55Z

CLANG-FORMAT TEST - FAILED (on last commit):
Run > ./scripts/clang-format-test.sh using clang-format v20 to check formatting

github-actions · 2025-12-29T09:10:18Z

CLANG-FORMAT TEST - PASSED

leekillough · 2025-12-29T09:11:33Z

src/sst/core/bootshared.cc

    fflush(stdout);

    int launch_sst = execve(real_binary_path, argv, environ);
+    free(real_binary_path);


clang-tidy complained about a possible memory leak in real_binary_path.

leekillough · 2025-12-29T09:12:56Z

src/sst/core/config.h

    {
        printf("SST-Core Version (" PACKAGE_VERSION);
-        if ( strcmp(SSTCORE_GIT_HEADSHA, PACKAGE_VERSION) ) {
+        if ( strcmp(SSTCORE_GIT_HEADSHA, PACKAGE_VERSION) != 0 ) {


clang-tidy complained about the integer result of strcmp() being converted to bool without a comparison or boolean operator like !. I added != 0 which gets rid of the warning and has the same effect, but I would double-check this logic.

leekillough · 2025-12-29T09:15:13Z

src/sst/core/configBase.cc

+    if ( columns ) {
        errno      = E_OK;
-        uint32_t x = strtoul(getenv("COLUMNS"), nullptr, 0);
+        uint32_t x = strtoul(columns, nullptr, 0);


clang-tidy complained about getenv("COLUMNS") possibly passing nullptr as the first argument of strtoul(), even though we test getenv("COLUMNS") above. By loading getenv("COLUMNS") into columns and making sure it is not nullptr before passing it to strtoul(), the warning goes away, besides it being more efficient.

leekillough · 2025-12-29T09:16:47Z

src/sst/core/configGraphOutput.cc

-
-} // namespace Core
-} // namespace SST
+} // namespace SST::Core


There were still a few places where nested namespace names were not used. clang-tidy was able to fix these automatically, although it sometimes left the old namespace closing comment around, so I had to manually remove them.

leekillough · 2025-12-29T09:17:24Z

src/sst/core/decimal_fixedpoint.h

     */
    decimal_fixedpoint& operator=(const decimal_fixedpoint& v)
    {
+        if ( &v == this ) return *this;


clang-tidy complained about decimal_fixedpoint's assignment operator not checking for self-assignment.

leekillough · 2025-12-29T09:19:36Z

src/sst/core/elemLoader.cc

    }
-
-    delete[] pathCopy;
+    free(pathCopy);


clang-tidy complained about strcpy() being inherently insecure. Since here we are simply allocating and copying a string, strdup() is simpler, and removes the warning.

sst-autotester · 2025-12-29T09:20:07Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

leekillough · 2025-12-29T09:20:20Z

src/sst/core/env/envquery.cc

    if ( nullptr != envConfigPaths ) {
-        char* envConfigPathBuffer = (char*)malloc(sizeof(char) * (strlen(envConfigPaths) + 1));
-        strcpy(envConfigPathBuffer, envConfigPaths);
+        char* envConfigPathBuffer = strdup(envConfigPaths);


Here again, but here, the old code used malloc()/free() instead of new[]/delete[].

leekillough · 2025-12-29T09:21:31Z

src/sst/core/impl/partitioners/simplepart.cc

 #include "sst/core/model/configGraph.h"
 #include "sst/core/warnmacros.h"

+#include <cstdlib>


There are numerous places where C-style headers were replaced with C++ headers ( <stdlib.h> -> <cstdlib> etc.).

leekillough · 2025-12-29T09:23:09Z

src/sst/core/model/element_python.h

 namespace SST {

-using genPythonModuleFunction = void* (*)(void);
+using genPythonModuleFunction = void* (*)();


(void) function parameter lists are the same as () in C++

sst-autotester · 2026-01-07T06:19:28Z

Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge

sst-autotester · 2026-01-07T10:19:42Z

Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge

sst-autotester · 2026-01-07T14:49:38Z

Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge

sst-autotester · 2026-01-07T19:19:36Z

Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge

berquist

This review is for the script only, and not the actual clang-tidy checks used.

scripts/clang-tidy.sh

sst-autotester · 2026-01-07T21:49:33Z

Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS BEEN REVIEWED, BUT NOT ACCEPTED OR REQUIRES CHANGES!

sst-autotester · 2026-01-07T21:49:40Z

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

github-actions · 2026-01-07T22:48:45Z

CLANG-FORMAT TEST - PASSED

sst-autotester · 2026-01-07T22:49:23Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

…er make clean

github-actions · 2026-01-07T23:53:42Z

CLANG-FORMAT TEST - PASSED

sst-autotester · 2026-01-08T20:23:11Z

Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; this inspection will remain valid until a new commit to source branch is performed.

sst-autotester · 2026-01-08T20:26:47Z

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements

Build Num: 2433
Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MR-2

Build Num: 2383
Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MT-2

Build Num: 2382
Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-core_Make-Dist

Build Num: 1002
Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-15-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements

Build Num: 933
Status: STARTED

Using Repos:

Repo: CORE (leekillough/sst-core)

Repo: SQE (sstsimulator/sst-sqe)

Repo: ELEMENTS (sstsimulator/sst-elements)

Repo: MACRO (sstsimulator/sst-macro)

Pull Request Author: leekillough

sst-autotester · 2026-01-08T21:34:24Z

Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED

Pull Request Auto Testing has PASSED (click to expand)

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements

Build Num: 2433
Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MR-2

Build Num: 2383
Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MT-2

Build Num: 2382
Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-core_Make-Dist

Build Num: 1002
Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-15-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements

Build Num: 933
Status: PASSED

sst-autotester · 2026-01-08T21:34:41Z

Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS NOT BEEN REVIEWED YET!

sst-autotester · 2026-01-08T21:34:48Z

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

sst-autotester · 2026-01-08T21:45:57Z

Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ feldergast ]!

sst-autotester · 2026-01-08T21:46:03Z

Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge

Items addressed. Can revisit at a future date if anything else comes up.

github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT FAIL labels Dec 29, 2025

leekillough closed this Dec 29, 2025

leekillough force-pushed the clang-tidy branch from fb8c802 to 2cc5467 Compare December 29, 2025 09:05

github-actions bot added AT: CLANG-FORMAT PASS and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT FAIL labels Dec 29, 2025

leekillough reopened this Dec 29, 2025

github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT FAIL and removed AT: CLANG-FORMAT PASS labels Dec 29, 2025

leekillough force-pushed the clang-tidy branch from bdaca56 to 83320b0 Compare December 29, 2025 09:09

github-actions bot added AT: CLANG-FORMAT PASS and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT FAIL labels Dec 29, 2025

leekillough commented Dec 29, 2025

View reviewed changes

berquist previously requested changes Jan 7, 2026

View reviewed changes

scripts/clang-tidy.sh Show resolved Hide resolved

scripts/clang-tidy.sh Outdated Show resolved Hide resolved

scripts/clang-tidy.sh Show resolved Hide resolved

scripts/clang-tidy.sh Show resolved Hide resolved

leekillough dismissed feldergast’s stale review via 6535fca January 7, 2026 22:48

github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) labels Jan 7, 2026

Unset MAKEFLAGS to prevent parallel builds; remove src/ directory aft…

95a1b12

…er make clean

leekillough force-pushed the clang-tidy branch from 6535fca to 95a1b12 Compare January 7, 2026 23:53

github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) labels Jan 7, 2026

feldergast added the AT: PRE-TEST INSPECTED label Jan 8, 2026

sst-autotester removed the AT: PRE-TEST INSPECTED label Jan 8, 2026

feldergast approved these changes Jan 8, 2026

View reviewed changes

feldergast merged commit 1973d80 into sstsimulator:devel Jan 8, 2026
10 checks passed

leekillough deleted the clang-tidy branch January 20, 2026 15:40

Add clang-tidy script; make the least-impact changes found by clang-tidy #1518

Add clang-tidy script; make the least-impact changes found by clang-tidy #1518

Uh oh!

Conversation

leekillough commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sst-autotester commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

berquist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 7, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

sst-autotester commented Jan 8, 2026

Uh oh!

sst-autotester commented Jan 8, 2026

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MR-2

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MT-2

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-core_Make-Dist

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-15-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements

Using Repos:

Uh oh!

sst-autotester commented Jan 8, 2026

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements

leekillough commented Dec 29, 2025 •

edited

Loading