Fix `cppcheck` errors: final follow-up by dareumnam · Pull Request #11584 · NatLabRockies/EnergyPlus

dareumnam · 2026-05-08T14:34:02Z

Pull request overview

After the cppcheck_final branch got merged, there are still a number of cppcheck issues remaining. More than half of them are false positives, but this PR aims to double-check and make sure all actionable ones are addressed.

60 checkLevelNormal errors: cppcheck reports that certain functions are too complex for full analysis. OK to skip.
122 useStlAlgorithm errors: cppcheck suggests replacing raw loops with std::all_of or std::none_of. OK to skip.
96 constParameterReference errors: Confirmed all remaining warnings are false positives. cppcheck tends to flag cases involving state pointer dereferencing and Array1D types where mutations are not properly detected.
1 duplicateExpression error: Addressed in this PR. The 0.1/0.1 assumption is correct per IndoorGreen.cc: Possible bug in aerodynamic resistance calculation (ETBaseFunction) #11560, so a cppcheck-suppress comment was added.
2 unreadVariable errors: Addressed in this PR.
17 variableScope errors: Addressed in this PR.
3 shadowVariable errors: Addressed in this PR.
10 unpreciseMathCall errors: Skipped, as fixes caused diffs on Mac.
1 duplicateCondition error: Addressed in this PR.
1 constParameterPointer error: Addressed in this PR.
1 uselessAssignmentArg error: Addressed in this PR.
1 constVariablePointer error: Addressed in this PR.
4 constVariable errors: Addressed in this PR.
1 invalidFunctionArg error: Addressed in this PR.
Review of remaining uselessCallsSubstr, knownConditionTrueFalse, unassignedVariable, passedByValueCallback, stlIfStrFind, redundantAssignment, and constVariableReference errors is in progress.

Description of the purpose of this PR

Pull Request Author

Reviewer

…verified to be correct

…into Cppcheck_final_followup

dareumnam · 2026-05-08T14:36:45Z

 #else
    if (number_of_threads > 1) {
        displayMessage("ConvertInputFormat is not compiled with OpenMP. Only running on 1 thread, not requested {} threads.", number_of_threads);
-        number_of_threads = 1;


In the non-OpenMP path, number_of_threads = 1 is assigned inside the if block but never read afterward. Removed the dead assignment.

dareumnam · 2026-05-08T14:37:19Z

            if (errorFound) {
                ShowSevereError(state, EnergyPlus::format("The index of \"{}\" is not found", thisFurnace.SuppHeatCoilName));
                ShowContinueError(state, EnergyPlus::format("...occurs for {}", thisFurnace.Name));
-                errorFound = false;


The last errorFound = false at the end of the suppHeatCoil error block is a dead assignment since the function returns right after and errorFound is never read again. Safe to just remove that line.

dareumnam · 2026-05-08T14:37:54Z


-        int BoilerNum = 1;
        if (boilerObjects != inputProcessor->epJSON.end()) {
+            int BoilerNum = 1;


Same issue as the ConvHWBaseboardNum case. BoilerNum is declared outside the if block but only used inside the for loop within it. Just move the declaration inside the if block to tighten the scope. Easy fix, no logic change needed.

dareumnam · 2026-05-08T14:38:46Z


-            int ConvHWBaseboardNum = 0;
            if (baseboardObjects != inputProcessor->epJSON.end()) {
+                int ConvHWBaseboardNum = 0;


ConvHWBaseboardNum is declared outside the if block but only used inside the for loop. Just move the declaration inside the if block to tighten the scope. No logic change needed.

dareumnam · 2026-05-08T14:39:23Z


-        int BaseboardNum = 0;
        if (elecBaseboardObjects != inputProcessor->epJSON.end()) {
+            int BaseboardNum = 0;


Same as above.

dareumnam · 2026-05-08T14:40:42Z

-        if (TotalLoad > this->MaxLoad) {
-            DeRate = true;
-        }
+        bool DeRate = (TotalLoad > this->MaxLoad); // If true, need to derate aircoils because don't carry over unmet energy


DeRate is declared way up at the top of the function(line 15207) but only used in the else block at the very bottom, and it's not even initialized at declaration. Move it into the else block and initialize inline works good and avoids the potential uninitialized read.

dareumnam · 2026-05-08T14:42:41Z

            }

-            AlphaOffset = 3;
+            constexpr int AlphaOffset = 3; // local temp var


AlphaOffset is only used inside the for loop, so let's move the declaration there to tighten the scope. No overhead concern here inside the for loop since it's a primitive int (and a constexpr at that, so the compiler just substitutes 3 at compile time anyway).

dareumnam · 2026-05-08T14:43:21Z

            state.dataHeatBal->ExtVentedCavity(Item).OSCMPtr = Found;

-            Roughness = s_ipsc->cAlphaArgs(3);
+            std::string Roughness = s_ipsc->cAlphaArgs(3);


Roughness is only used inside the for loop, so move the declaration there and initialize it directly.

dareumnam · 2026-05-08T14:47:07Z

-            IShadedConst = windowShadingControl.getInputShadedConstruction;
-            IShadingDevice = windowShadingControl.ShadingDevice;
+            int IShadedConst = windowShadingControl.getInputShadedConstruction; // Construction number of shaded construction
+            int IShadingDevice = windowShadingControl.ShadingDevice;            // Material number of shading device


Both are only used inside the for loop, so declare them there where they're first assigned. This is safe because both are re-initialized at the start of every iteration anyway, so no state leaks between iterations.

dareumnam · 2026-05-08T14:47:30Z


            // Check for illegal shading type name
-            Found = Util::FindItemInList(s_ipsc->cAlphaArgs(3), cValidShadingTypes, NumValidShadingTypes);
+            int Found = Util::FindItemInList(s_ipsc->cAlphaArgs(3), cValidShadingTypes, NumValidShadingTypes);


Found is only used in one spot inside the loop, so declare and initialize it there directly.

dareumnam · 2026-05-08T14:48:11Z

-            Aface = {{0.0}};
-            Bface = {0.0};
+            std::array<std::array<Real64, maxArraySize>, maxArraySize> Aface = {{0.0}}; // Coefficient in equation Aface*thetas = Bface
+            std::array<Real64, maxArraySize> Bface = {0.0};                             // Coefficient in equation Aface*thetas = Bface


Aface and Bface are only used inside the while loop and get reset to zero at the start of each iteration anyway. Move the declarations into the loop and combine them with the reset. No real performance concern here either since both arrays are fully overwritten every iteration and the compiler will optimize the stack allocation away.

dareumnam · 2026-05-08T14:49:03Z

-                    if (Util::SameString(AirflowNetworkLinkageData(i).NodeNames[0], DisSysNodeData(k).Name)) {
-                        AirflowNetworkLinkageData(i).NodeNums[0] = k;
+                for (int l = 1; l <= DisSysNumOfNodes; ++l) {
+                    if (Util::SameString(AirflowNetworkLinkageData(i).NodeNames[0], DisSysNodeData(l).Name)) {


This local k declared inside a nested block that shadows the outer function-level k. Rename the inner one to something more descriptive to avoid the shadow.

dareumnam · 2026-05-08T14:49:49Z

-            auto &vrfTU = state.dataHVACVarRefFlow->VRFTU(TUIndex);
-            if (vrfTU.CoolCoilIndex > 0) {
-                DXCoilCap = DXCoils::GetCoilCapacityByIndexType(state, vrfTU.CoolCoilIndex, vrfTU.coolCoilType, errFlag);
+            auto &vrfTUobj = state.dataHVACVarRefFlow->VRFTU(TUIndex);


vrfTU is declared at function scope and then shadowed inside the for loop by another vrfTU pointing to a different TU. Rename the inner one to vrfTUobj to make it clear these are different objects.

dareumnam · 2026-05-08T14:50:31Z


-                // Set current outside flux:
-                if (construct.SourceSinkPresent) {
+                    // Set current outside flux:


The second and third if (construct.SourceSinkPresent) checks are consecutive with no intervening changes to construct, so they can be merged into one.

dareumnam · 2026-05-08T14:50:56Z

+    inline int FindItemInList(std::string_view const String,
+                              Container const &ListOfItems,
+                              const std::string Container::value_type::*const name_p,
+                              int const NumItems)


name_p is a pointer-to-member that's only used for reading (ListOfItems[i].*name_p), but the member it points to isn't declared const. Change to const to correctly express that the pointed-to member is read-only.

dareumnam · 2026-05-08T14:51:29Z

            }
-        } else {
-            nRunPeriods = 1;
        }


nRunPeriods is passed by value, so nRunPeriods = 1 in the else branch is useless. The assignment has no effect after the function returns. Remove the else block entirely.

dareumnam added 10 commits May 7, 2026 13:37

Suppress the duplicateExpression cppcheck error as the line has been …

2b6e7b0

…verified to be correct

Merge branch 'develop' of https://github.com/NatLabRockies/EnergyPlus …

1b6e669

…into Cppcheck_final_followup

fix remaning unreadVariable

a5a0485

remaining variableScope

9a2af19

remaining shadowVariable

f86defa

remaining duplicateCondition

bb24caf

remaining constParameterPointer

f062ca0

remaining uselessAssignmentArg

bb49211

remaining constVariablePointer

5f4fe7b

remaining constVariable

b06afe3

dareumnam added the DoNotPublish Includes changes that shouldn't be reported in the changelog label May 8, 2026

dareumnam commented May 8, 2026

View reviewed changes

Conversation

dareumnam commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dareumnam May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

dareumnam May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dareumnam commented May 8, 2026 •

edited

Loading

dareumnam May 8, 2026 •

edited

Loading

dareumnam May 8, 2026 •

edited

Loading