Skip to content

fixed some -Wnrvo Clang compiler warnings#7883

Open
firewave wants to merge 2 commits intodanmar:mainfrom
firewave:nrvo
Open

fixed some -Wnrvo Clang compiler warnings#7883
firewave wants to merge 2 commits intodanmar:mainfrom
firewave:nrvo

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave marked this pull request as ready for review October 19, 2025 23:19
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know.. of course I want to have performance. But we obfuscate the code in various places that are probably not hotspots.
I feel that the original code is more well written and readable.

Comment thread lib/fwdanalysis.cpp
checkRecursive(expr, startEndTokens.first, startEndTokens.second->next(), exprVarIds, local, true, depth);
if (result.type != Result::Type::NONE)
return result;
{
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite pedantic by clang imho. FwdAnalysis::Result is small and simple data. But yes if we want to turn on the warning then you have to fix all warnings so I guess there's not much we can do.

Comment thread lib/mathlib.cpp
return "0.0";
if (s.find_first_of(".e") == std::string::npos)
return s + ".0";
s = "0.0";
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I like the old code better here from a readability point of view.

Comment thread lib/programmemory.cpp
result = execute(elseStart->scope());
} else {
return {unknown()};
result = {unknown()};
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is less obvious what happens here. if the loop continues then other results can be returned.. so I want to see an unconditional return.

overflow = true;
indexValues.push_back(values.front());
}
if (overflow)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks more weird imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants