Fixes from review, post-rebase

src/EventRegistry.cc

@@ -11,7 +11,7 @@ namespace zeek
 EventRegistry::EventRegistry() = default;
 EventRegistry::~EventRegistry() noexcept = default;
 
-EventHandlerPtr EventRegistry::Register(std::string_view name)
+EventHandlerPtr EventRegistry::Register(std::string_view name, bool is_from_script)
 	{
 	// If there already is an entry in the registry, we have a
 	// local handler on the script layer.
@@ -19,21 +19,29 @@ EventHandlerPtr EventRegistry::Register(std::string_view name)
 
 	if ( h )
 		{
+		if ( ! is_from_script )
+			not_only_from_script.insert(std::string(name));
+
 		h->SetUsed();
 		return h;
 		}
 
 	h = new EventHandler(std::string(name));
-	event_registry->Register(h);
+	event_registry->Register(h, is_from_script);
 
 	h->SetUsed();
 
 	return h;
 	}
 
-void EventRegistry::Register(EventHandlerPtr handler)
+void EventRegistry::Register(EventHandlerPtr handler, bool is_from_script)
 	{
-	handlers[std::string(handler->Name())] = std::unique_ptr<EventHandler>(handler.Ptr());
+	std::string name = handler->Name();
+
+	handlers[name] = std::unique_ptr<EventHandler>(handler.Ptr());
+
+	if ( ! is_from_script )
+		not_only_from_script.insert(name);
 	}
 
 EventHandler* EventRegistry::Lookup(std::string_view name)
@@ -45,6 +53,11 @@ EventHandler* EventRegistry::Lookup(std::string_view name)
 	return nullptr;
 	}
 
+bool EventRegistry::NotOnlyRegisteredFromScript(std::string_view name)
+	{
+	return not_only_from_script.count(std::string(name)) > 0;
+	}
+
 EventRegistry::string_list EventRegistry::Match(RE_Matcher* pattern)
 	{
 	string_list names;

src/EventRegistry.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 #include <string_view>
+#include <unordered_set>
 #include <vector>
 
 namespace zeek
@@ -28,15 +29,21 @@ public:
 	 * Performs a lookup for an existing event handler and returns it
 	 * if one exists, or else creates one, registers it, and returns it.
 	 * @param name  The name of the event handler to lookup/register.
+	 * @param name  Whether the registration is coming from a script element.
 	 * @return  The event handler.
 	 */
-	EventHandlerPtr Register(std::string_view name);
+	EventHandlerPtr Register(std::string_view name, bool is_from_script = false);
 
-	void Register(EventHandlerPtr handler);
+	void Register(EventHandlerPtr handler, bool is_from_script = false);
 
 	// Return nil if unknown.
 	EventHandler* Lookup(std::string_view name);
 
+	// True if the given event handler (1) exists, and (2) was registered
+	// in a non-script context (even if perhaps also registered in a script
+	// context).
+	bool NotOnlyRegisteredFromScript(std::string_view name);
+
 	// Returns a list of all local handlers that match the given pattern.
 	// Passes ownership of list.
 	using string_list = std::vector<std::string>;
@@ -55,6 +62,9 @@ public:
 
 private:
 	std::map<std::string, std::unique_ptr<EventHandler>, std::less<>> handlers;
+	// Tracks whether a given event handler was registered in a
+	// non-script context.
+	std::unordered_set<std::string> not_only_from_script;
 	};
 
 extern EventRegistry* event_registry;

src/Expr.cc

@@ -4877,7 +4877,7 @@ EventExpr::EventExpr(const char* arg_name, ListExprPtr arg_args)
 	if ( ! h )
 		{
 		h = new EventHandler(name.c_str());
-		event_registry->Register(h);
+		event_registry->Register(h, true);
 		}
 
 	h->SetUsed();

src/ID.cc

@@ -166,7 +166,7 @@ void ID::SetVal(ValPtr v)
 			{
 			handler = new EventHandler(name);
 			handler->SetFunc(func);
-			event_registry->Register(handler);
+			event_registry->Register(handler, true);
 
 			if ( ! IsExport() )
 				register_new_event({NewRef{}, this});

src/Options.cc

@@ -107,7 +107,7 @@ void usage(const char* prog, int code)
 	fprintf(stderr, "    -s|--rulefile <rulefile>        | read rules from given file\n");
 	fprintf(stderr, "    -t|--tracefile <tracefile>      | activate execution tracing\n");
 	fprintf(stderr, "    -u|--usage-issues               | find variable usage issues and exit\n");
-	fprintf(stderr, "       --no-usage-warnings          | suppress warnings of unused "
+	fprintf(stderr, "       --no-unused-warnings          | suppress warnings of unused "
 	                "functions/hooks/events\n");
 	fprintf(stderr, "    -v|--version                    | print version and exit\n");
 	fprintf(stderr, "    -w|--writefile <writefile>      | write to given tcpdump file\n");
@@ -369,7 +369,7 @@ Options parse_cmdline(int argc, char** argv)
 		}
 
 	int profile_scripts = 0;
-	int no_usage_warnings = 0;
+	int no_unused_warnings = 0;
 
 	struct option long_opts[] = {
 		{"parse-only", no_argument, nullptr, 'a'},
@@ -413,7 +413,7 @@ Options parse_cmdline(int argc, char** argv)
 #endif
 
 		{"profile-scripts", optional_argument, &profile_scripts, 1},
-		{"no-usage-warnings", no_argument, &no_usage_warnings, 1},
+		{"no-unused-warnings", no_argument, &no_unused_warnings, 1},
 		{"pseudo-realtime", optional_argument, nullptr, '~'},
 		{"jobs", optional_argument, nullptr, 'j'},
 		{"test", no_argument, nullptr, '#'},
@@ -618,8 +618,8 @@ Options parse_cmdline(int argc, char** argv)
 					profile_scripts = 0;
 					}
 
-				if ( no_usage_warnings )
+				if ( no_unused_warnings )
-					rval.no_usage_warnings = true;
+					rval.no_unused_warnings = true;
 				break;
 
 			case '?':

src/Options.h

@@ -58,7 +58,7 @@ struct Options
 	bool perftools_profile = false;
 	bool deterministic_mode = false;
 	bool abort_on_scripting_errors = false;
-	bool no_usage_warnings = false;
+	bool no_unused_warnings = false;
 
 	bool run_unit_tests = false;
 	std::vector<std::string> doctest_args;

src/plugin/Manager.h

@@ -107,7 +107,7 @@ public:
 	void ActivateDynamicPlugins(bool all);
 
 	/**
-	 * First-stage initializion of the manager. This is called early on
+	 * First-stage initialization of the manager. This is called early on
 	 * during Zeek's initialization, before any scripts are processed, and
 	 * forwards to the corresponding Plugin methods.
 	 */

src/script_opt/ScriptOpt.cc

@@ -520,7 +520,7 @@ static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)
 	finalize_functions(funcs);
 	}
 
-void analyze_scripts(bool no_usage_warnings)
+void analyze_scripts(bool no_unused_warnings)
 	{
 	static bool did_init = false;
 
@@ -531,7 +531,7 @@ void analyze_scripts(bool no_usage_warnings)
 		}
 
 	std::unique_ptr<UsageAnalyzer> ua;
-	if ( ! no_usage_warnings )
+	if ( ! no_unused_warnings )
 		ua = std::make_unique<UsageAnalyzer>(funcs);
 
 	auto& ofuncs = analysis_options.only_funcs;

src/script_opt/ScriptOpt.h

@@ -184,7 +184,7 @@ extern bool should_analyze(const ScriptFuncPtr& f, const StmtPtr& body);
 
 // Analyze all of the parsed scripts collectively for usage issues (unless
 // suppressed by the flag) and optimization.
-extern void analyze_scripts(bool no_usage_warnings);
+extern void analyze_scripts(bool no_unused_warnings);
 
 // Called when Zeek is terminating.
 extern void finish_script_execution();

src/script_opt/UsageAnalyzer.cc

@@ -2,6 +2,7 @@
 
 #include "zeek/script_opt/UsageAnalyzer.h"
 
+#include "zeek/EventRegistry.h"
 #include "zeek/module_util.h"
 #include "zeek/script_opt/IDOptInfo.h"
 
@@ -19,6 +20,15 @@ void register_new_event(const IDPtr& id)
 
 UsageAnalyzer::UsageAnalyzer(std::vector<FuncInfo>& funcs)
 	{
+	// First, prune the script events to only those that were never
+	// registered in a non-script context.
+	auto script_events_orig = script_events;
+	script_events.clear();
+
+	for ( auto& ev : script_events_orig )
+		if ( ! event_registry->NotOnlyRegisteredFromScript(ev) )
+			script_events.insert(ev);
+
 	// Setting a scope cues ID::Traverse to delve into function values.
 	current_scope = global_scope();
 
@@ -52,8 +62,7 @@ UsageAnalyzer::UsageAnalyzer(std::vector<FuncInfo>& funcs)
 		auto flavor = t->AsFuncType()->FlavorString();
 		auto loc = id->GetLocationInfo();
 
-		reporter->Warning("%s %s (%s:%d): cannot be invoked", flavor.c_str(), id->Name(),
+		id->Warn(util::fmt("handler for non-existing %s cannot be invoked", flavor.c_str()));
-		                  loc->filename, loc->first_line);
 
 		// Don't ding any functions that are reachable via this
 		// identifier.  This will also suppress flagging other events
@@ -77,8 +86,7 @@ UsageAnalyzer::UsageAnalyzer(std::vector<FuncInfo>& funcs)
 
 		auto loc = id->GetLocationInfo();
 
-		reporter->Warning("function %s (%s:%d): cannot be called", id->Name(), loc->filename,
+		id->Warn("function does not have any callers");
-		                  loc->first_line);
 
 		// Unlike for events/hooks above, we don't add the function to
 		// the reachables.  This is because an orphan function is a

src/zeek-setup.cc

@@ -589,8 +589,8 @@ SetupResult setup(int argc, char** argv, Options* zopts)
 	plugin_mgr = new plugin::Manager();
 	fragment_mgr = new detail::FragmentManager();
 
-	if ( options.no_usage_warnings && options.analysis_options.usage_issues > 0 )
+	if ( options.no_unused_warnings && options.analysis_options.usage_issues > 0 )
-		reporter->FatalError("-u incompatible with --no-usage-warnings");
+		reporter->FatalError("-u incompatible with --no-unused-warnings");
 
 #ifdef DEBUG
 	if ( options.debug_log_streams )
@@ -929,7 +929,7 @@ SetupResult setup(int argc, char** argv, Options* zopts)
 		if ( options.parse_only )
 			{
 			if ( analysis_options.usage_issues > 0 )
-				analyze_scripts(options.no_usage_warnings);
+				analyze_scripts(options.no_unused_warnings);
 
 			early_shutdown();
 			exit(reporter->Errors() != 0);
@@ -937,7 +937,7 @@ SetupResult setup(int argc, char** argv, Options* zopts)
 
 		auto init_stmts = stmts ? analyze_global_stmts(stmts) : nullptr;
 
-		analyze_scripts(options.no_usage_warnings);
+		analyze_scripts(options.no_unused_warnings);
 
 		if ( analysis_options.report_recursive )
 			{

testing/btest/Baseline/core.check-unused-event-handlers/.stderr

@@ -1,5 +1,6 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 ### NOTE: This file has been sorted with diff-sort.
+warning in <...>/check-unused-event-handlers.test, line 6: handler for non-existing event cannot be invoked (this_is_never_used)
 warning in <params>, line 1: event handler never invoked: InputConfig::new_value
 warning in <params>, line 1: event handler never invoked: InputRaw::process_finished
 warning in <params>, line 1: event handler never invoked: SupervisorControl::create_request
@@ -9,4 +10,3 @@ warning in <params>, line 1: event handler never invoked: SupervisorControl::sta
 warning in <params>, line 1: event handler never invoked: SupervisorControl::stop_request
 warning in <params>, line 1: event handler never invoked: spicy_analyzer_for_mime_type
 warning in <params>, line 1: event handler never invoked: this_is_never_used
-warning in <params>, line 1: event this_is_never_used (<...>/check-unused-event-handlers.test:6): cannot be invoked

testing/btest/Baseline/core.option-runtime-errors-8/.stderr

@@ -1,3 +1,3 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-warning: event option_changed (<...>/option-runtime-errors.zeek:4): cannot be invoked
+warning in <...>/option-runtime-errors.zeek, line 4: handler for non-existing event cannot be invoked (option_changed)
 error in <...>/option-runtime-errors.zeek, line 7: Option::on_change needs function argument; not hook or event (Option::set_change_handler(A, to_any_coerceoption_changed, (coerce 0 to int)))

testing/btest/Baseline/spicy.spicyz-aot/output

@@ -1,5 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-warning in <params>, line 1: event test::dummy (./test.zeek:3): cannot be invoked
 8, [$data=[b"POST /post HTTP/1.1", b"User-Agent: curl/7.29.0", b"Host: httpbin.org", b"Accept: */*", b"Content-Length: 11", b"Content-Type: application/x-www-form-urlencoded", b"", b"hello world"]]
 Event:, [POST /post HTTP/1.1, User-Agent: curl/7.29.0, Host: httpbin.org, Accept: */*, Content-Length: 11, Content-Type: application/x-www-form-urlencoded, , hello world]
 8, [$data=[b"HTTP/1.1 200 OK", b"Server: gunicorn/0.16.1", b"Date: Tue, 19 Mar 2013 16:05:11 GMT", b"Content-Type: application/json", b"Content-Length: 366", b"Connection: close", b"", b"{\x0a  \"origin\": \"10.142.133.148\",\x0a  \"files\": {},\x0a  \"form\": null,\x0a  \"url\": \"http://httpbin.org/post\",\x0a  \"args\": {},\x0a  \"headers\": {\x0a    \"Content-Length\": \"11\",\x0a    \"Connection\": \"close\",\x0a    \"Accept\": \"*/*\",\x0a    \"User-Agent\": \"curl/7.29.0\",\x0a    \"Host\": \"httpbin.org\",\x0a    \"Content-Type\": \"application/x-www-form-urlencoded\"\x0a  },\x0a  \"json\": null,\x0a  \"data\": \"hello world\"\x0a}"]]

testing/btest/Baseline/spicy.spicyz-jit/output

@@ -1,5 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-warning in <params>, line 1: event test::dummy (./test.zeek:3): cannot be invoked
 8, [$data=[b"POST /post HTTP/1.1", b"User-Agent: curl/7.29.0", b"Host: httpbin.org", b"Accept: */*", b"Content-Length: 11", b"Content-Type: application/x-www-form-urlencoded", b"", b"hello world"]]
 Event:, [POST /post HTTP/1.1, User-Agent: curl/7.29.0, Host: httpbin.org, Accept: */*, Content-Length: 11, Content-Type: application/x-www-form-urlencoded, , hello world]
 8, [$data=[b"HTTP/1.1 200 OK", b"Server: gunicorn/0.16.1", b"Date: Tue, 19 Mar 2013 16:05:11 GMT", b"Content-Type: application/json", b"Content-Length: 366", b"Connection: close", b"", b"{\x0a  \"origin\": \"10.142.133.148\",\x0a  \"files\": {},\x0a  \"form\": null,\x0a  \"url\": \"http://httpbin.org/post\",\x0a  \"args\": {},\x0a  \"headers\": {\x0a    \"Content-Length\": \"11\",\x0a    \"Connection\": \"close\",\x0a    \"Accept\": \"*/*\",\x0a    \"User-Agent\": \"curl/7.29.0\",\x0a    \"Host\": \"httpbin.org\",\x0a    \"Content-Type\": \"application/x-www-form-urlencoded\"\x0a  },\x0a  \"json\": null,\x0a  \"data\": \"hello world\"\x0a}"]]