Skip to content

Commit

Permalink
refactor: Fix unnecessary usages of char* (VowpalWabbit#3085)
Browse files Browse the repository at this point in the history
* refactor: Fix unnecessary usages of char*

* Formatting
  • Loading branch information
jackgerrits authored Jun 19, 2021
1 parent 5c8fd55 commit c7ff603
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 86 deletions.
24 changes: 12 additions & 12 deletions test/unit_test/ccb_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ BOOST_AUTO_TEST_CASE(ccb_explicit_included_actions_no_overlap)
{
auto& vw = *VW::initialize("--ccb_explore_adf --quiet");
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("ccb shared |")));
examples.push_back(VW::read_example(vw, std::string("ccb action |")));
examples.push_back(VW::read_example(vw, std::string("ccb action |")));
examples.push_back(VW::read_example(vw, std::string("ccb action |")));
examples.push_back(VW::read_example(vw, std::string("ccb action |")));
examples.push_back(VW::read_example(vw, std::string("ccb slot 0 |")));
examples.push_back(VW::read_example(vw, std::string("ccb slot 3 |")));
examples.push_back(VW::read_example(vw, std::string("ccb slot 1 |")));
examples.push_back(VW::read_example(vw, "ccb shared |"));
examples.push_back(VW::read_example(vw, "ccb action |"));
examples.push_back(VW::read_example(vw, "ccb action |"));
examples.push_back(VW::read_example(vw, "ccb action |"));
examples.push_back(VW::read_example(vw, "ccb action |"));
examples.push_back(VW::read_example(vw, "ccb slot 0 |"));
examples.push_back(VW::read_example(vw, "ccb slot 3 |"));
examples.push_back(VW::read_example(vw, "ccb slot 1 |"));

vw.predict(examples);

Expand Down Expand Up @@ -95,10 +95,10 @@ BOOST_AUTO_TEST_CASE(ccb_invalid_example_checks)
{
auto& vw = *VW::initialize("--ccb_explore_adf --quiet");
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("ccb shared |")));
examples.push_back(VW::read_example(vw, std::string("ccb action |")));
examples.push_back(VW::read_example(vw, std::string("ccb slot 0 |")));
examples.push_back(VW::read_example(vw, std::string("ccb slot 3 |")));
examples.push_back(VW::read_example(vw, "ccb shared |"));
examples.push_back(VW::read_example(vw, "ccb action |"));
examples.push_back(VW::read_example(vw, "ccb slot 0 |"));
examples.push_back(VW::read_example(vw, "ccb slot 3 |"));

for (auto* example : examples) { VW::setup_example(vw, example); }

Expand Down
2 changes: 1 addition & 1 deletion test/unit_test/chain_hashing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(chain_hashing_between_formats)
multi_ex examples;
examples.push_back(&VW::get_unused_example(vw));
auto example = examples[0];
VW::read_line(*vw, example, const_cast<char*>(text.c_str()));
VW::read_line(*vw, example, text.c_str());
auto& indices = example->feature_space['f'].indicies;
txt_idx = indices[0];
VW::finish_example(*vw, examples);
Expand Down
14 changes: 7 additions & 7 deletions test/unit_test/example_header_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
BOOST_AUTO_TEST_CASE(is_example_header_cb) {
auto& vw = *VW::initialize("--cb_explore_adf --quiet", nullptr, false, nullptr, nullptr);
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("shared | s_1 s_2")));
examples.push_back(VW::read_example(vw, std::string("0:1.0:0.5 | a:1 b:1 c:1")));
examples.push_back(VW::read_example(vw, std::string("| a:0.5 b:2 c:1")));
examples.push_back(VW::read_example(vw, "shared | s_1 s_2"));
examples.push_back(VW::read_example(vw, "0:1.0:0.5 | a:1 b:1 c:1"));
examples.push_back(VW::read_example(vw, "| a:0.5 b:2 c:1"));

BOOST_CHECK_EQUAL(CB::ec_is_example_header(*examples[0]), true);
BOOST_CHECK_EQUAL(COST_SENSITIVE::ec_is_example_header(*examples[0]), false);
Expand All @@ -32,8 +32,8 @@ BOOST_AUTO_TEST_CASE(is_example_header_cb) {
BOOST_AUTO_TEST_CASE(is_example_header_ccb) {
auto& vw = *VW::initialize("--ccb_explore_adf --quiet", nullptr, false, nullptr, nullptr);
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("ccb shared |User f")));
examples.push_back(VW::read_example(vw, std::string("ccb action |Action f")));
examples.push_back(VW::read_example(vw, "ccb shared |User f"));
examples.push_back(VW::read_example(vw, "ccb action |Action f"));

BOOST_CHECK_EQUAL(CCB::ec_is_example_header(*examples[0]), true);
BOOST_CHECK_EQUAL(CCB::ec_is_example_header(*examples[1]), false);
Expand All @@ -44,8 +44,8 @@ BOOST_AUTO_TEST_CASE(is_example_header_ccb) {
BOOST_AUTO_TEST_CASE(is_example_header_csoaa) {
auto& vw = *VW::initialize("--csoaa_ldf multiline --quiet", nullptr, false, nullptr, nullptr);
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("shared | a_2 b_2 c_2")));
examples.push_back(VW::read_example(vw, std::string("3:2.0 | a_3 b_3 c_3")));
examples.push_back(VW::read_example(vw, "shared | a_2 b_2 c_2"));
examples.push_back(VW::read_example(vw, "3:2.0 | a_3 b_3 c_3"));

BOOST_CHECK_EQUAL(CB::ec_is_example_header(*examples[0]), false);
BOOST_CHECK_EQUAL(COST_SENSITIVE::ec_is_example_header(*examples[0]), true);
Expand Down
4 changes: 2 additions & 2 deletions test/unit_test/interactions_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ inline void noop_func(float& unused_dat, const float ft_weight, const uint64_t f
BOOST_AUTO_TEST_CASE(eval_count_of_generated_ft_test)
{
auto& vw = *VW::initialize("--quiet -q :: --noconstant", nullptr, false, nullptr, nullptr);
auto* ex = VW::read_example(vw, std::string("3 |f a b c |e x y z"));
auto* ex = VW::read_example(vw, "3 |f a b c |e x y z");

size_t naive_features_count;
float naive_features_value;
Expand Down Expand Up @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(eval_count_of_generated_ft_permuations_test)
{
auto& vw = *VW::initialize(
"--quiet -q :: --leave_duplicate_interactions --permutations --noconstant", nullptr, false, nullptr, nullptr);
auto* ex = VW::read_example(vw, std::string("3 |f a b c |e x y z"));
auto* ex = VW::read_example(vw, "3 |f a b c |e x y z");

auto interactions =
INTERACTIONS::compile_interactions<INTERACTIONS::generate_namespace_permutations_with_repetition, true>(
Expand Down
10 changes: 5 additions & 5 deletions test/unit_test/prediction_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ BOOST_AUTO_TEST_CASE(predict_modifying_state)
float prediction_one;
{
auto& vw = *VW::initialize("--quiet --sgd --noconstant --learning_rate 0.1");
auto& pre_learn_predict_example = *VW::read_example(vw, std::string("0.19574759682114784 | 1:1.430"));
auto& learn_example = *VW::read_example(vw, std::string("0.19574759682114784 | 1:1.430"));
auto& predict_example = *VW::read_example(vw, std::string("| 1:1.0"));
auto& pre_learn_predict_example = *VW::read_example(vw, "0.19574759682114784 | 1:1.430");
auto& learn_example = *VW::read_example(vw, "0.19574759682114784 | 1:1.430");
auto& predict_example = *VW::read_example(vw, "| 1:1.0");

vw.predict(pre_learn_predict_example);
vw.finish_example(pre_learn_predict_example);
Expand All @@ -31,8 +31,8 @@ BOOST_AUTO_TEST_CASE(predict_modifying_state)
{
auto& vw = *VW::initialize("--quiet --sgd --noconstant --learning_rate 0.1");

auto& learn_example = *VW::read_example(vw, std::string("0.19574759682114784 | 1:1.430"));
auto& predict_example = *VW::read_example(vw, std::string("| 1:1.0"));
auto& learn_example = *VW::read_example(vw, "0.19574759682114784 | 1:1.430");
auto& predict_example = *VW::read_example(vw, "| 1:1.0");

vw.learn(learn_example);
vw.finish_example(learn_example);
Expand Down
12 changes: 6 additions & 6 deletions test/unit_test/slates_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ BOOST_AUTO_TEST_CASE(slates_reduction_mock_test)
{
auto& vw = *VW::initialize("--slates --quiet");
multi_ex examples;
examples.push_back(VW::read_example(vw, std::string("slates shared 0.8 | ignore_me")));
examples.push_back(VW::read_example(vw, std::string("slates action 0 | ignore_me")));
examples.push_back(VW::read_example(vw, std::string("slates action 1 | ignore_me")));
examples.push_back(VW::read_example(vw, std::string("slates action 1 | ignore_me")));
examples.push_back(VW::read_example(vw, std::string("slates slot 0:0.8 | ignore_me")));
examples.push_back(VW::read_example(vw, std::string("slates slot 1:0.6 | ignore_me")));
examples.push_back(VW::read_example(vw, "slates shared 0.8 | ignore_me"));
examples.push_back(VW::read_example(vw, "slates action 0 | ignore_me"));
examples.push_back(VW::read_example(vw, "slates action 1 | ignore_me"));
examples.push_back(VW::read_example(vw, "slates action 1 | ignore_me"));
examples.push_back(VW::read_example(vw, "slates slot 0:0.8 | ignore_me"));
examples.push_back(VW::read_example(vw, "slates slot 1:0.6 | ignore_me"));

auto mock_learn_or_pred = [](multi_ex& examples)
{
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/json_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ struct Namespace

void AddFeature(vw* all, const char* str)
{
ftrs->push_back(1., VW::hash_feature_cstr(*all, const_cast<char*>(str), namespace_hash));
ftrs->push_back(1., VW::hash_feature_cstr(*all, str, namespace_hash));
feature_count++;

if (audit) ftrs->space_names.push_back(audit_strings_ptr(new audit_strings(name, str)));
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/parse_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ void read_line(vw& all, example* ex, VW::string_view line)
substring_to_example(&all, ex, line);
}

void read_line(vw& all, example* ex, char* line) { return read_line(all, ex, VW::string_view(line)); }
void read_line(vw& all, example* ex, const char* line) { return read_line(all, ex, VW::string_view(line)); }

void read_lines(vw* all, const char* line, size_t /*len*/, v_array<example*>& examples)
{
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/parse_example.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void substring_to_example(vw* all, example* ae, VW::string_view example);
namespace VW
{
example& get_unused_example(vw* all);
void read_line(vw& all, example* ex, char* line); // read example from the line.
void read_line(vw& all, example* ex, const char* line); // read example from the line.
void read_lines(vw* all, const char* line, size_t len,
v_array<example*>& examples); // read examples from the new line separated strings.

Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/parse_example_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ class DefaultState : public BaseState<audit>
BaseState<audit>* Float(Context<audit>& ctx, float f) override
{
auto& ns = ctx.CurrentNamespace();
ns.AddFeature(f, VW::hash_feature_cstr(*ctx.all, const_cast<char*>(ctx.key), ns.namespace_hash), ctx.key);
ns.AddFeature(f, VW::hash_feature_cstr(*ctx.all, ctx.key, ns.namespace_hash), ctx.key);

return this;
}
Expand Down
5 changes: 2 additions & 3 deletions vowpalwabbit/parse_slates_example_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,8 @@ void handle_features_value(const char* key_namespace, const Value& value, exampl
{
assert(!namespaces.empty());
float number = get_number(value);
namespaces.back().AddFeature(number,
VW::hash_feature_cstr(all, const_cast<char*>(key_namespace), namespaces.back().namespace_hash),
key_namespace);
namespaces.back().AddFeature(
number, VW::hash_feature_cstr(all, key_namespace, namespaces.back().namespace_hash), key_namespace);
}
break;
default:
Expand Down
7 changes: 2 additions & 5 deletions vowpalwabbit/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ example* new_unused_example(vw& all)
ec->example_counter = static_cast<size_t>(all.example_parser->begin_parsed_examples.load());
return ec;
}
example* read_example(vw& all, char* example_line)
example* read_example(vw& all, const char* example_line)
{
example* ret = &get_unused_example(&all);

Expand All @@ -762,10 +762,7 @@ example* read_example(vw& all, char* example_line)
return ret;
}

example* read_example(vw& all, std::string example_line)
{
return read_example(all, const_cast<char*>(example_line.c_str()));
}
example* read_example(vw& all, const std::string& example_line) { return read_example(all, example_line.c_str()); }

void add_constant_feature(vw& vw, example* ec)
{
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/slim/include/example_predict_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ class example_predict_builder
void add_namespace(namespace_index feature_group);

public:
example_predict_builder(example_predict* ex, char* namespace_name, uint32_t feature_index_num_bits = 18);
example_predict_builder(example_predict* ex, const char* namespace_name, uint32_t feature_index_num_bits = 18);
example_predict_builder(example_predict* ex, namespace_index namespace_idx, uint32_t feature_index_num_bits = 18);

void push_feature_string(char* feature_idx, feature_value value);
void push_feature_string(const char* feature_idx, feature_value value);
void push_feature(feature_index feature_idx, feature_value value);
};
} // namespace vw_slim
4 changes: 2 additions & 2 deletions vowpalwabbit/slim/src/example_predict_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace vw_slim
{
example_predict_builder::example_predict_builder(
example_predict* ex, char* namespace_name, uint32_t feature_index_num_bits)
example_predict* ex, const char* namespace_name, uint32_t feature_index_num_bits)
: _ex(ex)
{
_feature_index_bit_mask = ((uint64_t)1 << feature_index_num_bits) - 1;
Expand All @@ -27,7 +27,7 @@ void example_predict_builder::add_namespace(namespace_index feature_group)
_ex->indices.unique_add_sorted(feature_group);
}

void example_predict_builder::push_feature_string(char* feature_name, feature_value value)
void example_predict_builder::push_feature_string(const char* feature_name, feature_value value)
{
feature_index feature_hash =
_feature_index_bit_mask & hashstring(feature_name, strlen(feature_name), _namespace_hash);
Expand Down
Loading

0 comments on commit c7ff603

Please sign in to comment.