Use fmt::format() function instead of ""_format literal

joto · joto · commit 5e8d774fa070 · 2023-01-06T20:15:40.000+01:00
This replaces most uses of the ""_format literal with function calls to fmt::format(). The latter form is available in all versions of the fmt library, while the ""_format literal is deprecated in version 8 and removed in version 9. This might cost us some performance and early error detection, because in at least some versions of the fmt library the _format() construct was used to compile the format patterns at run time. That's why there are some uses of the literal left in places where performance might be more critical, they need a closer look. See #1704
diff --git a/src/flex-index.cpp b/src/flex-index.cpp
@@ -49,7 +49,7 @@ flex_index_t::create_index(std::string const &qualified_table_name) const
 
     if (m_fillfactor != 0) {
         joiner.add("WITH");
-        joiner.add("(fillfactor = {})"_format(m_fillfactor));
+        joiner.add(fmt::format("(fillfactor = {})", m_fillfactor));
     }
 
     if (!m_tablespace.empty()) {
diff --git a/src/flex-table-column.cpp b/src/flex-table-column.cpp
@@ -146,21 +146,21 @@ std::string flex_table_column_t::sql_type_name() const
     case table_column_type::direction:
         return "int2";
     case table_column_type::geometry:
-        return "Geometry(GEOMETRY, {})"_format(m_srid);
+        return fmt::format("Geometry(GEOMETRY, {})", m_srid);
     case table_column_type::point:
-        return "Geometry(POINT, {})"_format(m_srid);
+        return fmt::format("Geometry(POINT, {})", m_srid);
     case table_column_type::linestring:
-        return "Geometry(LINESTRING, {})"_format(m_srid);
+        return fmt::format("Geometry(LINESTRING, {})", m_srid);
     case table_column_type::polygon:
-        return "Geometry(POLYGON, {})"_format(m_srid);
+        return fmt::format("Geometry(POLYGON, {})", m_srid);
     case table_column_type::multipoint:
-        return "Geometry(MULTIPOINT, {})"_format(m_srid);
+        return fmt::format("Geometry(MULTIPOINT, {})", m_srid);
     case table_column_type::multilinestring:
-        return "Geometry(MULTILINESTRING, {})"_format(m_srid);
+        return fmt::format("Geometry(MULTILINESTRING, {})", m_srid);
     case table_column_type::multipolygon:
-        return "Geometry(MULTIPOLYGON, {})"_format(m_srid);
+        return fmt::format("Geometry(MULTIPOLYGON, {})", m_srid);
     case table_column_type::geometrycollection:
-        return "Geometry(GEOMETRYCOLLECTION, {})"_format(m_srid);
+        return fmt::format("Geometry(GEOMETRYCOLLECTION, {})", m_srid);
     case table_column_type::area:
         return "real";
     case table_column_type::id_type:
@@ -188,5 +188,6 @@ std::string flex_table_column_t::sql_modifiers() const
 
 std::string flex_table_column_t::sql_create() const
 {
-    return R"("{}" {} {})"_format(m_name, sql_type_name(), sql_modifiers());
+    return fmt::format(R"("{}" {} {})", m_name, sql_type_name(),
+                       sql_modifiers());
 }
diff --git a/src/flex-table.cpp b/src/flex-table.cpp
@@ -150,15 +150,16 @@ flex_table_column_t &flex_table_t::add_column(std::string const &name,
 std::string flex_table_t::build_sql_prepare_get_wkb() const
 {
     if (has_multicolumn_id_index()) {
-        return "PREPARE get_wkb(char(1), bigint) AS"
-               " SELECT \"{}\" FROM {} WHERE \"{}\" = $1 AND \"{}\" = $2"_format(
-                   geom_column().name(), full_name(), m_columns[0].name(),
-                   m_columns[1].name());
+        return fmt::format(
+            "PREPARE get_wkb(char(1), bigint) AS"
+            " SELECT \"{}\" FROM {} WHERE \"{}\" = $1 AND \"{}\" = $2",
+            geom_column().name(), full_name(), m_columns[0].name(),
+            m_columns[1].name());
     }
 
-    return "PREPARE get_wkb(bigint) AS"
-           " SELECT \"{}\" FROM {} WHERE \"{}\" = $1"_format(
-               geom_column().name(), full_name(), id_column_names());
+    return fmt::format("PREPARE get_wkb(bigint) AS"
+                       " SELECT \"{}\" FROM {} WHERE \"{}\" = $1",
+                       geom_column().name(), full_name(), id_column_names());
 }
 
 std::string
@@ -167,8 +168,9 @@ flex_table_t::build_sql_create_table(table_type ttype,
 {
     assert(!m_columns.empty());
 
-    std::string sql = "CREATE {} TABLE IF NOT EXISTS {} ("_format(
-        ttype == table_type::interim ? "UNLOGGED" : "", table_name);
+    std::string sql =
+        fmt::format("CREATE {} TABLE IF NOT EXISTS {} (",
+                    ttype == table_type::interim ? "UNLOGGED" : "", table_name);
 
     util::string_joiner_t joiner{','};
     for (auto const &column : m_columns) {
@@ -208,8 +210,9 @@ std::string flex_table_t::build_sql_column_list() const
 
 std::string flex_table_t::build_sql_create_id_index() const
 {
-    return "CREATE INDEX ON {} USING BTREE ({}) {}"_format(
-        full_name(), id_column_names(), tablespace_clause(index_tablespace()));
+    return fmt::format("CREATE INDEX ON {} USING BTREE ({}) {}", full_name(),
+                       id_column_names(),
+                       tablespace_clause(index_tablespace()));
 }
 
 flex_index_t &flex_table_t::add_index(std::string method)
@@ -232,9 +235,9 @@ enable_check_trigger(pg_conn_t *db_connection, flex_table_t const &table)
 
     for (auto const &column : table) {
         if (column.is_geometry_column() && column.needs_isvalid()) {
-            checks.append(
-                R"((NEW."{0}" IS NULL OR ST_IsValid(NEW."{0}")) AND )"_format(
-                    column.name()));
+            checks.append(fmt::format(
+                R"((NEW."{0}" IS NULL OR ST_IsValid(NEW."{0}")) AND )",
+                column.name()));
         }
     }
 
@@ -303,8 +306,9 @@ void table_connection_t::stop(bool updateable, bool append)
             flex_table_t::table_type::permanent, table().full_tmp_name()));
 
         std::string const columns = table().build_sql_column_list();
-        std::string sql = "INSERT INTO {} ({}) SELECT {} FROM {}"_format(
-            table().full_tmp_name(), columns, columns, table().full_name());
+        std::string sql = fmt::format("INSERT INTO {} ({}) SELECT {} FROM {}",
+                                      table().full_tmp_name(), columns, columns,
+                                      table().full_name());
 
         auto const postgis_version = get_postgis_version();
 
@@ -316,11 +320,11 @@ void table_connection_t::stop(bool updateable, bool append)
             log_debug("Using GeoHash for clustering table '{}'",
                       table().name());
             if (table().geom_column().srid() == 4326) {
-                sql += "ST_GeoHash({},10)"_format(geom_column_name);
+                sql += fmt::format("ST_GeoHash({},10)", geom_column_name);
             } else {
-                sql +=
-                    "ST_GeoHash(ST_Transform(ST_Envelope({}),4326),10)"_format(
-                        geom_column_name);
+                sql += fmt::format(
+                    "ST_GeoHash(ST_Transform(ST_Envelope({}),4326),10)",
+                    geom_column_name);
             }
             sql += " COLLATE \"C\"";
         } else {
diff --git a/src/flex-write.cpp b/src/flex-write.cpp
@@ -36,8 +36,8 @@ static void write_null(db_copy_mgr_t<db_deleter_by_type_and_id_t> *copy_mgr,
 {
     if (column.not_null()) {
         throw not_null_exception{
-            "Can not add NULL to column '{}' declared NOT NULL."_format(
-                column.name()),
+            fmt::format("Can not add NULL to column '{}' declared NOT NULL.",
+                        column.name()),
             &column};
     }
     copy_mgr->add_null_column();
diff --git a/src/node-persistent-cache.cpp b/src/node-persistent-cache.cpp
@@ -38,7 +38,7 @@ node_persistent_cache::node_persistent_cache(std::string file_name,
     if (m_fd < 0) {
         throw std::system_error{
             errno, std::system_category(),
-            "Unable to open flatnode file '{}'"_format(m_file_name)};
+            fmt::format("Unable to open flatnode file '{}'", m_file_name)};
     }
 
     m_index = std::make_unique<index_t>(m_fd);
diff --git a/src/output-flex.cpp b/src/output-flex.cpp
@@ -739,7 +739,7 @@ int output_flex_t::table_tostring()
 {
     auto const &table = get_table_from_param();
 
-    std::string const str{"osm2pgsql.Table[{}]"_format(table.name())};
+    std::string const str{fmt::format("osm2pgsql.Table[{}]", table.name())};
     lua_pushstring(lua_state(), str.c_str());
 
     return 1;
diff --git a/src/output-gazetteer.cpp b/src/output-gazetteer.cpp
@@ -57,16 +57,17 @@ void output_gazetteer_t::start()
         /* Create the new table */
 
         std::string const sql =
-            "CREATE TABLE place ("
-            "  osm_id int8 NOT NULL,"
-            "  osm_type char(1) NOT NULL,"
-            "  class text NOT NULL,"
-            "  type text NOT NULL,"
-            "  name hstore,"
-            "  admin_level smallint,"
-            "  address hstore,"
-            "  extratags hstore," +
-            "  geometry Geometry(Geometry,{}) NOT NULL"_format(srid) + ")" +
+            fmt::format("CREATE TABLE place ("
+                        "  osm_id int8 NOT NULL,"
+                        "  osm_type char(1) NOT NULL,"
+                        "  class text NOT NULL,"
+                        "  type text NOT NULL,"
+                        "  name hstore,"
+                        "  admin_level smallint,"
+                        "  address hstore,"
+                        "  extratags hstore,"
+                        "  geometry Geometry(Geometry,{}) NOT NULL)",
+                        srid) +
             tablespace_clause(get_options()->tblsmain_data);
 
         conn.exec(sql);
diff --git a/src/pgsql-capabilities.cpp b/src/pgsql-capabilities.cpp
@@ -32,8 +32,8 @@ static void init_set_from_query(std::set<std::string> *set,
                                 char const *table, char const *column,
                                 char const *condition = "true")
 {
-    auto const res = db_connection.exec(
-        "SELECT {} FROM {} WHERE {}"_format(column, table, condition));
+    auto const res = db_connection.exec("SELECT {} FROM {} WHERE {}", column,
+                                        table, condition);
     for (int i = 0; i < res.num_tuples(); ++i) {
         set->emplace(res.get(i, 0));
     }
diff --git a/src/pgsql-helper.cpp b/src/pgsql-helper.cpp
@@ -82,9 +82,9 @@ void analyze_table(pg_conn_t const &db_connection, std::string const &schema,
 bool has_table(pg_conn_t const &db_connection, std::string const &schema,
                std::string const &table)
 {
-    auto const sql = "SELECT count(*) FROM pg_tables"
-                     "  WHERE schemaname='{}' AND tablename='{}'"_format(
-                         schema.empty() ? "public" : schema, table);
+    auto const sql = fmt::format("SELECT count(*) FROM pg_tables"
+                                 " WHERE schemaname='{}' AND tablename='{}'",
+                                 schema.empty() ? "public" : schema, table);
     auto const res = db_connection.exec(sql);
     char const *const num = res.get_value(0, 0);
 
diff --git a/src/progress-display.cpp b/src/progress-display.cpp
@@ -30,9 +30,9 @@ static std::string cps_display(std::size_t count, uint64_t elapsed)
     double const cps = count_per_second(count, elapsed);
 
     if (cps >= 1000.0) {
-        return "{:.0f}k/s"_format(cps / 1000);
+        return fmt::format("{:.0f}k/s", cps / 1000);
     }
-    return "{:.0f}/s"_format(cps);
+    return fmt::format("{:.0f}/s", cps);
 }
 
 void progress_display_t::print_summary() const
diff --git a/src/reprojection-generic-proj4.cpp b/src/reprojection-generic-proj4.cpp
@@ -71,5 +71,7 @@ std::shared_ptr<reprojection> reprojection::make_generic_projection(int srs)
     return std::make_shared<generic_reprojection_t>(srs);
 }
 
-std::string get_proj_version() { return "[API 4] {}"_format(pj_get_release()); }
-
+std::string get_proj_version()
+{
+    return fmt::format("[API 4] {}", pj_get_release());
+}
diff --git a/src/reprojection-generic-proj6.cpp b/src/reprojection-generic-proj6.cpp
@@ -60,8 +60,8 @@ class generic_reprojection_t : public reprojection
     {
         assert(m_context);
 
-        std::string const source = "epsg:{}"_format(from);
-        std::string const target = "epsg:{}"_format(to);
+        std::string const source = fmt::format("epsg:{}", from);
+        std::string const target = fmt::format("epsg:{}", to);
 
         std::unique_ptr<PJ, pj_deleter_t> trans{proj_create_crs_to_crs(
             m_context.get(), source.c_str(), target.c_str(), nullptr)};
@@ -119,6 +119,6 @@ std::shared_ptr<reprojection> reprojection::make_generic_projection(int srs)
 
 std::string get_proj_version()
 {
-    return "[API 6] {}"_format(proj_info().version);
+    return fmt::format("[API 6] {}", proj_info().version);
 }
 
diff --git a/src/table.cpp b/src/table.cpp
@@ -102,27 +102,27 @@ void table_t::start(std::string const &conninfo, std::string const &table_space)
     if (!m_append) {
         //define the new table
         auto sql =
-            "CREATE UNLOGGED TABLE {} (osm_id int8,"_format(qual_name);
+            fmt::format("CREATE UNLOGGED TABLE {} (osm_id int8,", qual_name);
 
         //first with the regular columns
         for (auto const &column : m_columns) {
             check_identifier(column.name, "column names");
             check_identifier(column.type_name, "column types");
-            sql += R"("{}" {},)"_format(column.name, column.type_name);
+            sql += fmt::format(R"("{}" {},)", column.name, column.type_name);
         }
 
         //then with the hstore columns
         for (auto const &hcolumn : m_hstore_columns) {
             check_identifier(hcolumn, "column names");
-            sql += R"("{}" hstore,)"_format(hcolumn);
+            sql += fmt::format(R"("{}" hstore,)", hcolumn);
         }
 
         //add tags column
         if (m_hstore_mode != hstore_column::none) {
             sql += "\"tags\" hstore,";
         }
 
-        sql += "way geometry({},{}) )"_format(m_type, m_srid);
+        sql += fmt::format("way geometry({},{}) )", m_type, m_srid);
 
         // The final tables are created with CREATE TABLE AS ... SELECT * FROM ...
         // This means that they won't get this autovacuum setting, so it doesn't
@@ -201,9 +201,8 @@ void table_t::stop(bool updateable, bool enable_hstore_index,
         // because they say nothing about the validity of the geometry in OSM.
         m_sql_conn->exec("SET client_min_messages = WARNING");
 
-        std::string sql =
-            "CREATE TABLE {} {} AS SELECT * FROM {}"_format(
-                qual_tmp_name, m_table_space, qual_name);
+        std::string sql = fmt::format("CREATE TABLE {} {} AS SELECT * FROM {}",
+                                      qual_tmp_name, m_table_space, qual_name);
 
         auto const postgis_version = get_postgis_version();
 
diff --git a/src/taginfo.cpp b/src/taginfo.cpp
@@ -91,7 +91,7 @@ bool read_style_file(std::string const &filename, export_list *exlist)
     if (!in) {
         throw std::system_error{
             errno, std::system_category(),
-            "Couldn't open style file '{}'"_format(filename)};
+            fmt::format("Couldn't open style file '{}'", filename)};
     }
 
     char buffer[1024];
@@ -179,7 +179,7 @@ bool read_style_file(std::string const &filename, export_list *exlist)
         std::fclose(in);
         throw std::system_error{
             err, std::system_category(),
-            "Error reading style file '{}'"_format(filename)};
+            fmt::format("Error reading style file '{}'", filename)};
     }
 
     std::fclose(in);
diff --git a/src/util.cpp b/src/util.cpp
@@ -36,16 +36,18 @@ std::string const &string_id_list_t::get()
 std::string human_readable_duration(uint64_t seconds)
 {
     if (seconds < 60) {
-        return "{}s"_format(seconds);
+        return fmt::format("{}s", seconds);
     }
 
     if (seconds < (60 * 60)) {
-        return "{}s ({}m {}s)"_format(seconds, seconds / 60, seconds % 60);
+        return fmt::format("{}s ({}m {}s)", seconds, seconds / 60,
+                           seconds % 60);
     }
 
     auto const secs = seconds % 60;
     auto const mins = seconds / 60;
-    return "{}s ({}h {}m {}s)"_format(seconds, mins / 60, mins % 60, secs);
+    return fmt::format("{}s ({}h {}m {}s)", seconds, mins / 60, mins % 60,
+                       secs);
 }
 
 std::string human_readable_duration(std::chrono::microseconds duration)
diff --git a/tests/common-buffer.hpp b/tests/common-buffer.hpp
@@ -44,12 +44,12 @@ class test_buffer_t
         std::string nodes;
 
         for (auto const id : ids) {
-            nodes += "n{},"_format(id);
+            nodes += fmt::format("n{},", id);
         }
 
         nodes.resize(nodes.size() - 1);
 
-        return add_way("w{} N{}"_format(wid, nodes));
+        return add_way(fmt::format("w{} N{}", wid, nodes));
     }
 
     osmium::Relation const &add_relation(std::string const &data)
diff --git a/tests/common-pg.hpp b/tests/common-pg.hpp
@@ -78,15 +78,16 @@ class conn_t : public pg_conn_t
 
     int get_count(char const *table_name, std::string const &where = "") const
     {
-        auto const query = "SELECT count(*) FROM {} {} {}"_format(
-            table_name, (where.empty() ? "" : "WHERE"), where);
+        auto const query =
+            fmt::format("SELECT count(*) FROM {} {} {}", table_name,
+                        (where.empty() ? "" : "WHERE"), where);
 
         return result_as_int(query);
     }
 
     void require_has_table(char const *table_name) const
     {
-        auto const where = "oid = '{}'::regclass"_format(table_name);
+        auto const where = fmt::format("oid = '{}'::regclass", table_name);
 
         REQUIRE(get_count("pg_catalog.pg_class", where) == 1);
     }
@@ -100,7 +101,8 @@ class tempdb_t
         try {
             conn_t conn{"dbname=postgres"};
 
-            m_db_name = "osm2pgsql-test-{}-{}"_format(getpid(), time(nullptr));
+            m_db_name =
+                fmt::format("osm2pgsql-test-{}-{}", getpid(), time(nullptr));
             conn.exec(R"(DROP DATABASE IF EXISTS "{}")", m_db_name);
             conn.exec(R"(CREATE DATABASE "{}" WITH ENCODING 'UTF8')",
                       m_db_name);
diff --git a/tests/test-db-copy-mgr.cpp b/tests/test-db-copy-mgr.cpp
@@ -192,7 +192,7 @@ TEST_CASE("copy_mgr_t")
 
         for (auto const &[k, v] : values) {
             auto const res = c.result_as_string(
-                "SELECT h->'{}' FROM test_copy_mgr"_format(k));
+                fmt::format("SELECT h->'{}' FROM test_copy_mgr", k));
             CHECK(res == v);
         }
     }
diff --git a/tests/test-middle.cpp b/tests/test-middle.cpp
diff --git a/tests/test-output-gazetteer.cpp b/tests/test-output-gazetteer.cpp
diff --git a/tests/test-output-pgsql-z_order.cpp b/tests/test-output-pgsql-z_order.cpp

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ flex_index_t::create_index(std::string const &qualified_table_name) const`
`49`	`49`
`50`	`50`	`if (m_fillfactor != 0) {`
`51`	`51`	`joiner.add("WITH");`
`52`		`- joiner.add("(fillfactor = {})"_format(m_fillfactor));`
	`52`	`+ joiner.add(fmt::format("(fillfactor = {})", m_fillfactor));`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`if (!m_tablespace.empty()) {`
Original file line number	Diff line number	Diff line change
`@@ -36,8 +36,8 @@ static void write_null(db_copy_mgr_t<db_deleter_by_type_and_id_t> *copy_mgr,`
`36`	`36`	`{`
`37`	`37`	`if (column.not_null()) {`
`38`	`38`	`throw not_null_exception{`
`39`		`- "Can not add NULL to column '{}' declared NOT NULL."_format(`
`40`		`- column.name()),`
	`39`	`+ fmt::format("Can not add NULL to column '{}' declared NOT NULL.",`
	`40`	`+ column.name()),`
`41`	`41`	`&column};`
`42`	`42`	`}`
`43`	`43`	`copy_mgr->add_null_column();`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ node_persistent_cache::node_persistent_cache(std::string file_name,`
`38`	`38`	`if (m_fd < 0) {`
`39`	`39`	`throw std::system_error{`
`40`	`40`	`errno, std::system_category(),`
`41`		`- "Unable to open flatnode file '{}'"_format(m_file_name)};`
	`41`	`+ fmt::format("Unable to open flatnode file '{}'", m_file_name)};`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`m_index = std::make_unique<index_t>(m_fd);`
Original file line number	Diff line number	Diff line change
`@@ -739,7 +739,7 @@ int output_flex_t::table_tostring()`
`739`	`739`	`{`
`740`	`740`	`auto const &table = get_table_from_param();`
`741`	`741`
`742`		`- std::string const str{"osm2pgsql.Table[{}]"_format(table.name())};`
	`742`	`+ std::string const str{fmt::format("osm2pgsql.Table[{}]", table.name())};`
`743`	`743`	`lua_pushstring(lua_state(), str.c_str());`
`744`	`744`
`745`	`745`	`return 1;`
Original file line number	Diff line number	Diff line change
`@@ -32,8 +32,8 @@ static void init_set_from_query(std::set<std::string> *set,`
`32`	`32`	`char const table, char const column,`
`33`	`33`	`char const *condition = "true")`
`34`	`34`	`{`
`35`		`- auto const res = db_connection.exec(`
`36`		`- "SELECT {} FROM {} WHERE {}"_format(column, table, condition));`
	`35`	`+ auto const res = db_connection.exec("SELECT {} FROM {} WHERE {}", column,`
	`36`	`+ table, condition);`
`37`	`37`	`for (int i = 0; i < res.num_tuples(); ++i) {`
`38`	`38`	`set->emplace(res.get(i, 0));`
`39`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,9 +30,9 @@ static std::string cps_display(std::size_t count, uint64_t elapsed)`
`30`	`30`	`double const cps = count_per_second(count, elapsed);`
`31`	`31`
`32`	`32`	`if (cps >= 1000.0) {`
`33`		`- return "{:.0f}k/s"_format(cps / 1000);`
	`33`	`+ return fmt::format("{:.0f}k/s", cps / 1000);`
`34`	`34`	`}`
`35`		`- return "{:.0f}/s"_format(cps);`
	`35`	`+ return fmt::format("{:.0f}/s", cps);`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`void progress_display_t::print_summary() const`