diff --git a/CHANGES b/CHANGES index 018b7385a6..f4833caea4 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,37 @@ +3.2.0-dev.157 | 2020-02-26 10:04:32 -0800 + + * IntrusivePtr: overload std::swap() (Max Kellermann) + + * IntrusivePtr: eliminate setPtr() (Max Kellermann) + + There are only two call sites, and those hard-code the `add_ref` + parameter. + + * IntrusivePtr: optimize release() using std::exchange() (Max Kellermann) + + * Expr: remove unused default constructors and `friend` declarations (Max Kellermann) + + * Remove useless override: RecordAssignExpr::Eval() (Max Kellermann) + + * Type: don't pass reference to pointer to MatchesIndex() (Max Kellermann) + + No implementation modifies the pointer value. To guard against this, + this commit changes `&` to `const`. + + * Expr: don't pass reference to pointer to check_and_promote*() (Max Kellermann) + + The function never modifies the pointer value. + + check_and_promote_expr() is left untouched because it really does + modify the pointer. + + * ID: remove unused default constructor (Max Kellermann) + + * Scope: simplify PList access, use pop_back() and back() (Max Kellermann) + + * List: add method empty() (Max Kellermann) + 3.2.0-dev.146 | 2020-02-26 09:40:18 -0800 * Type: fix double free bug in SetType::ShallowClone() (Max Kellermann) diff --git a/VERSION b/VERSION index 4314331ede..c39a045b4a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.146 +3.2.0-dev.157 diff --git a/src/Expr.cc b/src/Expr.cc index fb2583fc6c..b1f43c5536 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -5195,7 +5195,7 @@ int check_and_promote_expr(Expr*& e, BroType* t) return 1; } -int check_and_promote_exprs(ListExpr*& elements, TypeList* types) +int check_and_promote_exprs(ListExpr* const elements, TypeList* types) { expr_list& el = elements->Exprs(); const type_list* tl = types->Types(); @@ -5225,7 +5225,7 @@ int check_and_promote_exprs(ListExpr*& elements, TypeList* types) return 1; } -int check_and_promote_args(ListExpr*& args, RecordType* types) +int check_and_promote_args(ListExpr* const args, RecordType* types) { expr_list& el = args->Exprs(); int ntypes = types->NumFields(); @@ -5269,7 +5269,7 @@ int check_and_promote_args(ListExpr*& args, RecordType* types) return rval; } -int check_and_promote_exprs_to_type(ListExpr*& elements, BroType* type) +int check_and_promote_exprs_to_type(ListExpr* const elements, BroType* type) { expr_list& el = elements->Exprs(); diff --git a/src/Expr.h b/src/Expr.h index 701e826b19..771f4782e5 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -245,9 +245,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - NameExpr() { id = 0; } - void ExprDescribe(ODesc* d) const override; ID* id; @@ -266,9 +263,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - ConstExpr() { val = 0; } - void ExprDescribe(ODesc* d) const override; Val* val; }; @@ -287,9 +281,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - UnaryExpr() { op = 0; } - UnaryExpr(BroExprTag arg_tag, Expr* arg_op); ~UnaryExpr() override; @@ -316,9 +307,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - BinaryExpr() { op1 = op2 = 0; } - BinaryExpr(BroExprTag arg_tag, Expr* arg_op1, Expr* arg_op2) : Expr(arg_tag), op1(arg_op1), op2(arg_op2) { @@ -369,9 +357,6 @@ public: Val* Eval(Frame* f) const override; protected: - friend class Expr; - CloneExpr() { } - Val* Fold(Val* v) const override; }; @@ -382,10 +367,6 @@ public: Val* Eval(Frame* f) const override; Val* DoSingleEval(Frame* f, Val* v) const; int IsPure() const override; - -protected: - friend class Expr; - IncrExpr() { } }; class ComplementExpr : public UnaryExpr { @@ -393,9 +374,6 @@ public: explicit ComplementExpr(Expr* op); protected: - friend class Expr; - ComplementExpr() { } - Val* Fold(Val* v) const override; }; @@ -404,9 +382,6 @@ public: explicit NotExpr(Expr* op); protected: - friend class Expr; - NotExpr() { } - Val* Fold(Val* v) const override; }; @@ -415,9 +390,6 @@ public: explicit PosExpr(Expr* op); protected: - friend class Expr; - PosExpr() { } - Val* Fold(Val* v) const override; }; @@ -426,9 +398,6 @@ public: explicit NegExpr(Expr* op); protected: - friend class Expr; - NegExpr() { } - Val* Fold(Val* v) const override; }; @@ -438,9 +407,6 @@ public: Val* Eval(Frame* f) const override; protected: - friend class Expr; - SizeExpr() { } - Val* Fold(Val* v) const override; }; @@ -448,49 +414,29 @@ class AddExpr : public BinaryExpr { public: AddExpr(Expr* op1, Expr* op2); void Canonicize() override; - -protected: - friend class Expr; - AddExpr() { } }; class AddToExpr : public BinaryExpr { public: AddToExpr(Expr* op1, Expr* op2); Val* Eval(Frame* f) const override; - -protected: - friend class Expr; - AddToExpr() { } }; class RemoveFromExpr : public BinaryExpr { public: RemoveFromExpr(Expr* op1, Expr* op2); Val* Eval(Frame* f) const override; - -protected: - friend class Expr; - RemoveFromExpr() { } }; class SubExpr : public BinaryExpr { public: SubExpr(Expr* op1, Expr* op2); - -protected: - friend class Expr; - SubExpr() { } }; class TimesExpr : public BinaryExpr { public: TimesExpr(Expr* op1, Expr* op2); void Canonicize() override; - -protected: - friend class Expr; - TimesExpr() { } }; class DivideExpr : public BinaryExpr { @@ -498,19 +444,12 @@ public: DivideExpr(Expr* op1, Expr* op2); protected: - friend class Expr; - DivideExpr() { } - Val* AddrFold(Val* v1, Val* v2) const override; }; class ModExpr : public BinaryExpr { public: ModExpr(Expr* op1, Expr* op2); - -protected: - friend class Expr; - ModExpr() { } }; class BoolExpr : public BinaryExpr { @@ -519,19 +458,11 @@ public: Val* Eval(Frame* f) const override; Val* DoSingleEval(Frame* f, Val* v1, Expr* op2) const; - -protected: - friend class Expr; - BoolExpr() { } }; class BitExpr : public BinaryExpr { public: BitExpr(BroExprTag tag, Expr* op1, Expr* op2); - -protected: - friend class Expr; - BitExpr() { } }; class EqExpr : public BinaryExpr { @@ -540,9 +471,6 @@ public: void Canonicize() override; protected: - friend class Expr; - EqExpr() { } - Val* Fold(Val* v1, Val* v2) const override; }; @@ -550,10 +478,6 @@ class RelExpr : public BinaryExpr { public: RelExpr(BroExprTag tag, Expr* op1, Expr* op2); void Canonicize() override; - -protected: - friend class Expr; - RelExpr() { } }; class CondExpr : public Expr { @@ -571,9 +495,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - CondExpr() { op1 = op2 = op3 = 0; } - void ExprDescribe(ODesc* d) const override; Expr* op1; @@ -587,10 +508,6 @@ public: void Assign(Frame* f, Val* v) override; Expr* MakeLvalue() override; - -protected: - friend class Expr; - RefExpr() { } }; class AssignExpr : public BinaryExpr { @@ -608,9 +525,6 @@ public: int IsPure() const override; protected: - friend class Expr; - AssignExpr() { } - bool TypeCheck(attr_list* attrs = 0); bool TypeCheckArithmetics(TypeTag bt1, TypeTag bt2); @@ -622,10 +536,6 @@ class IndexSliceAssignExpr : public AssignExpr { public: IndexSliceAssignExpr(Expr* op1, Expr* op2, int is_init); Val* Eval(Frame* f) const override; - -protected: - friend class Expr; - IndexSliceAssignExpr() {} }; class IndexExpr : public BinaryExpr { @@ -650,9 +560,6 @@ public: bool IsSlice() const { return is_slice; } protected: - friend class Expr; - IndexExpr() { } - Val* Fold(Val* v1, Val* v2) const override; void ExprDescribe(ODesc* d) const override; @@ -676,9 +583,6 @@ public: Expr* MakeLvalue() override; protected: - friend class Expr; - FieldExpr() { field_name = 0; td = 0; } - Val* Fold(Val* v) const override; void ExprDescribe(ODesc* d) const override; @@ -698,9 +602,6 @@ public: const char* FieldName() const { return field_name; } protected: - friend class Expr; - HasFieldExpr() { field_name = 0; } - Val* Fold(Val* v) const override; void ExprDescribe(ODesc* d) const override; @@ -715,9 +616,6 @@ public: ~RecordConstructorExpr() override; protected: - friend class Expr; - RecordConstructorExpr() { } - Val* InitVal(const BroType* t, Val* aggr) const override; Val* Fold(Val* v) const override; @@ -735,9 +633,6 @@ public: Val* Eval(Frame* f) const override; protected: - friend class Expr; - TableConstructorExpr() { } - Val* InitVal(const BroType* t, Val* aggr) const override; void ExprDescribe(ODesc* d) const override; @@ -756,9 +651,6 @@ public: Val* Eval(Frame* f) const override; protected: - friend class Expr; - SetConstructorExpr() { } - Val* InitVal(const BroType* t, Val* aggr) const override; void ExprDescribe(ODesc* d) const override; @@ -773,9 +665,6 @@ public: Val* Eval(Frame* f) const override; protected: - friend class Expr; - VectorConstructorExpr() { } - Val* InitVal(const BroType* t, Val* aggr) const override; void ExprDescribe(ODesc* d) const override; @@ -791,9 +680,6 @@ public: int IsRecordElement(TypeDecl* td) const override; protected: - friend class Expr; - FieldAssignExpr() { } - void ExprDescribe(ODesc* d) const override; string field_name; @@ -804,9 +690,6 @@ public: ArithCoerceExpr(Expr* op, TypeTag t); protected: - friend class Expr; - ArithCoerceExpr() { } - Val* FoldSingleVal(Val* v, InternalTypeTag t) const; Val* Fold(Val* v) const override; }; @@ -817,9 +700,6 @@ public: ~RecordCoerceExpr() override; protected: - friend class Expr; - RecordCoerceExpr() { map = 0; } - Val* InitVal(const BroType* t, Val* aggr) const override; Val* Fold(Val* v) const override; @@ -835,9 +715,6 @@ public: ~TableCoerceExpr() override; protected: - friend class Expr; - TableCoerceExpr() { } - Val* Fold(Val* v) const override; }; @@ -847,9 +724,6 @@ public: ~VectorCoerceExpr() override; protected: - friend class Expr; - VectorCoerceExpr() { } - Val* Fold(Val* v) const override; }; @@ -860,9 +734,6 @@ public: explicit FlattenExpr(Expr* op); protected: - friend class Expr; - FlattenExpr() { } - Val* Fold(Val* v) const override; int num_fields; @@ -897,9 +768,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - ScheduleExpr() { when = 0; event = 0; } - void ExprDescribe(ODesc* d) const override; Expr* when; @@ -911,9 +779,6 @@ public: InExpr(Expr* op1, Expr* op2); protected: - friend class Expr; - InExpr() { } - Val* Fold(Val* v1, Val* v2) const override; }; @@ -933,9 +798,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - CallExpr() { func = 0; args = 0; } - void ExprDescribe(ODesc* d) const override; Expr* func; @@ -980,9 +842,6 @@ public: TraversalCode Traverse(TraversalCallback* cb) const override; protected: - friend class Expr; - EventExpr() { args = 0; } - void ExprDescribe(ODesc* d) const override; string name; @@ -1028,12 +887,6 @@ protected: class RecordAssignExpr : public ListExpr { public: RecordAssignExpr(Expr* record, Expr* init_list, int is_init); - - Val* Eval(Frame* f) const override { return ListExpr::Eval(f); } - -protected: - friend class Expr; - RecordAssignExpr() { } }; class CastExpr : public UnaryExpr { @@ -1041,9 +894,6 @@ public: CastExpr(Expr* op, BroType* t); protected: - friend class Expr; - CastExpr() { } - Val* Eval(Frame* f) const override; void ExprDescribe(ODesc* d) const override; }; @@ -1054,9 +904,6 @@ public: virtual ~IsExpr(); protected: - friend class Expr; - IsExpr() { } - Val* Fold(Val* v) const override; void ExprDescribe(ODesc* d) const override; @@ -1085,9 +932,9 @@ Expr* get_assign_expr(Expr* op1, Expr* op2, int is_init); // // Note, the type is not "const" because it can be ref'd. extern int check_and_promote_expr(Expr*& e, BroType* t); -extern int check_and_promote_exprs(ListExpr*& elements, TypeList* types); -extern int check_and_promote_args(ListExpr*& args, RecordType* types); -extern int check_and_promote_exprs_to_type(ListExpr*& elements, BroType* type); +extern int check_and_promote_exprs(ListExpr* elements, TypeList* types); +extern int check_and_promote_args(ListExpr* args, RecordType* types); +extern int check_and_promote_exprs_to_type(ListExpr* elements, BroType* type); // Returns a ListExpr simplified down to a list a values, or a nil // pointer if they couldn't all be reduced. diff --git a/src/ID.h b/src/ID.h index 7b6a1ada6c..0eaf1d42b8 100644 --- a/src/ID.h +++ b/src/ID.h @@ -113,8 +113,6 @@ public: std::vector GetOptionHandlers() const; protected: - ID() { name = 0; type = 0; val = 0; attrs = 0; } - void EvalFunc(Expr* ef, Expr* ev); #ifdef DEBUG diff --git a/src/IntrusivePtr.h b/src/IntrusivePtr.h index 767aae197c..b6c273a3ff 100644 --- a/src/IntrusivePtr.h +++ b/src/IntrusivePtr.h @@ -71,9 +71,9 @@ public: * * @param raw_ptr Pointer to the shared object. */ - IntrusivePtr(AdoptRef, pointer raw_ptr) noexcept + constexpr IntrusivePtr(AdoptRef, pointer raw_ptr) noexcept + : ptr_(raw_ptr) { - setPtr(raw_ptr, false); } /** @@ -85,8 +85,10 @@ public: * @param raw_ptr Pointer to the shared object. */ IntrusivePtr(NewRef, pointer raw_ptr) noexcept + : ptr_(raw_ptr) { - setPtr(raw_ptr, true); + if ( ptr_ ) + Ref(ptr_); } IntrusivePtr(IntrusivePtr&& other) noexcept : ptr_(other.release()) @@ -116,6 +118,12 @@ public: std::swap(ptr_, other.ptr_); } + friend void swap(IntrusivePtr& a, IntrusivePtr& b) noexcept + { + using std::swap; + swap(a.ptr_, b.ptr_); + } + /** * Detaches an object from the automated lifetime management and sets this * intrusive pointer to @c nullptr. @@ -123,10 +131,7 @@ public: */ pointer release() noexcept { - auto result = ptr_; - if ( result ) - ptr_ = nullptr; - return result; + return std::exchange(ptr_, nullptr); } IntrusivePtr& operator=(IntrusivePtr other) noexcept @@ -161,13 +166,6 @@ public: } private: - void setPtr(pointer raw_ptr, bool add_ref) noexcept - { - ptr_ = raw_ptr; - if ( raw_ptr && add_ref ) - Ref(raw_ptr); - } - pointer ptr_ = nullptr; }; diff --git a/src/List.h b/src/List.h index 2b25dfb6f8..16393a34ed 100644 --- a/src/List.h +++ b/src/List.h @@ -135,6 +135,8 @@ public: num_entries = max_entries = 0; } + bool empty() const noexcept { return num_entries == 0; } + int length() const { return num_entries; } int max() const { return max_entries; } int resize(int new_size = 0) // 0 => size to fit current number of entries diff --git a/src/Scope.cc b/src/Scope.cc index f53d4e0ba4..dce70f7639 100644 --- a/src/Scope.cc +++ b/src/Scope.cc @@ -160,7 +160,7 @@ ID* lookup_ID(const char* name, const char* curr_module, bool no_global, ID* install_ID(const char* name, const char* module_name, bool is_global, bool is_export) { - if ( scopes.length() == 0 && ! is_global ) + if ( scopes.empty() && ! is_global ) reporter->InternalError("local identifier in global scope"); IDScope scope; @@ -200,10 +200,9 @@ void push_scope(ID* id, attr_list* attrs) Scope* pop_scope() { - int n = scopes.length() - 1; - if ( n < 0 ) + if ( scopes.empty() ) reporter->InternalError("scope underflow"); - scopes.remove_nth(n); + scopes.pop_back(); Scope* old_top = top_scope; // Don't delete the scope; keep it around for later name resolution @@ -211,7 +210,7 @@ Scope* pop_scope() // ### SERIOUS MEMORY LEAK!? // delete top_scope; - top_scope = n == 0 ? 0 : scopes[n-1]; + top_scope = scopes.empty() ? nullptr : scopes.back(); return old_top; } @@ -223,5 +222,5 @@ Scope* current_scope() Scope* global_scope() { - return scopes.length() == 0 ? 0 : scopes[0]; + return scopes.empty() ? 0 : scopes.front(); } diff --git a/src/Type.cc b/src/Type.cc index 33427ba24b..3cb6b09257 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -93,7 +93,7 @@ BroType* BroType::ShallowClone() return nullptr; } -int BroType::MatchesIndex(ListExpr*& index) const +int BroType::MatchesIndex(ListExpr* const index) const { if ( Tag() == TYPE_STRING ) { @@ -226,7 +226,7 @@ IndexType::~IndexType() Unref(yield_type); } -int IndexType::MatchesIndex(ListExpr*& index) const +int IndexType::MatchesIndex(ListExpr* const index) const { // If we have a type indexed by subnets, addresses are ok. const type_list* types = indices->Types(); @@ -527,7 +527,7 @@ const BroType* FuncType::YieldType() const return yield; } -int FuncType::MatchesIndex(ListExpr*& index) const +int FuncType::MatchesIndex(ListExpr* const index) const { return check_and_promote_args(index, args) ? MATCHES_INDEX_SCALAR : DOES_NOT_MATCH_INDEX; @@ -1399,7 +1399,7 @@ const BroType* VectorType::YieldType() const return yield_type; } -int VectorType::MatchesIndex(ListExpr*& index) const +int VectorType::MatchesIndex(ListExpr* const index) const { expr_list& el = index->Exprs(); diff --git a/src/Type.h b/src/Type.h index 740fdbe330..e2c674b438 100644 --- a/src/Type.h +++ b/src/Type.h @@ -164,7 +164,7 @@ public: // if it matches and produces a vector result; and // DOES_NOT_MATCH_INDEX = 0 if it can't match (or the type // is not an indexable type). - virtual int MatchesIndex(ListExpr*& index) const; + virtual int MatchesIndex(ListExpr* index) const; // Returns the type yielded by this type. For example, if // this type is a table[string] of port, then returns the "port" @@ -383,7 +383,7 @@ protected: class IndexType : public BroType { public: - int MatchesIndex(ListExpr*& index) const override; + int MatchesIndex(ListExpr* index) const override; TypeList* Indices() const { return indices; } const type_list* IndexTypes() const { return indices->Types(); } @@ -459,7 +459,7 @@ public: void ClearYieldType(function_flavor arg_flav) { Unref(yield); yield = 0; flavor = arg_flav; } - int MatchesIndex(ListExpr*& index) const override; + int MatchesIndex(ListExpr* index) const override; int CheckArgs(const type_list* args, bool is_init = false) const; TypeList* ArgTypes() const { return arg_types; } @@ -673,7 +673,7 @@ public: BroType* YieldType() override; const BroType* YieldType() const override; - int MatchesIndex(ListExpr*& index) const override; + int MatchesIndex(ListExpr* index) const override; // Returns true if this table type is "unspecified", which is what one // gets using an empty "vector()" constructor.