Remove footgun!!!!

arbor-sim · Aug 3, 2023 · c525fc6 · c525fc6
1 parent 25069d9
commit c525fc6
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 66 deletions.
diff --git a/arbor/arbexcept.cpp b/arbor/arbexcept.cpp
@@ -10,8 +10,6 @@
 #include <fmt/format.h>
 
 namespace arb {
-using util::to_string;
-
 arbor_exception::arbor_exception(const std::string& what):
  std::runtime_error{what} {
  // Backtrace w/o this c'tor and that of backtrace.
@@ -27,14 +25,14 @@ arbor_internal_error::arbor_internal_error(const std::string& what):
 domain_error::domain_error(const std::string& w): arbor_exception(w) {}
 
 bad_cell_probe::bad_cell_probe(cell_kind kind, cell_gid_type gid):
- arbor_exception(fmt::format("Probe kind is not supported for cell with gid {} of kind {}. Possibly the cell kind is not probeable at all.", gid, to_string(kind))),
+ arbor_exception(fmt::format("Probe kind is not supported for cell with gid {} of kind {}. Possibly the cell kind is not probeable at all.", gid, util::to_string(kind))),
  gid(gid),
  kind(kind)
 {}
 
 bad_cell_description::bad_cell_description(cell_kind kind, cell_gid_type gid):
 
- arbor_exception(fmt::format("recipe::get_cell_kind(gid={}) -> {} does not match the cell type provided by recipe::get_cell_description(gid={})", gid, to_string(kind), gid)),
+ arbor_exception(fmt::format("recipe::get_cell_kind(gid={}) -> {} does not match the cell type provided by recipe::get_cell_description(gid={})", gid, util::to_string(kind), gid)),
  gid(gid),
  kind(kind)
 {}
@@ -65,7 +63,7 @@ bad_connection_label::bad_connection_label(cell_gid_type gid, const cell_tag_typ
 {}
 
 bad_global_property::bad_global_property(cell_kind kind):
- arbor_exception(fmt::format("bad global property for cell kind {}", to_string(kind))),
+ arbor_exception(fmt::format("bad global property for cell kind {}", util::to_string(kind))),
  kind(kind)
 {}
 

diff --git a/arbor/cable_cell.cpp b/arbor/cable_cell.cpp
@@ -20,8 +20,6 @@
 
 namespace arb {
 
-using util::to_string;
-
 using value_type = cable_cell::value_type;
 using index_type = cable_cell::index_type;
 using size_type = cable_cell::size_type;
@@ -195,7 +193,7 @@ struct cable_cell_impl {
  if (!mm.insert(c, {prop.t_mech, im})) {
  throw cable_cell_error(fmt::format("Painting density mechanism '{}' on region '{}' overpaints at '{}'.",
  prop.t_mech.mech.name(),
- to_string(c)));
+ util::to_string(c)));
  }
  }
  }
@@ -213,7 +211,7 @@ struct cable_cell_impl {
  throw cable_cell_error(fmt::format("Setting property '{}' on region '{}' overpaints at '{}'",
  show(prop),
  show(reg),
- to_string(c)));
+ util::to_string(c)));
  }
  }
  }

diff --git a/arbor/domdecexcept.cpp b/arbor/domdecexcept.cpp
@@ -8,8 +8,6 @@
 
 namespace arb {
 
-using util::to_string;
-
 invalid_gj_cell_group::invalid_gj_cell_group(cell_gid_type gid_0, cell_gid_type gid_1):
  dom_dec_exception(fmt::format("cell {} needs to be in the same group as cell {} because they are connected via gap-junction.",
  gid_0, gid_1)),
@@ -46,7 +44,7 @@ invalid_backend::invalid_backend(int rank):
 incompatible_backend::incompatible_backend(int rank, cell_kind kind):
  dom_dec_exception(fmt::format("rank {} contains a group with cells of kind {1} meant to run on the GPU backend, "
  "but no GPU backend support exists for {1}",
- rank, to_string(kind))),
+ rank, util::to_string(kind))),
  rank(rank),
  kind(kind)
 {}

diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp
@@ -32,8 +32,6 @@
 #include <fmt/format.h>
 
 namespace arb {
-
-using util::to_string;
 using util::assign;
 using util::assign_by;
 using util::count_along;
@@ -343,7 +341,7 @@ fvm_cv_discretize(const cable_cell& cell, const cable_cell_parameter_set& global
  auto sc = ie->eval(provider, cable);
  if (def <= 0.0 || std::isnan(def)) {
  throw make_cc_error("Illegal diffusivity '{}' for ion '{}' at cable {}."
- " Please check your expressions.", sc, ion, to_string(cable));
+ " Please check your expressions.", sc, ion, util::to_string(cable));
  }
  return sc;
  };

diff --git a/arbor/morph/morphexcept.cpp b/arbor/morph/morphexcept.cpp
@@ -8,14 +8,13 @@
 #include <arbor/morph/morphexcept.hpp>
 
 namespace arb {
- using util::to_string;
 
 static std::string msize_string(msize_t x) {
  return x==mnpos? "mnpos": fmt::format("{}", x);
 }
 
 invalid_mlocation::invalid_mlocation(mlocation loc):
- morphology_error(fmt::format("invalid mlocation {}", to_string(loc))),
+ morphology_error(fmt::format("invalid mlocation {}", util::to_string(loc))),
  loc(loc)
 {}
 
@@ -30,7 +29,7 @@ no_such_segment::no_such_segment(msize_t id):
 {}
 
 invalid_mcable::invalid_mcable(mcable cable):
- morphology_error(fmt::format("invalid mcable {}", to_string(cable))),
+ morphology_error(fmt::format("invalid mcable {}", util::to_string(cable))),
  cable(cable)
 {}
 

diff --git a/arbor/partition_load_balance.cpp b/arbor/partition_load_balance.cpp
@@ -22,8 +22,6 @@
 
 namespace arb {
 
-using util::to_string;
-
 ARB_ARBOR_API domain_decomposition partition_load_balance(
  const recipe& rec,
  context ctx,
@@ -189,12 +187,12 @@ ARB_ARBOR_API domain_decomposition partition_load_balance(
  hint = opt_hint.value();
  if(!hint.cpu_group_size) {
  throw arbor_exception(fmt::format("unable to perform load balancing because {} has invalid suggested cpu_cell_group size of {}",
- to_string(k),
+ util::to_string(k),
  hint.cpu_group_size));
  }
  if(hint.prefer_gpu && !hint.gpu_group_size) {
  throw arbor_exception(fmt::format("unable to perform load balancing because {} has invalid suggested gpu_cell_group size of {}",
- to_string(k),
+ util::to_string(k),
  hint.gpu_group_size));
  }
  }

diff --git a/modcc/parser.cpp b/modcc/parser.cpp
@@ -8,34 +8,29 @@
 
 #include "io/pprintf.hpp"
 
+#include <fmt/format.h>
+#include <fmt/color.h>
+
 // specialize on const char* for lazy evaluation of compile time strings
 bool Parser::expect(tok tok, const char* str) {
- if (tok == token_.type) {
- return true;
- }
-
- error(
- strlen(str) > 0 ? str
- : std::string("unexpected token ") + yellow(token_.spelling));
-
+ if (tok == token_.type) return true;
+ error(strlen(str) > 0
+ ? str
+ : fmt::format("unexpected token {}", fmt::styled(token_.spelling, fmt::fg(fmt::color::yellow))));
  return false;
 }
 
 bool Parser::expect(tok tok, std::string const& str) {
- if (tok == token_.type) {
- return true;
- }
-
- error(
- str.size() > 0 ? str
- : std::string("unexpected token ") + yellow(token_.spelling));
-
+ if (tok == token_.type) return true;
+ error(str.size() > 0
+ ? str
+ : fmt::format("unexpected token {}", fmt::styled(token_.spelling, fmt::fg(fmt::color::yellow))));
  return false;
 }
 
 void Parser::error(std::string msg) {
- std::string location_info = pprintf(
- "%:% ", module_ ? module_->source_name() : "", token_.location);
+ std::string location_info = fmt::format(
+ "{} {}:{} ", module_ ? module_->source_name() : "", token_.location.line, token_.location.column);
  if (status_ == lexerStatus::error) {
  // append to current string
  error_string_ += "\n" + white(location_info) + "\n " + msg;

diff --git a/test/unit/test_morph_expr.cpp b/test/unit/test_morph_expr.cpp
@@ -28,8 +28,6 @@ using testing::cablelist_eq;
 using testing::mlocationlist_eq;
 
 TEST(region, expr_repn) {
- using util::to_string;
-
  auto c1 = reg::cable(1, 0, 1);
  auto c2 = reg::cable(4, 0.125, 0.5);
  auto c3 = join(reg::cable(4, 0.125, 0.5), reg::cable(3, 0, 1));
@@ -42,20 +40,20 @@ TEST(region, expr_repn) {
  auto df = difference(t2, t1);
  auto cm = complement(t3);
 
- EXPECT_EQ(to_string(c1), "(cable 1 0 1)");
- EXPECT_EQ(to_string(c2), "(cable 4 0.125 0.5)");
- EXPECT_EQ(to_string(c3), "(join (cable 4 0.125 0.5) (cable 3 0 1))");
- EXPECT_EQ(to_string(b1), "(cable 1 0 1)");
- EXPECT_EQ(to_string(t1), "(tag 1)");
- EXPECT_EQ(to_string(t2), "(tag 2)");
- EXPECT_EQ(to_string(s), "(complete (tag 3))");
- EXPECT_EQ(to_string(intersect(c1, t2)), "(intersect (cable 1 0 1) (tag 2))");
- EXPECT_EQ(to_string(join(c1, t2)), "(join (cable 1 0 1) (tag 2))");
- EXPECT_EQ(to_string(join(t1, t2, t3)), "(join (join (tag 1) (tag 2)) (tag 3))");
- EXPECT_EQ(to_string(intersect(t1, t2, t3)), "(intersect (intersect (tag 1) (tag 2)) (tag 3))");
- EXPECT_EQ(to_string(all), "(all)");
- EXPECT_EQ(to_string(df), "(difference (tag 2) (tag 1))");
- EXPECT_EQ(to_string(cm), "(complement (tag 3))");
+ EXPECT_EQ(util::to_string(c1), "(cable 1 0 1)");
+ EXPECT_EQ(util::to_string(c2), "(cable 4 0.125 0.5)");
+ EXPECT_EQ(util::to_string(c3), "(join (cable 4 0.125 0.5) (cable 3 0 1))");
+ EXPECT_EQ(util::to_string(b1), "(cable 1 0 1)");
+ EXPECT_EQ(util::to_string(t1), "(tag 1)");
+ EXPECT_EQ(util::to_string(t2), "(tag 2)");
+ EXPECT_EQ(util::to_string(s), "(complete (tag 3))");
+ EXPECT_EQ(util::to_string(intersect(c1, t2)), "(intersect (cable 1 0 1) (tag 2))");
+ EXPECT_EQ(util::to_string(join(c1, t2)), "(join (cable 1 0 1) (tag 2))");
+ EXPECT_EQ(util::to_string(join(t1, t2, t3)), "(join (join (tag 1) (tag 2)) (tag 3))");
+ EXPECT_EQ(util::to_string(intersect(t1, t2, t3)), "(intersect (intersect (tag 1) (tag 2)) (tag 3))");
+ EXPECT_EQ(util::to_string(all), "(all)");
+ EXPECT_EQ(util::to_string(df), "(difference (tag 2) (tag 1))");
+ EXPECT_EQ(util::to_string(cm), "(complement (tag 3))");
 }
 
 TEST(region, invalid_mcable) {
@@ -65,23 +63,21 @@ TEST(region, invalid_mcable) {
 }
 
 TEST(locset, expr_repn) {
- using util::to_string;
-
  auto root = ls::root();
  auto term = ls::terminal();
  auto loc = ls::location(2, 0.5);
  auto bdy = ls::boundary(reg::tagged(1));
  auto prox = ls::proximal_translate(ls::terminal(), 100);
  auto dist = ls::distal_translate(ls::root(), 42);
 
- EXPECT_EQ(to_string(root), "(root)");
- EXPECT_EQ(to_string(term), "(terminal)");
- EXPECT_EQ(to_string(sum(root, term)), "(sum (root) (terminal))");
- EXPECT_EQ(to_string(sum(root, term, loc)), "(sum (sum (root) (terminal)) (location 2 0.5))");
- EXPECT_EQ(to_string(loc), "(location 2 0.5)");
- EXPECT_EQ(to_string(bdy), "(boundary (tag 1))");
- EXPECT_EQ(to_string(prox), "(proximal-translate (terminal) 100)");
- EXPECT_EQ(to_string(dist), "(distal-translate (root) 42)");
+ EXPECT_EQ(util::to_string(root), "(root)");
+ EXPECT_EQ(util::to_string(term), "(terminal)");
+ EXPECT_EQ(util::to_string(sum(root, term)), "(sum (root) (terminal))");
+ EXPECT_EQ(util::to_string(sum(root, term, loc)), "(sum (sum (root) (terminal)) (location 2 0.5))");
+ EXPECT_EQ(util::to_string(loc), "(location 2 0.5)");
+ EXPECT_EQ(util::to_string(bdy), "(boundary (tag 1))");
+ EXPECT_EQ(util::to_string(prox), "(proximal-translate (terminal) 100)");
+ EXPECT_EQ(util::to_string(dist), "(distal-translate (root) 42)");
 }
 
 TEST(locset, invalid_mlocation) {