Merge branch 'refactor' of https://github.com/MaxKellermann/zeek

- Minor code style/format tweaks in merge

* 'refactor' of https://github.com/MaxKellermann/zeek:
  IntrusivePtr: overload std::swap()
  IntrusivePtr: eliminate setPtr()
  IntrusivePtr: optimize release() using std::exchange()
  Expr: remove unused default constructors and `friend` declarations
  Expr: remove useless override
  Type: don't pass reference to pointer to MatchesIndex()
  Expr: don't pass reference to pointer to check_and_promote*()
  ID: remove unused default constructor
  Scope: simplify PList access, use pop_back() and back()
  List: add method empty()

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)

VERSION

@@ -1 +1 @@
-3.2.0-dev.146
+3.2.0-dev.157

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();
 

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_exprs(ListExpr* elements, TypeList* types);
-extern int check_and_promote_args(ListExpr*& args, RecordType* 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_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.

src/ID.h

@@ -113,8 +113,6 @@ public:
 	std::vector<Func*> GetOptionHandlers() const;
 
 protected:
-	ID()	{ name = 0; type = 0; val = 0; attrs = 0; }
-
 	void EvalFunc(Expr* ef, Expr* ev);
 
 #ifdef DEBUG

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_;
+		return std::exchange(ptr_, nullptr);
-		if ( result )
-			ptr_ = nullptr;
-		return result;
 		}
 
 	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;
 };
 

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

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 ( scopes.empty() )
-	if ( n < 0 )
 		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();
 	}

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();
 

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.