Skip to content

Commit 17a6721

Browse files
greenc-FNALknoepfelgithub-actions[bot]
authored
Fix "source" name issues for product_store (#121)
* Resolve issue with `string_view` to temporary `string` * Optional name for base product store (defaults to "Source") * Provide a source name for the product store * Remove unneeded default args per @knoepfel --------- Co-authored-by: Kyle Knoepfel <knoepfel@fnal.gov> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent f896b93 commit 17a6721

File tree

5 files changed

+40
-32
lines changed

5 files changed

+40
-32
lines changed

‎phlex/core/declared_unfold.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
namespace phlex::experimental {
99

1010
generator::generator(product_store_const_ptr const& parent,
11-
std::string const& node_name,
11+
std::string node_name,
1212
std::string const& new_level_name) :
1313
parent_{std::const_pointer_cast<product_store>(parent)},
14-
node_name_{node_name},
14+
node_name_{std::move(node_name)},
1515
new_level_name_{new_level_name}
1616
{
1717
}

‎phlex/core/declared_unfold.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace phlex::experimental {
3636
class generator {
3737
public:
3838
explicit generator(product_store_const_ptr const& parent,
39-
std::string const& node_name,
39+
std::string node_name,
4040
std::string const& new_level_name);
4141
product_store_const_ptr flush_store() const;
4242

@@ -48,7 +48,7 @@ namespace phlex::experimental {
4848
private:
4949
product_store_const_ptr make_child(std::size_t i, products new_products);
5050
product_store_ptr parent_;
51-
std::string_view node_name_;
51+
std::string node_name_;
5252
std::string const& new_level_name_;
5353
std::map<level_id::hash_type, std::size_t> child_counts_;
5454
};

‎phlex/model/product_store.cpp‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,45 +8,49 @@ namespace phlex::experimental {
88

99
product_store::product_store(product_store_const_ptr parent,
1010
level_id_ptr id,
11-
std::string_view source,
11+
std::string source,
1212
stage processing_stage,
1313
products new_products) :
1414
parent_{std::move(parent)},
1515
products_{std::move(new_products)},
1616
id_{std::move(id)},
17-
source_{source},
17+
source_{std::move(source)},
1818
stage_{processing_stage}
1919
{
2020
}
2121

2222
product_store::product_store(product_store_const_ptr parent,
2323
std::size_t new_level_number,
2424
std::string const& new_level_name,
25-
std::string_view source,
25+
std::string source,
2626
products new_products) :
2727
parent_{parent},
2828
products_{std::move(new_products)},
2929
id_{parent->id()->make_child(new_level_number, new_level_name)},
30-
source_{source},
30+
source_{std::move(source)},
3131
stage_{stage::process}
3232
{
3333
}
3434

3535
product_store::product_store(product_store_const_ptr parent,
3636
std::size_t new_level_number,
3737
std::string const& new_level_name,
38-
std::string_view source,
38+
std::string source,
3939
stage processing_stage) :
4040
parent_{parent},
4141
id_{parent->id()->make_child(new_level_number, new_level_name)},
42-
source_{source},
42+
source_{std::move(source)},
4343
stage_{processing_stage}
4444
{
4545
}
4646

4747
product_store::~product_store() = default;
4848

49-
product_store_ptr product_store::base() { return product_store_ptr{new product_store}; }
49+
product_store_ptr product_store::base(std::string base_name)
50+
{
51+
return product_store_ptr{
52+
new product_store{nullptr, level_id::base_ptr(), std::move(base_name)}};
53+
}
5054

5155
product_store_const_ptr product_store::parent(std::string const& level_name) const noexcept
5256
{
@@ -77,33 +81,36 @@ namespace phlex::experimental {
7781
return product_store_ptr{new product_store{parent_, id_, "[inserted]", stage::flush}};
7882
}
7983

80-
product_store_ptr product_store::make_continuation(std::string_view source,
84+
product_store_ptr product_store::make_continuation(std::string source,
8185
products new_products) const
8286
{
8387
return product_store_ptr{
84-
new product_store{parent_, id_, source, stage::process, std::move(new_products)}};
88+
new product_store{parent_, id_, std::move(source), stage::process, std::move(new_products)}};
8589
}
8690

8791
product_store_ptr product_store::make_child(std::size_t new_level_number,
8892
std::string const& new_level_name,
89-
std::string_view source,
93+
std::string source,
9094
products new_products)
9195
{
92-
return product_store_ptr{new product_store{
93-
shared_from_this(), new_level_number, new_level_name, source, std::move(new_products)}};
96+
return product_store_ptr{new product_store{shared_from_this(),
97+
new_level_number,
98+
new_level_name,
99+
std::move(source),
100+
std::move(new_products)}};
94101
}
95102

96103
product_store_ptr product_store::make_child(std::size_t new_level_number,
97104
std::string const& new_level_name,
98-
std::string_view source,
105+
std::string source,
99106
stage processing_stage)
100107
{
101108
return product_store_ptr{new product_store{
102-
shared_from_this(), new_level_number, new_level_name, source, processing_stage}};
109+
shared_from_this(), new_level_number, new_level_name, std::move(source), processing_stage}};
103110
}
104111

105112
std::string const& product_store::level_name() const noexcept { return id_->level_name(); }
106-
std::string_view product_store::source() const noexcept { return source_; }
113+
std::string const& product_store::source() const noexcept { return source_; }
107114
product_store_const_ptr product_store::parent() const noexcept { return parent_; }
108115
level_id_ptr const& product_store::id() const noexcept { return id_; }
109116
bool product_store::is_flush() const noexcept { return stage_ == stage::flush; }

‎phlex/model/product_store.hpp‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,26 @@ namespace phlex::experimental {
1717
class product_store : public std::enable_shared_from_this<product_store> {
1818
public:
1919
~product_store();
20-
static product_store_ptr base();
20+
static product_store_ptr base(std::string base_name = "Source");
2121

2222
product_store_const_ptr store_for_product(std::string const& product_name) const;
2323

2424
auto begin() const noexcept { return products_.begin(); }
2525
auto end() const noexcept { return products_.end(); }
2626

2727
std::string const& level_name() const noexcept;
28-
std::string_view source() const noexcept; // FIXME: Think carefully of using std::string_view
28+
std::string const& source() const noexcept;
2929
product_store_const_ptr parent(std::string const& level_name) const noexcept;
3030
product_store_const_ptr parent() const noexcept;
3131
product_store_ptr make_flush() const;
32-
product_store_ptr make_continuation(std::string_view source, products new_products = {}) const;
32+
product_store_ptr make_continuation(std::string source, products new_products = {}) const;
3333
product_store_ptr make_child(std::size_t new_level_number,
3434
std::string const& new_level_name,
35-
std::string_view source,
35+
std::string source,
3636
products new_products);
3737
product_store_ptr make_child(std::size_t new_level_number,
3838
std::string const& new_level_name,
39-
std::string_view source = {},
39+
std::string source = {},
4040
stage st = stage::process);
4141
level_id_ptr const& id() const noexcept;
4242
bool is_flush() const noexcept;
@@ -58,26 +58,27 @@ namespace phlex::experimental {
5858
void add_product(std::string const& key, std::unique_ptr<product<T>>&& t);
5959

6060
private:
61-
explicit product_store(product_store_const_ptr parent = nullptr,
62-
level_id_ptr id = level_id::base_ptr(),
63-
std::string_view source = {},
61+
explicit product_store(product_store_const_ptr parent,
62+
level_id_ptr id,
63+
std::string source,
6464
stage processing_stage = stage::process,
6565
products new_products = {});
6666
explicit product_store(product_store_const_ptr parent,
6767
std::size_t new_level_number,
6868
std::string const& new_level_name,
69-
std::string_view source,
69+
std::string source,
7070
products new_products);
7171
explicit product_store(product_store_const_ptr parent,
7272
std::size_t new_level_number,
7373
std::string const& new_level_name,
74-
std::string_view source,
74+
std::string source,
7575
stage processing_stage);
7676

7777
product_store_const_ptr parent_{nullptr};
7878
products products_{};
7979
level_id_ptr id_;
80-
std::string_view source_;
80+
std::string
81+
source_; // FIXME: Should not have to copy the string (the source should outlive the product store)
8182
stage stage_;
8283
};
8384

‎test/hierarchical_nodes.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ namespace {
4343
auto job_store = product_store::base();
4444
driver.yield(job_store);
4545
for (unsigned i : std::views::iota(0u, index_limit)) {
46-
auto run_store = job_store->make_child(i, "run");
46+
auto run_store = job_store->make_child(i, "run", "levels_to_process");
4747
run_store->add_product<std::time_t>("time", std::time(nullptr));
4848
driver.yield(run_store);
4949
for (unsigned j : std::views::iota(0u, number_limit)) {
50-
auto event_store = run_store->make_child(j, "event");
50+
auto event_store = run_store->make_child(j, "event", "levels_to_process");
5151
event_store->add_product("number", i + j);
5252
driver.yield(event_store);
5353
}

0 commit comments

Comments
 (0)