Merge remote-tracking branch 'origin/topic/vern/add-C++-removal'

* origin/topic/vern/add-C++-removal:
  removed -O add-C++ option and updated documentation

CHANGES

@@ -1,3 +1,7 @@
+5.2.0-dev.521 | 2023-01-25 14:11:03 -0700
+
+  * removed -O add-C++ option and updated documentation (Vern Paxson, Corelight)
+
 5.2.0-dev.519 | 2023-01-25 11:12:15 -0700
 
   * GH-2684: Stop violating VXLAN for forwarding failures (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-5.2.0-dev.519
+5.2.0-dev.521

src/Options.cc

@@ -205,7 +205,6 @@ static void print_analysis_help()
 	fprintf(stderr, "    xform	transform scripts to \"reduced\" form\n");
 
 	fprintf(stderr, "\n--optimize options when generating C++:\n");
-	fprintf(stderr, "    add-C++	add C++ script bodies to existing generated code\n");
 	fprintf(
 		stderr,
 		"    allow-cond	allow standalone compilation of functions influenced by conditionals\n");
@@ -240,8 +239,6 @@ static void set_analysis_option(const char* opt, Options& opts)
 		a_o.activate = a_o.dump_xform = true;
 	else if ( util::streq(opt, "dump-ZAM") )
 		a_o.activate = a_o.dump_ZAM = true;
-	else if ( util::streq(opt, "add-C++") )
-		a_o.add_CPP = true;
 	else if ( util::streq(opt, "allow-cond") )
 		a_o.allow_cond = true;
 	else if ( util::streq(opt, "gen-C++") )

src/script_opt/CPP/Compile.h

@@ -127,7 +127,7 @@ class CPPCompile
 	{
 public:
 	CPPCompile(std::vector<FuncInfo>& _funcs, ProfileFuncs& pfs, const std::string& gen_name,
-	           bool add, bool _standalone, bool report_uncompilable);
+	           bool _standalone, bool report_uncompilable);
 	~CPPCompile();
 
 	// Constructing a CPPCompile object does all of the compilation.
@@ -345,10 +345,6 @@ class CPPCompile
 	// Maps functions (not hooks or events) to upstream compiled names.
 	std::unordered_map<std::string, std::string> hashed_funcs;
 
-	// If non-zero, provides a tag used for auxiliary/additional
-	// compilation units.
-	int addl_tag = 0;
-
 	// If true, the generated code should run "standalone".
 	bool standalone = false;
 
@@ -359,10 +355,6 @@ class CPPCompile
 	// likely not important to make distinct).
 	p_hash_type total_hash = 0;
 
-	// Working directory in which we're compiling.  Used to quasi-locate
-	// error messages when doing test-suite "add-C++" crunches.
-	std::string working_dir;
-
 	//
 	// End of methods related to script/C++ variables.
 

src/script_opt/CPP/Driver.cc

@@ -15,45 +15,18 @@ namespace zeek::detail
 using namespace std;
 
 CPPCompile::CPPCompile(vector<FuncInfo>& _funcs, ProfileFuncs& _pfs, const string& gen_name,
-                       bool add, bool _standalone, bool report_uncompilable)
+                       bool _standalone, bool report_uncompilable)
 	: funcs(_funcs), pfs(_pfs), standalone(_standalone)
 	{
 	auto target_name = gen_name.c_str();
-	auto mode = add ? "a" : "w";
 
-	write_file = fopen(target_name, mode);
+	write_file = fopen(target_name, "w");
 	if ( ! write_file )
 		{
 		reporter->Error("can't open C++ target file %s", target_name);
 		exit(1);
 		}
 
-	if ( add )
-		{
-		// We need a unique number to associate with the name
-		// space for the code we're adding.  A convenient way to
-		// generate this safely is to use the present size of the
-		// file we're appending to.  That guarantees that every
-		// incremental compilation will wind up with a different
-		// number.
-		struct stat st;
-		if ( fstat(fileno(write_file), &st) != 0 )
-			{
-			char buf[256];
-			util::zeek_strerror_r(errno, buf, sizeof(buf));
-			reporter->Error("fstat failed on %s: %s", target_name, buf);
-			exit(1);
-			}
-
-		// We use a value of "0" to mean "we're not appending,
-		// we're generating from scratch", so make sure we're
-		// distinct from that.
-		addl_tag = st.st_size + 1;
-		}
-
-	else
-		addl_tag = 0;
-
 	Compile(report_uncompilable);
 	}
 
@@ -64,15 +37,6 @@ CPPCompile::~CPPCompile()
 
 void CPPCompile::Compile(bool report_uncompilable)
 	{
-	// Get the working directory so we can use it in diagnostic messages
-	// as a way to identify this compilation.  Only germane when doing
-	// incremental compilation (particularly of the test suite).
-	char buf[8192];
-	if ( ! getcwd(buf, sizeof buf) )
-		reporter->FatalError("getcwd failed: %s", strerror(errno));
-
-	working_dir = buf;
-
 	unordered_set<string> filenames_reported_as_skipped;
 	bool had_to_skip = false;
 
@@ -228,13 +192,16 @@ void CPPCompile::Compile(bool report_uncompilable)
 
 void CPPCompile::GenProlog()
 	{
-	if ( addl_tag <= 1 )
-		// This is either a compilation via gen-C++, or
-		// one using add-C++ and an empty CPP-gen.cc file.
-		Emit("#include \"zeek/script_opt/CPP/Runtime.h\"\n");
+	Emit("#include \"zeek/script_opt/CPP/Runtime.h\"\n");
+
+	// Get the working directory for annotating the output to help
+	// with debugging.
+	char working_dir[8192];
+	if ( ! getcwd(working_dir, sizeof working_dir) )
+		reporter->FatalError("getcwd failed: %s", strerror(errno));
 
 	Emit("namespace zeek::detail { //\n");
-	Emit("namespace CPP_%s { // %s\n", Fmt(total_hash), working_dir);
+	Emit("namespace CPP_%s { // %s\n", Fmt(total_hash), string(working_dir));
 
 	// The following might-or-might-not wind up being populated/used.
 	Emit("std::vector<int> field_mapping;");
@@ -390,7 +357,7 @@ void CPPCompile::GenEpilog()
 
 	GenInitHook();
 
-	Emit("} // %s\n\n", scope_prefix(addl_tag));
+	Emit("} //\n\n");
 	Emit("} // zeek::detail");
 	}
 

src/script_opt/CPP/README.md

@@ -108,40 +108,8 @@ On the other hand, it's possible (not yet established) that code created
 using `gen-C++` can be made to compile significantly faster than
 standalone code.
 
-Another option, `-O add-C++`, instead _appends_ the generated code to existing C++ in `CPP-gen.cc`.
-You can use this option repeatedly for different scripts and then
-compile the collection _en masse_.
-
-There are additional workflows relating to running the test suite, which
-we document only briefly here as they're likely going to change or go away
-, as it's not clear they're actually needed.
-
-* `non-embedded-build`  
-Builds `zeek` without any embedded compiled-to-C++ scripts.
-* `bare-embedded-build`  
-Builds `zeek` with the `-b` "bare-mode" scripts compiled in.
-* `full-embedded-build`  
-Builds `zeek` with the default scripts compiled in.
-
-<br>
-
-* `eval-test-suite`  
-Runs the test suite using the `cpp` alternative over the given set of tests.
-* `test-suite-build`  
-Incrementally compiles to `CPP-gen-addl.h` code for the given test suite elements.
-
-<br>
-
-* `single-test.sh`  
-Builds the given btest test as a single `add-C++` add-on and then runs it.
-* `single-full-test.sh`  
-Builds the given btest test from scratch as a self-contained `zeek`, and runs it.
-* `update-single-test.sh`  
-Given an already-compiled `zeek` for the given test, updates its `cpp` test suite alternative.
-
-Some of these scripts could be made less messy if `btest` supported
-a "dry run" option that reported the executions it would do for a given
-test without actually undertaking them.
+There are additional workflows relating to running the test suite: see
+`src/script_opt/CPP/maint/README`.
 
 <br>
 

src/script_opt/ProfileFunc.cc

@@ -23,49 +23,6 @@ p_hash_type p_hash(const Obj* o)
 	return p_hash(d.Description());
 	}
 
-// Returns a filename associated with the given function body.  Used to
-// provide distinctness to identical function bodies seen in separate,
-// potentially conflicting incremental compilations.  An example of this
-// is a function named foo() that calls bar(), for which in two different
-// compilation contexts bar() has differing semantics, even though foo()'s
-// (shallow) semantics are the same.
-static std::string script_specific_filename(const Stmt* body)
-	{
-	// The specific filename is taken from the location filename, making
-	// it absolute if necessary.
-	auto body_loc = body->GetLocationInfo();
-	auto bl_f = body_loc->filename;
-	ASSERT(bl_f != nullptr);
-
-	if ( (bl_f[0] != '.' && bl_f[0] != '/') ||
-	     (bl_f[0] == '.' && (bl_f[1] == '/' || (bl_f[1] == '.' && bl_f[2] == '/'))) )
-		{
-		// Add working directory to avoid collisions over the
-		// same relative name.
-		static std::string working_dir;
-		if ( working_dir.empty() )
-			{
-			char buf[8192];
-			if ( ! getcwd(buf, sizeof buf) )
-				reporter->InternalError("getcwd failed: %s", strerror(errno));
-
-			working_dir = buf;
-			}
-
-		return working_dir + "/" + bl_f;
-		}
-
-	return bl_f;
-	}
-
-// Returns a incremental-compilation-specific hash for the given function
-// body, given its non-specific hash is "generic_hash".
-static p_hash_type script_specific_hash(const Stmt* body, p_hash_type generic_hash)
-	{
-	auto bl_f = script_specific_filename(body);
-	return merge_p_hashes(generic_hash, p_hash(bl_f));
-	}
-
 ProfileFunc::ProfileFunc(const Func* func, const StmtPtr& body, bool _abs_rec_fields)
 	{
 	profiled_func = func;
@@ -717,9 +674,6 @@ void ProfileFuncs::ComputeProfileHash(std::shared_ptr<ProfileFunc> pf)
 	for ( auto i : pf->AdditionalHashes() )
 		h = merge_p_hashes(h, i);
 
-	if ( ! pf->Stmts().empty() )
-		h = script_specific_hash(pf->Stmts()[0], h);
-
 	pf->SetHashVal(h);
 	}
 

src/script_opt/ScriptOpt.cc

@@ -272,15 +272,14 @@ static void init_options()
 	check_env_opt("ZEEK_PROFILE", analysis_options.profile_ZAM);
 
 	// Compile-to-C++-related options.
-	check_env_opt("ZEEK_ADD_CPP", analysis_options.add_CPP);
 	check_env_opt("ZEEK_GEN_CPP", analysis_options.gen_CPP);
 	check_env_opt("ZEEK_GEN_STANDALONE_CPP", analysis_options.gen_standalone_CPP);
 	check_env_opt("ZEEK_COMPILE_ALL", analysis_options.compile_all);
 	check_env_opt("ZEEK_REPORT_CPP", analysis_options.report_CPP);
 	check_env_opt("ZEEK_USE_CPP", analysis_options.use_CPP);
 	check_env_opt("ZEEK_ALLOW_COND", analysis_options.allow_cond);
 
-	if ( analysis_options.gen_standalone_CPP || analysis_options.add_CPP )
+	if ( analysis_options.gen_standalone_CPP )
 		analysis_options.gen_CPP = true;
 
 	if ( analysis_options.gen_CPP )
@@ -426,11 +425,10 @@ static void generate_CPP(std::unique_ptr<ProfileFuncs>& pfs)
 	{
 	const auto gen_name = CPP_dir + "CPP-gen.cc";
 
-	const bool add = analysis_options.add_CPP;
 	const bool standalone = analysis_options.gen_standalone_CPP;
 	const bool report = analysis_options.report_uncompilable;
 
-	CPPCompile cpp(funcs, *pfs, gen_name, add, standalone, report);
+	CPPCompile cpp(funcs, *pfs, gen_name, standalone, report);
 	}
 
 static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)

src/script_opt/ScriptOpt.h

@@ -102,9 +102,6 @@ struct AnalyOpt
 	// of the corresponding script, and not activated by default).
 	bool gen_standalone_CPP = false;
 
-	// Generate C++ that's added to existing generated code.
-	bool add_CPP = false;
-
 	// If true, use C++ bodies if available.
 	bool use_CPP = false;