Merge remote-tracking branch 'origin/topic/vern/cpp-prep2'

* origin/topic/vern/cpp-prep2:
  Avoid a redundant set operation in ProfileFuncs::MergeInProfile()
  Use std::move() where possible in parse.y build_local()/build_global()
  optionally hash original, not extended, form of records for profiling profile types associated with global initializations incorporate type name, if present, into type hash
  fixed out-of-range enum constant
  method to add a fully-qualified enum name (module name included)
  enum types track whether they've had values added via "redef"
  tracking of original size of records (pre redef'ing)
  Added TableVal::ToMap to retrieve a table's entire contents as a unordered_map
  micro whitespace nit
  factoring out some replicated code in the parser
  some micro-preening of parse.y; no semantic changes
  for readability, removed explicit scoping in parse.y; no semantic changes

scripts/base/frameworks/openflow/consts.zeek

@@ -123,7 +123,7 @@ export {
 	## Return value for a cookie from a flow
 	## which is not added, modified or deleted
 	## from the Zeek openflow framework.
-	const INVALID_COOKIE = 0xffffffffffffffff;
+	const INVALID_COOKIE = 0x7fffffffffffffff;
 	# Openflow physical port definitions
 	## Send the packet out the input port. This
 	## virual port must be explicitly used in

src/Expr.cc

@@ -3397,7 +3397,7 @@ ValPtr TableConstructorExpr::InitVal(const zeek::Type* t, ValPtr aggr) const
 
 	auto tval = aggr ?
 	        TableValPtr{AdoptRef{}, aggr.release()->AsTableVal()} :
-	make_intrusive<TableVal>(std::move(tt), attrs);
+	        make_intrusive<TableVal>(std::move(tt), attrs);
 	const ExprPList& exprs = op->AsListExpr()->Exprs();
 
 	for ( const auto& expr : exprs )

src/Type.cc

@@ -850,6 +850,8 @@ RecordType::RecordType(type_decl_list* arg_types) : Type(TYPE_RECORD)
 		}
 	else
 		num_fields = 0;
+
+	num_orig_fields = num_fields;
 	}
 
 // in this case the clone is actually not so shallow, since
@@ -1339,6 +1341,9 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name,
                                bro_int_t val, bool is_export, detail::Expr* deprecation,
                                bool from_redef)
 	{
+	if ( from_redef )
+		has_redefs = true;
+
 	if ( Lookup(val) )
 		{
 		reporter->Error("enumerator value in enumerated type definition already exists");
@@ -1409,8 +1414,12 @@ void EnumType::AddNameInternal(const string& full_name, bro_int_t val)
 
 bro_int_t EnumType::Lookup(const string& module_name, const char* name) const
 	{
-	NameMap::const_iterator pos =
+	return Lookup(detail::make_full_var_name(module_name.c_str(), name));
-		names.find(detail::make_full_var_name(module_name.c_str(), name).c_str());
+	}
+
+bro_int_t EnumType::Lookup(const string& full_name) const
+	{
+	NameMap::const_iterator pos = names.find(full_name.c_str());
 
 	if ( pos == names.end() )
 		return -1;

src/Type.h

@@ -611,6 +611,7 @@ public:
 		{ return managed_fields; }
 
 	int NumFields() const			{ return num_fields; }
+	int NumOrigFields() const		{ return num_orig_fields; }
 
 	/**
 	 * Returns a "record_field_table" value for introspection purposes.
@@ -654,7 +655,12 @@ protected:
 	// use std::bitset here instead.
 	std::vector<bool> managed_fields;
 
+	// Number of fields in the type.
 	int num_fields;
+
+	// Number of fields in the type when originally declared.
+	int num_orig_fields;
+
 	type_decl_list* types;
 };
 
@@ -714,14 +720,19 @@ public:
 	// added that aren't likewise explicitly initalized.
 	void AddName(const std::string& module_name, const char* name, bro_int_t val, bool is_export, detail::Expr* deprecation = nullptr, bool from_redef = false);
 
-	// -1 indicates not found.
+	// -1 indicates not found.  Second version is for full names
+	// that already incorporate the module.
 	bro_int_t Lookup(const std::string& module_name, const char* name) const;
+	bro_int_t Lookup(const std::string& full_name) const;
+
 	const char* Lookup(bro_int_t value) const; // Returns 0 if not found
 
 	// Returns the list of defined names with their values. The names
 	// will be fully qualified with their module name.
 	enum_name_list Names() const;
 
+	bool HasRedefs() const		{ return has_redefs; }
+
 	void Describe(ODesc* d) const override;
 	void DescribeReST(ODesc* d, bool roles_only = false) const override;
 
@@ -741,6 +752,9 @@ protected:
 	typedef std::map<std::string, bro_int_t> NameMap;
 	NameMap names;
 
+	// Whether any of the elements of the enum were added via redef's.
+	bool has_redefs = false;
+
 	using ValMap = std::unordered_map<bro_int_t, EnumValPtr>;
 	ValMap vals;
 

src/Val.cc

@@ -2311,6 +2311,22 @@ ListValPtr TableVal::ToPureListVal() const
 	return ToListVal(tl[0]->Tag());
 	}
 
+std::unordered_map<Val*, ValPtr> TableVal::ToMap() const
+	{
+	std::unordered_map<Val*, ValPtr> res;
+
+	for ( const auto& iter : *table_val )
+		{
+		auto k = iter.GetHashKey();
+		auto v = iter.GetValue<TableEntryVal*>();
+		auto vl = table_hash->RecoverVals(*k);
+
+		res[vl.release()] = v->GetVal();
+		}
+
+	return res;
+	}
+
 const detail::AttrPtr& TableVal::GetAttr(detail::AttrTag t) const
 	{
 	return attrs ? attrs->Find(t) : detail::Attr::nil;

src/Val.h

@@ -895,6 +895,11 @@ public:
 	// with non-composite index type).
 	ListValPtr ToPureListVal() const;
 
+	// Returns a map of index-to-value's.  The value is nil for sets.
+	// It's up to the caller to Unref() the index Val* when done
+	// with it.
+	std::unordered_map<Val*, ValPtr> ToMap() const;
+
 	void SetAttrs(detail::AttributesPtr attrs);
 
 	const detail::AttrPtr& GetAttr(detail::AttrTag t) const;

src/script_opt/ProfileFunc.cc

@@ -59,13 +59,16 @@ p_hash_type script_specific_hash(const StmtPtr& body, p_hash_type generic_hash)
 	}
 
 
-ProfileFunc::ProfileFunc(const Func* func, const StmtPtr& body)
+ProfileFunc::ProfileFunc(const Func* func, const StmtPtr& body, bool _abs_rec_fields)
 	{
+	abs_rec_fields = _abs_rec_fields;
 	Profile(func->GetType().get(), body);
 	}
 
-ProfileFunc::ProfileFunc(const Expr* e)
+ProfileFunc::ProfileFunc(const Expr* e, bool _abs_rec_fields)
 	{
+	abs_rec_fields = _abs_rec_fields;
+
 	if ( e->Tag() == EXPR_LAMBDA )
 		{
 		auto func = e->AsLambdaExpr();
@@ -227,10 +230,16 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e)
 		}
 
 	case EXPR_FIELD:
-		{
+		if ( abs_rec_fields )
-		auto f = e->AsFieldExpr()->Field();
+			{
-		addl_hashes.push_back(p_hash(f));
+			auto f = e->AsFieldExpr()->Field();
-		}
+			addl_hashes.push_back(p_hash(f));
+			}
+		else
+			{
+			auto fn = e->AsFieldExpr()->FieldName();
+			addl_hashes.push_back(p_hash(fn));
+			}
 		break;
 
 	case EXPR_ASSIGN:
@@ -405,14 +414,18 @@ void ProfileFunc::TrackID(const ID* id)
 	}
 
 
-ProfileFuncs::ProfileFuncs(std::vector<FuncInfo>& funcs, is_compilable_pred pred)
+ProfileFuncs::ProfileFuncs(std::vector<FuncInfo>& funcs,
+                           is_compilable_pred pred, bool _full_record_hashes)
 	{
+	full_record_hashes = _full_record_hashes;
+
 	for ( auto& f : funcs )
 		{
 		if ( f.ShouldSkip() )
 			continue;
 
-		auto pf = std::make_unique<ProfileFunc>(f.Func(), f.Body());
+		auto pf = std::make_unique<ProfileFunc>(f.Func(), f.Body(),
+		                                        full_record_hashes);
 
 		if ( ! pred || (*pred)(pf.get()) )
 			MergeInProfile(pf.get());
@@ -442,7 +455,19 @@ ProfileFuncs::ProfileFuncs(std::vector<FuncInfo>& funcs, is_compilable_pred pred
 void ProfileFuncs::MergeInProfile(ProfileFunc* pf)
 	{
 	all_globals.insert(pf->AllGlobals().begin(), pf->AllGlobals().end());
-	globals.insert(pf->Globals().begin(), pf->Globals().end());
+
+	for ( auto& g : pf->Globals() )
+		{
+		auto [it, inserted] = globals.emplace(g);
+
+		if ( ! inserted )
+			continue;
+
+		auto& v = g->GetVal();
+		if ( v )
+			main_types.push_back(v->GetType().get());
+		}
+
 	constants.insert(pf->Constants().begin(), pf->Constants().end());
 	main_types.insert(main_types.end(),
 			pf->OrderedTypes().begin(), pf->OrderedTypes().end());
@@ -471,7 +496,7 @@ void ProfileFuncs::DrainPendingExprs()
 
 		for ( auto e : pe )
 			{
-			auto pf = std::make_shared<ProfileFunc>(e);
+			auto pf = std::make_shared<ProfileFunc>(e, full_record_hashes);
 
 			expr_profs[e] = pf;
 			MergeInProfile(pf.get());
@@ -575,6 +600,8 @@ p_hash_type ProfileFuncs::HashType(const Type* t)
 		}
 
 	auto h = p_hash(t->Tag());
+	if ( ! tn.empty() )
+		h = merge_p_hashes(h, p_hash(tn));
 
 	// Enter an initial value for this type's hash.  We'll update it
 	// at the end, but having it here first will prevent recursive
@@ -611,13 +638,30 @@ p_hash_type ProfileFuncs::HashType(const Type* t)
 		{
 		const auto& ft = t->AsRecordType();
 		auto n = ft->NumFields();
+		auto orig_n = ft->NumOrigFields();
 
 		h = merge_p_hashes(h, p_hash("record"));
-		h = merge_p_hashes(h, p_hash(n));
+
+		if ( full_record_hashes )
+			h = merge_p_hashes(h, p_hash(n));
+		else
+			h = merge_p_hashes(h, p_hash(orig_n));
 
 		for ( auto i = 0; i < n; ++i )
 			{
+			bool do_hash = full_record_hashes;
+			if ( ! do_hash )
+				do_hash = (i < orig_n);
+
 			const auto& f = ft->FieldDecl(i);
+			auto type_h = HashType(f->type);
+
+			if ( do_hash )
+				{
+				h = merge_p_hashes(h, p_hash(f->id));
+				h = merge_p_hashes(h, type_h);
+				}
+
 			h = merge_p_hashes(h, p_hash(f->id));
 			h = merge_p_hashes(h, HashType(f->type));
 
@@ -626,7 +670,8 @@ p_hash_type ProfileFuncs::HashType(const Type* t)
 
 			if ( f->attrs )
 				{
-				h = merge_p_hashes(h, p_hash(f->attrs));
+				if ( do_hash )
+					h = merge_p_hashes(h, HashAttrs(f->attrs));
 				AnalyzeAttrs(f->attrs.get());
 				}
 			}
@@ -702,6 +747,29 @@ p_hash_type ProfileFuncs::HashType(const Type* t)
 	return h;
 	}
 
+p_hash_type ProfileFuncs::HashAttrs(const AttributesPtr& Attrs)
+	{
+	// It's tempting to just use p_hash, but that won't work
+	// if the attributes wind up with extensible records in their
+	// descriptions, if we're not doing full record hashes.
+	auto attrs = Attrs->GetAttrs();
+	p_hash_type h = 0;
+
+	for ( const auto& a : attrs )
+		{
+		h = merge_p_hashes(h, p_hash(a->Tag()));
+		auto e = a->GetExpr();
+
+		// We don't try to hash an associated expression, since those
+		// can vary in structure due to compilation of elements.  We
+		// do though enforce consistency for their types.
+		if ( e )
+			h = merge_p_hashes(h, HashType(e->GetType()));
+		}
+
+	return h;
+	}
+
 void ProfileFuncs::AnalyzeAttrs(const Attributes* Attrs)
 	{
 	auto attrs = Attrs->GetAttrs();

src/script_opt/ProfileFunc.h

@@ -84,13 +84,13 @@ class ProfileFunc : public TraversalCallback {
 public:
 	// Constructor used for the usual case of profiling a script
 	// function and one of its bodies.
-	ProfileFunc(const Func* func, const StmtPtr& body);
+	ProfileFunc(const Func* func, const StmtPtr& body, bool abs_rec_fields);
 
 	// Constructor for profiling an AST expression.  This exists
 	// to support (1) profiling lambda expressions, and (2) traversing
 	// attribute expressions (such as &default=expr) to discover what
 	// components they include.
-	ProfileFunc(const Expr* func);
+	ProfileFunc(const Expr* func, bool abs_rec_fields);
 
 	// See the comments for the associated member variables for each
 	// of these accessors.
@@ -263,6 +263,10 @@ protected:
 	// whether a given body contains any.
 	int num_when_stmts = 0;
 
+	// Whether we should treat record field accesses as absolute
+	// (integer offset) or relative (name-based).
+	bool abs_rec_fields;
+
 	// Whether we're separately processing a "when" condition to
 	// mine out its script calls.
 	bool in_when = false;
@@ -280,8 +284,11 @@ public:
 	// Updates entries in "funcs" to include profiles.  If pred is
 	// non-nil, then it is called for each profile to see whether it's
 	// compilable, and, if not, the FuncInfo is marked as ShouldSkip().
+	// "full_record_hashes" controls whether the hashes for extended
+	// records covers their final, full form, or should only their
+	// original fields.
 	ProfileFuncs(std::vector<FuncInfo>& funcs,
-	             is_compilable_pred pred = nullptr);
+	             is_compilable_pred pred, bool full_record_hashes);
 
 	// The following accessors provide a global profile across all of
 	// the (non-skipped) functions in "funcs".  See the comments for
@@ -326,6 +333,8 @@ public:
 	p_hash_type HashType(const TypePtr& t)	{ return HashType(t.get()); }
 	p_hash_type HashType(const Type* t);
 
+	p_hash_type HashAttrs(const AttributesPtr& attrs);
+
 protected:
 	// Incorporate the given function profile into the global profile.
 	void MergeInProfile(ProfileFunc* pf);
@@ -414,6 +423,10 @@ protected:
 	// profile.  These can arise for example due to lambdas or
 	// record attributes.
 	std::vector<const Expr*> pending_exprs;
+
+	// Whether the hashes for extended records should cover their final,
+	// full form, or only their original fields.
+	bool full_record_hashes;
 };
 
 

src/script_opt/ScriptOpt.cc

@@ -81,7 +81,7 @@ void optimize_func(ScriptFunc* f, std::shared_ptr<ProfileFunc> pf,
 
 	if ( analysis_options.optimize_AST )
 		{
-		pf = std::make_shared<ProfileFunc>(f, body);
+		pf = std::make_shared<ProfileFunc>(f, body, true);
 		body->Traverse(pf.get());
 
 		RD_Decorate reduced_rds(pf);
@@ -111,7 +111,7 @@ void optimize_func(ScriptFunc* f, std::shared_ptr<ProfileFunc> pf,
 		}
 
 	// Profile the new body.
-	pf = std::make_shared<ProfileFunc>(f, body);
+	pf = std::make_shared<ProfileFunc>(f, body, true);
 	body->Traverse(pf.get());
 
 	// Compute its reaching definitions.
@@ -224,7 +224,7 @@ void analyze_scripts()
 
 	// Now that everything's parsed and BiF's have been initialized,
 	// profile the functions.
-	auto pfs = std::make_unique<ProfileFuncs>(funcs);
+	auto pfs = std::make_unique<ProfileFuncs>(funcs, nullptr, true);
 
 	// Figure out which functions either directly or indirectly
 	// appear in "when" clauses.