Skip to content

Commit

Permalink
Fix somes issues spotted by SonarSource
Browse files Browse the repository at this point in the history
  • Loading branch information
@tristan0x
tristan0x committed Sep 29, 2023
1 parent 2f433c3 commit 5efdd29
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 77 deletions.
6 changes: 3 additions & 3 deletions src/codegen/codegen_acc_visitor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ class CodegenAccVisitor: public CodegenCppVisitor {
void print_net_send_buf_update_to_host() const override;

/// update NetSendBuffer_t count from host to device
virtual void print_net_send_buf_count_update_to_device() const override;
void print_net_send_buf_count_update_to_device() const override;

/// update dt from host to device
virtual void print_dt_update_to_device() const override;
void print_dt_update_to_device() const override;

// synchronise/wait on stream specific to NrnThread
virtual void print_device_stream_wait() const override;
void print_device_stream_wait() const override;

/// print atomic capture pragma
void print_device_atomic_capture_annotation() const override;
Expand Down
5 changes: 2 additions & 3 deletions src/codegen/codegen_compatibility_visitor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "utils/logger.hpp"
#include "visitors/visitor_utils.hpp"


namespace nmodl::codegen {

const std::map<ast::AstNodeType, CodegenCompatibilityVisitor::FunctionPointer>
Expand Down Expand Up @@ -100,7 +99,7 @@ std::string CodegenCompatibilityVisitor::return_error_if_no_bbcore_read_write(
parser::CDriver driver;

driver.scan_string(verbatim_statement_string);
auto tokens = driver.all_tokens();
const auto& tokens = driver.all_tokens();

for (const auto& token: tokens) {
if (token == "bbcore_read") {
Expand Down Expand Up @@ -131,7 +130,7 @@ std::string CodegenCompatibilityVisitor::return_error_if_no_bbcore_read_write(
bool CodegenCompatibilityVisitor::find_unhandled_ast_nodes(Ast& node) const {
std::vector<ast::AstNodeType> unhandled_ast_types;
unhandled_ast_types.reserve(unhandled_ast_types_func.size());
for (auto [node_type, _]: unhandled_ast_types_func) {
for (const auto& [node_type, _]: unhandled_ast_types_func) {
unhandled_ast_types.push_back(node_type);
}
const auto& unhandled_ast_nodes = collect_nodes(node, unhandled_ast_types);
Expand Down
75 changes: 32 additions & 43 deletions src/codegen/codegen_cpp_visitor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,6 @@ std::string CodegenCppVisitor::breakpoint_current(std::string current) const {
return current;
}


int CodegenCppVisitor::float_variables_size() const {
return codegen_float_variables.size();
}


int CodegenCppVisitor::int_variables_size() const {
const auto count_semantics = [](int sum, const IndexSemantics& sem) { return sum += sem.size; };
return std::accumulate(info.semantics.begin(), info.semantics.end(), 0, count_semantics);
Expand All @@ -553,20 +547,19 @@ std::vector<std::string> CodegenCppVisitor::ion_read_statements(BlockType type)
}
std::vector<std::string> statements;
for (const auto& ion: info.ions) {
auto name = ion.name;
for (const auto& var: ion.reads) {
auto const iter = std::find(ion.implicit_reads.begin(), ion.implicit_reads.end(), var);
if (iter != ion.implicit_reads.end()) {
continue;
}
auto variable_names = read_ion_variable_name(var);
const auto& variable_names = read_ion_variable_name(var);
auto first = get_variable_name(variable_names.first);
auto second = get_variable_name(variable_names.second);
statements.push_back(fmt::format("{} = {};", first, second));
}
for (const auto& var: ion.writes) {
if (ion.is_ionic_conc(var)) {
auto variables = read_ion_variable_name(var);
const auto& variables = read_ion_variable_name(var);
auto first = get_variable_name(variables.first);
auto second = get_variable_name(variables.second);
statements.push_back(fmt::format("{} = {};", first, second));
Expand All @@ -577,7 +570,7 @@ std::vector<std::string> CodegenCppVisitor::ion_read_statements(BlockType type)
}


std::vector<std::string> CodegenCppVisitor::ion_read_statements_optimized(BlockType type) const {
std::vector<std::string> CodegenCppVisitor::ion_read_statements_optimized(BlockType /* type */) const {

Check warning on line 573 in src/codegen/codegen_cpp_visitor.cpp

View check run for this annotation

Codecov / codecov/patch

src/codegen/codegen_cpp_visitor.cpp#L573 

Added line #L573 was not covered by tests
std::vector<std::string> statements;
for (const auto& ion: info.ions) {
for (const auto& var: ion.writes) {
Expand All @@ -593,13 +586,12 @@ std::vector<std::string> CodegenCppVisitor::ion_read_statements_optimized(BlockT
}

// NOLINTNEXTLINE(readability-function-cognitive-complexity)
std::vector<ShadowUseStatement> CodegenCppVisitor::ion_write_statements(BlockType type) {
std::vector<ShadowUseStatement> CodegenCppVisitor::ion_write_statements(BlockType type) const {
std::vector<ShadowUseStatement> statements;
for (const auto& ion: info.ions) {
std::string concentration;
auto name = ion.name;
for (const auto& var: ion.writes) {
auto variable_names = write_ion_variable_name(var);
const auto& variable_names = write_ion_variable_name(var);
if (ion.is_ionic_current(var)) {
if (type == BlockType::Equation) {
auto current = breakpoint_current(var);
Expand All @@ -616,9 +608,9 @@ std::vector<ShadowUseStatement> CodegenCppVisitor::ion_write_statements(BlockTyp
if (!ion.is_rev_potential(var)) {
concentration = var;
}
auto lhs = variable_names.first;
const auto& lhs = variable_names.first;
auto op = "=";
auto rhs = get_variable_name(variable_names.second);
const auto& rhs = get_variable_name(variable_names.second);
statements.push_back(ShadowUseStatement{lhs, op, rhs});
}
}
Expand Down Expand Up @@ -741,7 +733,7 @@ void CodegenCppVisitor::update_index_semantics() {
info.semantics.emplace_back(index++, fmt::format("#{}_ion", ion.name), 1);
}
}
for (auto& var: info.pointer_variables) {
for (const auto& var: info.pointer_variables) {
if (info.first_pointer_var_index == -1) {
info.first_pointer_var_index = index;
}
Expand Down Expand Up @@ -1372,8 +1364,6 @@ void CodegenCppVisitor::print_function_prototypes() {


static const TableStatement* get_table_statement(const ast::Block& node) {
// TableStatementVisitor v;

const auto& table_statements = collect_nodes(node, {AstNodeType::TABLE_STATEMENT});

if (table_statements.size() != 1) {
Expand All @@ -1386,7 +1376,7 @@ static const TableStatement* get_table_statement(const ast::Block& node) {
}


std::tuple<bool, int> CodegenCppVisitor::check_if_var_is_array(const std::string& name) {
std::tuple<bool, int> CodegenCppVisitor::check_if_var_is_array(const std::string& name) const {
auto symbol = program_symtab->lookup_in_scope(name);
if (!symbol) {
throw std::runtime_error(
Expand All @@ -1402,8 +1392,8 @@ std::tuple<bool, int> CodegenCppVisitor::check_if_var_is_array(const std::string

void CodegenCppVisitor::print_table_check_function(const Block& node) {
auto statement = get_table_statement(node);
auto table_variables = statement->get_table_vars();
auto depend_variables = statement->get_depend_vars();
const auto& table_variables = statement->get_table_vars();
const auto& depend_variables = statement->get_depend_vars();
const auto& from = statement->get_from();
const auto& to = statement->get_to();
auto name = node.get_node_name();
Expand All @@ -1412,7 +1402,6 @@ void CodegenCppVisitor::print_table_check_function(const Block& node) {
auto use_table_var = get_variable_name(naming::USE_TABLE_VARIABLE);
auto tmin_name = get_variable_name("tmin_" + name);
auto mfac_name = get_variable_name("mfac_" + name);
auto float_type = default_float_data_type();

printer->add_newline(2);
print_device_method_annotation();
Expand All @@ -1426,7 +1415,7 @@ void CodegenCppVisitor::print_table_check_function(const Block& node) {

printer->add_line("static bool make_table = true;");
for (const auto& variable: depend_variables) {
printer->fmt_line("static {} save_{};", float_type, variable->get_node_name());
printer->fmt_line("static {} save_{};", default_float_data_type(), variable->get_node_name());
}

for (const auto& variable: depend_variables) {
Expand Down Expand Up @@ -1921,7 +1910,7 @@ void CodegenCppVisitor::print_eigen_linear_solver(const std::string& float_type,
/****************************************************************************************/


std::string CodegenCppVisitor::internal_method_arguments() {
std::string CodegenCppVisitor::internal_method_arguments() const {
if (ion_variable_struct_required()) {
return "id, pnodecount, inst, ionvar, data, indexes, thread, nt, v";
}
Expand All @@ -1932,7 +1921,7 @@ std::string CodegenCppVisitor::internal_method_arguments() {
/**
* @todo: figure out how to correctly handle qualifiers
*/
CodegenCppVisitor::ParamVector CodegenCppVisitor::internal_method_parameters() {
CodegenCppVisitor::ParamVector CodegenCppVisitor::internal_method_parameters() const {
ParamVector params;
params.emplace_back("", "int", "", "id");
params.emplace_back("", "int", "", "pnodecount");
Expand Down Expand Up @@ -1976,7 +1965,7 @@ std::string CodegenCppVisitor::nrn_thread_arguments() const {
* Function call arguments when function or procedure is defined in the
* same mod file itself
*/
std::string CodegenCppVisitor::nrn_thread_internal_arguments() {
std::string CodegenCppVisitor::nrn_thread_internal_arguments() const {

Check warning on line 1968 in src/codegen/codegen_cpp_visitor.cpp

View check run for this annotation

Codecov / codecov/patch

src/codegen/codegen_cpp_visitor.cpp#L1968 

Added line #L1968 was not covered by tests
if (ion_variable_struct_required()) {
return "id, pnodecount, inst, ionvar, data, indexes, thread, nt, v";
}
Expand Down Expand Up @@ -2075,7 +2064,7 @@ std::pair<std::string, std::string> CodegenCppVisitor::write_ion_variable_name(

std::string CodegenCppVisitor::conc_write_statement(const std::string& ion_name,
const std::string& concentration,
int index) {
int index) const {
auto conc_var_name = get_variable_name(naming::ION_VARNAME_PREFIX + concentration);
auto style_var_name = get_variable_name("style_" + ion_name);
return fmt::format(
Expand All @@ -2102,7 +2091,7 @@ std::string CodegenCppVisitor::conc_write_statement(const std::string& ion_name,
* to queue that will be used in reduction queue.
*/
std::string CodegenCppVisitor::process_shadow_update_statement(const ShadowUseStatement& statement,
BlockType /* type */) {
BlockType /* type */) const {
// when there is no operator or rhs then that statement doesn't need shadow update
if (statement.op.empty() && statement.rhs.empty()) {
auto text = statement.lhs + ";";
Expand Down Expand Up @@ -2524,7 +2513,7 @@ void CodegenCppVisitor::print_mechanism_global_var_structure(bool print_initiali
// is false. But as v is always local variable and passed as argument,
// we don't need to use global variable v

auto& top_locals = info.top_local_variables;
const auto& top_locals = info.top_local_variables;
if (!info.vectorize && !top_locals.empty()) {
for (const auto& var: top_locals) {
auto name = var->get_name();
Expand Down Expand Up @@ -2586,7 +2575,7 @@ void CodegenCppVisitor::print_mechanism_global_var_structure(bool print_initiali

for (const auto& var: info.constant_variables) {
auto const name = var->get_name();
auto* const value_ptr = var->get_value().get();
const auto* const value_ptr = var->get_value().get();
double const value{value_ptr ? *value_ptr : 0};
printer->fmt_line("{}{} {}{};",
qualifier,
Expand Down Expand Up @@ -3074,13 +3063,13 @@ void CodegenCppVisitor::print_mechanism_range_var_structure(bool print_initializ
print_initializers ? fmt::format("{{&coreneuron::{}}}", name)
: std::string{});
}
for (auto& var: codegen_float_variables) {
for (const auto& var: codegen_float_variables) {
const auto& name = var->get_name();
auto type = get_range_var_float_type(var);
auto qualifier = is_constant_variable(name) ? "const " : "";
printer->fmt_line("{}{}* {}{};", qualifier, type, name, value_initialize);
}
for (auto& var: codegen_int_variables) {
for (const auto& var: codegen_int_variables) {
const auto& name = var.symbol->get_name();
if (var.is_index || var.is_integer) {
auto qualifier = var.is_constant ? "const " : "";
Expand Down Expand Up @@ -3178,7 +3167,7 @@ void CodegenCppVisitor::print_setup_range_variable() {
* are pointers to internal variables (e.g. ions). Hence, we check if given
* variable can be safely converted to new type. If so, return new type.
*/
std::string CodegenCppVisitor::get_range_var_float_type(const SymbolType& symbol) {
std::string CodegenCppVisitor::get_range_var_float_type(const SymbolType& symbol) const {
// clang-format off
auto with = NmodlType::read_ion_var
| NmodlType::write_ion_var
Expand Down Expand Up @@ -3282,7 +3271,7 @@ void CodegenCppVisitor::print_instance_variable_setup() {
id += var->get_length();
}

for (auto& var: codegen_int_variables) {
for (const auto& var: codegen_int_variables) {
auto name = var.symbol->get_name();
auto const variable = [&var]() {
if (var.is_index || var.is_integer) {
Expand Down Expand Up @@ -3340,9 +3329,9 @@ void CodegenCppVisitor::print_initial_block(const InitialBlock* node) {
}

// write ion statements
auto write_statements = ion_write_statements(BlockType::Initial);
for (auto& statement: write_statements) {
auto text = process_shadow_update_statement(statement, BlockType::Initial);
const auto& write_statements = ion_write_statements(BlockType::Initial);
for (const auto& statement: write_statements) {
const auto& text = process_shadow_update_statement(statement, BlockType::Initial);
printer->add_line(text);
}
}
Expand Down Expand Up @@ -3747,8 +3736,8 @@ void CodegenCppVisitor::print_net_receive_common_code(const Block& node, bool ne
if (!parameters.empty()) {
int i = 0;
printer->add_newline();
for (auto& parameter: parameters) {
auto name = parameter->get_node_name();
for (const auto& parameter: parameters) {
const auto& name = parameter->get_node_name();
bool var_used = VarUsageVisitor().variable_used(node, "(*" + name + ")");
if (var_used) {
printer->fmt_line("double* {} = weights + weight_index + {};", name, i);
Expand Down Expand Up @@ -4362,7 +4351,7 @@ void CodegenCppVisitor::print_nrn_current(const BreakpointBlock& node) {
get_parameter_str(args));
printer->add_line("double current = 0.0;");
print_statement_block(*block, false, false);
for (auto& current: info.currents) {
for (const auto& current: info.currents) {
const auto& name = get_variable_name(current);
printer->fmt_line("current += {};", name);
}
Expand Down Expand Up @@ -4408,12 +4397,12 @@ void CodegenCppVisitor::print_nrn_cur_conductance_kernel(const BreakpointBlock&
}


void CodegenCppVisitor::print_nrn_cur_non_conductance_kernel() {
void CodegenCppVisitor::print_nrn_cur_non_conductance_kernel() const {
printer->fmt_line("double g = nrn_current_{}({}+0.001);",
info.mod_suffix,
internal_method_arguments());
for (auto& ion: info.ions) {
for (auto& var: ion.writes) {
for (const auto& ion: info.ions) {
for (const auto& var: ion.writes) {

Check warning on line 4405 in src/codegen/codegen_cpp_visitor.cpp

View check run for this annotation

Codecov / codecov/patch

src/codegen/codegen_cpp_visitor.cpp#L4405 

Added line #L4405 was not covered by tests
if (ion.is_ionic_current(var)) {
const auto& name = get_variable_name(var);
printer->fmt_line("double di{} = {};", ion.name, name);
Expand Down
Loading

0 comments on commit 5efdd29

Please sign in to comment.