Stop registering FactorDefs to the UnitTable

src/parser/unit.yy

@@ -104,7 +104,12 @@ table_insertion
         $$ = driver.table;
       }
     | table_insertion item  {
-        $1->insert($2);
+        try {
+          $1->insert($2);
+        }
+        catch (std::runtime_error e) {
+          error(scanner.loc, e.what());
+        }
         $$ = $1;
       }
     | table_insertion prefix  {

src/units/units.cpp

@@ -261,6 +261,20 @@ void UnitTable::calc_denominator_dims(const std::shared_ptr<Unit>& unit,
 }
 
 void UnitTable::insert(const std::shared_ptr<Unit>& unit) {
+    // if the unit is already in the table throw error because
+    // redefinition of a unit is not allowed
+    if (table.find(unit->get_name()) != table.end()) {
+        std::stringstream ss_unit_string;
+        ss_unit_string << fmt::format("{:g} {}",
+                                      unit->get_factor(),
+                                      fmt::join(unit->get_nominator_unit(), ""));
+        if (!unit->get_denominator_unit().empty()) {
+            ss_unit_string << fmt::format("/{}", fmt::join(unit->get_denominator_unit(), ""));
+        }
+        throw std::runtime_error(fmt::format("Redefinition of units ({}) to {} is not allowed.",
+                                             unit->get_name(),
+                                             ss_unit_string.str()));
+    }
     // check if the unit is a base unit and
     // then add it to the base units vector
     auto unit_nominator = unit->get_nominator_unit();
@@ -273,14 +287,7 @@ void UnitTable::insert(const std::shared_ptr<Unit>& unit) {
         assert(index >= 0 && index < base_units_names.size());
         // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
         base_units_names[index] = unit->get_name();
-        // if  unit is found in table replace it
-        auto find_unit_name = table.find(unit->get_name());
-        if (find_unit_name == table.end()) {
-            table.insert({unit->get_name(), unit});
-        } else {
-            table.erase(unit->get_name());
-            table.insert({unit->get_name(), unit});
-        }
+        table.insert({unit->get_name(), unit});
         return;
     }
     // calculate unit's dimensions based on its nominator and denominator
@@ -290,15 +297,7 @@ void UnitTable::insert(const std::shared_ptr<Unit>& unit) {
     for (const auto& it: unit->get_denominator_unit()) {
         calc_denominator_dims(unit, it);
     }
-    // if unit is not in the table simply insert it, else replace with it with
-    // new definition
-    auto find_unit_name = table.find(unit->get_name());
-    if (find_unit_name == table.end()) {
-        table.insert({unit->get_name(), unit});
-    } else {
-        table.erase(unit->get_name());
-        table.insert({unit->get_name(), unit});
-    }
+    table.insert({unit->get_name(), unit});
 }
 
 void UnitTable::insert_prefix(const std::shared_ptr<Prefix>& prfx) {

src/visitors/units_visitor.cpp

@@ -81,23 +81,8 @@ void UnitsVisitor::visit_unit_def(ast::UnitDef& node) {
  * care of all the units calculations.
  */
 void UnitsVisitor::visit_factor_def(ast::FactorDef& node) {
-    std::ostringstream ss;
     const auto node_has_value_defined_in_modfile = node.get_value() != nullptr;
-    if (node_has_value_defined_in_modfile) {
-        /*
-         * In nrnunits.lib file "1" is defined as "fuzz", so
-         * there must be a conversion to be able to parse "1" as unit
-         */
-        if (node.get_unit1()->get_node_name() == "1") {
-            ss << node.get_node_name() << "\t" << node.get_value()->eval() << " ";
-            ss << UNIT_FUZZ;
-        } else {
-            ss << node.get_node_name() << "\t" << node.get_value()->eval() << " ";
-            ss << node.get_unit1()->get_node_name();
-        }
-        // Parse the generated string for the defined unit using the units::UnitParser
-        units_driver.parse_string(ss.str());
-    } else {
+    if (!node_has_value_defined_in_modfile) {
         std::ostringstream ss_unit1, ss_unit2;
         std::string unit1_name, unit2_name;
         /*
@@ -127,10 +112,6 @@ void UnitsVisitor::visit_factor_def(ast::FactorDef& node) {
         ss_unit2 << node.get_node_name() << "_unit2\t" << unit2_name;
         units_driver.parse_string(ss_unit2.str());
 
-        // Parse the generated string for the defined unit using the units::UnitParser
-        ss << node.get_node_name() << "\t" << unit1_name;
-        units_driver.parse_string(ss.str());
-
         /**
          * \note If the ast::FactorDef was made by using two units (second case),
          * the factors of both of them must be calculated based on the

test/unit/units/parser.cpp

@@ -187,7 +187,7 @@ SCENARIO("Unit parser accepting dependent/nested units definition", "[unit][pars
                 dummy3  25-3  / m2
                 dummy4  -0.025 /m2
                 dummy5  2.5 %
-                R       k-mole
+                newR       k-mole
                 R1      8.314 volt-coul/degC
                 R2      8314 mV-coul/degC
                 )";
@@ -202,7 +202,7 @@ SCENARIO("Unit parser accepting dependent/nested units definition", "[unit][pars
                 REQUIRE_THAT(parsed_units, Contains("dummy3 0.025: -2 0 0 0 0 0 0 0 0 0"));
                 REQUIRE_THAT(parsed_units, Contains("dummy4 -0.025: -2 0 0 0 0 0 0 0 0 0"));
                 REQUIRE_THAT(parsed_units, Contains("dummy5 0.025: 0 0 0 0 0 0 0 0 0 0"));
-                REQUIRE_THAT(parsed_units, Contains("R 8.31446: 2 1 -2 0 0 0 0 0 0 -1"));
+                REQUIRE_THAT(parsed_units, Contains("newR 8.31446: 2 1 -2 0 0 0 0 0 0 -1"));
                 REQUIRE_THAT(parsed_units, Contains("R1 8.314: 2 1 -2 0 0 0 0 0 0 -1"));
                 REQUIRE_THAT(parsed_units, Contains("R2 8.314: 2 1 -2 0 0 0 0 0 0 -1"));
                 REQUIRE_THAT(parsed_units, Contains("m kg sec coul candela dollar bit erlang K"));