From 0e392f58c6e44d886a188f882c99e9dd0618c16c Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Tue, 15 Dec 2020 16:24:31 +0200 Subject: [PATCH] s3select:clang-tidying of s3select_oper.h ... and some undoing of formatting changes Signed-off-by: Ronen Friedman --- include/s3select.h | 13 ++-- include/s3select_functions.h | 21 +++--- include/s3select_oper.h | 124 ++++++++++++++++++----------------- test/s3select_test.cpp | 1 - 4 files changed, 79 insertions(+), 80 deletions(-) diff --git a/include/s3select.h b/include/s3select.h index e8474717..e7450478 100644 --- a/include/s3select.h +++ b/include/s3select.h @@ -15,7 +15,6 @@ #include #include -using namespace std::string_literals; #define _DEBUG_TERM {string token(a,b);std::cout << __FUNCTION__ << token << std::endl;} @@ -732,23 +731,23 @@ void push_compare_operator::operator()(s3select* self, const char* a, const char std::string token(a, b); arithmetic_operand::cmp_t c; - if (token =="=="s) + if (token =="==") { c = arithmetic_operand::cmp_t::EQ; } - else if (token =="!="s) + else if (token =="!=") { c = arithmetic_operand::cmp_t::NE; } - else if (token ==">="s) + else if (token ==">=") { c = arithmetic_operand::cmp_t::GE; } - else if (token == "<="s) + else if (token == "<=") { c = arithmetic_operand::cmp_t::LE; } - else if (token == ">"s) + else if (token == ">") { c = arithmetic_operand::cmp_t::GT; } @@ -840,7 +839,7 @@ void push_negation::operator()(s3select* self, const char* a, const char* b) con //upon NOT operator, the logical and arithmetical operators are "tagged" to negate result. if (dynamic_cast(pred)) { - logical_operand* f = S3SELECT_NEW(self, logical_operand, pred); // todo: marked as "empty statemant" + logical_operand* f = S3SELECT_NEW(self, logical_operand, pred); self->getAction()->condQ.push_back(f); } else if (dynamic_cast<__function*>(pred) || dynamic_cast(pred)) diff --git a/include/s3select_functions.h b/include/s3select_functions.h index 84df2a5c..940d2c92 100644 --- a/include/s3select_functions.h +++ b/include/s3select_functions.h @@ -7,7 +7,6 @@ #include using namespace std::string_literals; - #define BOOST_BIND_ACTION_PARAM( push_name ,param ) boost::bind( &push_name::operator(), g_ ## push_name , _1 ,_2, param) namespace s3selectEngine { @@ -349,7 +348,7 @@ struct _fn_avg : public base_function return true; } - void get_aggregate_result(variable *result) override + void get_aggregate_result(variable *result) override { if(count == 0) { throw base_s3select_exception("count cannot be zero!"); @@ -381,7 +380,7 @@ struct _fn_min : public base_function return true; } - void get_aggregate_result(variable* result) override + void get_aggregate_result(variable* result) override { *result = min; } @@ -411,7 +410,7 @@ struct _fn_max : public base_function return true; } - void get_aggregate_result(variable* result) override + void get_aggregate_result(variable* result) override { *result = max; } @@ -437,13 +436,13 @@ struct _fn_to_int : public base_function if ((errno == ERANGE && (i == LONG_MAX || i == LONG_MIN)) || (errno != 0 && i == 0)) { throw base_s3select_exception("converted value would fall out of the range of the result type!"); return false; - } + } - if (*perr != '\0') { - throw base_s3select_exception("characters after int!"); - return false; + if (*perr != '\0') { + throw base_s3select_exception("characters after int!"); + return false; + } } - } else if (func_arg.type == value::value_En_t::FLOAT) { i = func_arg.dbl(); @@ -488,8 +487,8 @@ struct _fn_to_float : public base_function } var_result = d; - break; } + break; case value::value_En_t::FLOAT: var_result = v.dbl(); @@ -586,7 +585,7 @@ struct _fn_to_timestamp : public base_function bsc::parse_info<> info_dig = bsc::parse(v_str.str(), d_yyyymmdd_dig >> *(separator) >> d_time_dig); - if(!datetime_validation() or !info_dig.full) + if(!datetime_validation() || !info_dig.full) { throw base_s3select_exception("input date-time is illegal"); } diff --git a/include/s3select_oper.h b/include/s3select_oper.h index 9e8e35e9..efd0114b 100644 --- a/include/s3select_oper.h +++ b/include/s3select_oper.h @@ -31,7 +31,6 @@ class ChunkAllocator : public std::allocator public: typedef size_t size_type; typedef T* pointer; - //typedef const T* const_pointer; size_t buffer_capacity; char* buffer_ptr; @@ -65,7 +64,7 @@ class ChunkAllocator : public std::allocator } //================================== - inline pointer allocate(size_type n, const void* hint = 0) // todo remove as both hides non-virtual and unused + inline pointer allocate(size_type n, [[maybe_unused]] const void* hint = 0) // todo remove as both hides non-virtual and unused { return (_Allocate(n, (pointer)0)); } @@ -278,18 +277,18 @@ class s3select_reserved_word using reserved_words = std::map; - const reserved_words m_reserved_words= + inline static const reserved_words m_reserved_words= { {"null",reserve_word_en_t::S3S_NULL},{"NULL",reserve_word_en_t::S3S_NULL}, {"nan",reserve_word_en_t::S3S_NAN},{"NaN",reserve_word_en_t::S3S_NAN} }; - bool is_reserved_word(std::string & token) + static bool is_reserved_word(std::string & token) { return m_reserved_words.find(token) != m_reserved_words.end() ; } - reserve_word_en_t get_reserved_word(std::string & token) + static reserve_word_en_t get_reserved_word(std::string & token) // todo consider returning std::optional { if (is_reserved_word(token)) { @@ -336,16 +335,17 @@ class projection_alias return true; } + // todo consider returning std::optional base_statement* search_alias(const std::string&& alias_name) const { for(const auto& alias: alias_map) { if(alias.first == alias_name) { - return alias.second; //refernce to execution node + return alias.second; //reference to execution node } } - return 0; + return nullptr; } }; @@ -544,11 +544,11 @@ class value m_to_string.assign( __val.str ); } - return std::string( m_to_string.c_str() ); + return std::string( m_to_string ); } - value& operator=(value& o) + value& operator=(const value& o) { if(o.type == value_En_t::STRING) { @@ -606,17 +606,17 @@ class value return *this; } - int64_t i64() + int64_t i64() const { return __val.num; } - const char* str() + const char* str() const { return __val.str; } - double dbl() + double dbl() const { return __val.dbl; } @@ -626,7 +626,7 @@ class value return __val.timestamp; } - bool operator<(const value& v)//basic compare operator , most itensive runtime operation + bool operator<(const value& v) const//basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -675,7 +675,7 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator>(const value& v) //basic compare operator , most itensive runtime operation + bool operator>(const value& v) const //basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -724,7 +724,7 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator==(const value& v) //basic compare operator , most itensive runtime operation + bool operator==(const value& v) const //basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -773,31 +773,32 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator<=(const value& v) + + bool operator<=(const value& v) const { - if ((is_null() || v.is_null()) || (is_nan() || v.is_nan())) { + if (is_null() || v.is_null() || is_nan() || v.is_nan()) { return false; - } else { - return !(*this>v); - } + } + + return !(*this > v); } - bool operator>=(const value& v) + bool operator>=(const value& v) const { - if ((is_null() || v.is_null()) || (is_nan() || v.is_nan())) { + if (is_null() || v.is_null() || is_nan() || v.is_nan()) { return false; - } else { - return !(*this //conversion rules for arithmetical binary operations @@ -846,7 +847,6 @@ class value if ((l.is_null() || r.is_null()) || (l.is_nan() || r.is_nan())) { l.set_nan(); - return l; } return l; @@ -912,15 +912,15 @@ class base_statement int m_eval_stack_depth; public: - base_statement():m_scratch(0), is_last_call(false), m_is_cache_result(false), m_projection_alias(0), m_eval_stack_depth(0) {} + base_statement():m_scratch(nullptr), is_last_call(false), m_is_cache_result(false), m_projection_alias(nullptr), m_eval_stack_depth(0) {} virtual value& eval() =0; virtual base_statement* left() const { - return 0; + return nullptr; } virtual base_statement* right() const { - return 0; + return nullptr; } virtual std::string print(int ident) =0;//TODO complete it, one option to use level parametr in interface , virtual bool semantic() =0;//done once , post syntax , traverse all nodes and validate semantics. @@ -950,11 +950,10 @@ class base_statement bool is_function() const; - //bool is_aggregate_exist_in_expression(const base_statement* e) const;//TODO obsolete ? bool is_aggregate_exist_in_expression() const;//TODO obsolete ? const base_statement* get_aggregate() const; - //bool is_nested_aggregate(base_statement* e) const; + bool is_nested_aggregate() const; bool is_binop_aggregate_and_column(const base_statement* skip) const; @@ -1078,7 +1077,7 @@ class variable : public base_statement } } - variable(s3select_reserved_word::reserve_word_en_t reserve_word) + explicit variable(s3select_reserved_word::reserve_word_en_t reserve_word) { if (reserve_word == s3select_reserved_word::reserve_word_en_t::S3S_NULL) { @@ -1100,9 +1099,10 @@ class variable : public base_statement } } - void operator=(value& v) + variable& operator=(value& v) { var_value = v; + return *this; } void set_value(const char* s) @@ -1190,7 +1190,7 @@ class variable : public base_statement return var_value; } - value& eval() override + value& eval() override { if (m_var_type == var_t::COL_VALUE) { @@ -1218,7 +1218,7 @@ class variable : public base_statement //not enter this scope again column_pos = column_alias; - if(m_projection_alias == 0) + if(m_projection_alias == nullptr) { throw base_s3select_exception(std::string("alias {")+_name+std::string("} or column not exist in schema"), base_s3select_exception::s3select_exp_en_t::FATAL); } @@ -1253,14 +1253,14 @@ class variable : public base_statement return var_value; } - std::string print(int ident) override + std::string print(int ident) override { //std::string out = std::string(ident,' ') + std::string("var:") + std::to_string(var_value.__val.num); //return out; return std::string("#");//TBD } - bool semantic() override + bool semantic() override { return false; } @@ -1284,28 +1284,28 @@ class arithmetic_operand : public base_statement public: - bool semantic() override + bool semantic() override { return true; } - base_statement* left() const override + base_statement* left() const override { return l; } - base_statement* right() const override + base_statement* right() const override { return r; } - std::string print(int ident) override + std::string print(int ident) override { //std::string out = std::string(ident,' ') + "compare:" += std::to_string(_cmp) + "\n" + l->print(ident-5) +r->print(ident+5); //return out; return std::string("#");//TBD } - value& eval() override + value& eval() override { switch (_cmp) @@ -1398,7 +1398,7 @@ class logical_operand : public base_statement virtual ~logical_operand() {} - std::string print(int ident) override + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "logical_operand:" += std::to_string(_oplog) + "\n" + l->print(ident - 5) + r->print(ident + 5); //return out; @@ -1462,28 +1462,29 @@ class mulldiv_operation : public base_statement public: - base_statement* left() const override + base_statement* left() const override { return l; } - base_statement* right() const override + + base_statement* right() const override { return r; } - bool semantic() override + bool semantic() override { return true; } - std::string print(int ident) override + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "mulldiv_operation:" += std::to_string(_mulldiv) + "\n" + l->print(ident - 5) + r->print(ident + 5); //return out; return std::string("#");//TBD } - value& eval() override + value& eval() override { switch (_mulldiv) { @@ -1535,16 +1536,17 @@ class addsub_operation : public base_statement public: - base_statement* left() const override + base_statement* left() const override { return l; } - base_statement* right() const override + + base_statement* right() const override { return r; } - bool semantic() override + bool semantic() override { return true; } @@ -1553,13 +1555,13 @@ class addsub_operation : public base_statement virtual ~addsub_operation() = default; - std::string print(int ident) override + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "addsub_operation:" += std::to_string(_op) + "\n" + l->print(ident - 5) + r->print(ident + 5); return std::string("#");//TBD } - value& eval() override + value& eval() override { if (_op == addsub_op_t::NA) // -num , +num , unary-operation on number { @@ -1598,12 +1600,12 @@ class negate_function_operation : public base_statement explicit negate_function_operation(base_statement *f):function_to_negate(f){} - std::string print(int ident) override + std::string print(int ident) override { return std::string("#");//TBD } - bool semantic() override + bool semantic() override { return true; } @@ -1613,7 +1615,7 @@ class negate_function_operation : public base_statement return function_to_negate; } - value& eval() override + value& eval() override { res = function_to_negate->eval(); diff --git a/test/s3select_test.cpp b/test/s3select_test.cpp index 5faf8ec6..b68bf89d 100644 --- a/test/s3select_test.cpp +++ b/test/s3select_test.cpp @@ -1,7 +1,6 @@ #include "s3select.h" #include "gtest/gtest.h" #include -//#include "boost/date_time/gregorian/gregorian.hpp" #include "boost/date_time/posix_time/posix_time.hpp" using namespace s3selectEngine;