[Zeek-Dev] support for event handlers using a subset of parameters

[Vern Paxson]

> > (4) avoid certain forms of user errors
> 
> Which particular user errors were a concern?

The main one being when there's an API change that's *not* backward
compatible, and the user doesn't update their scripts to be in conformance
with it as is required.  Clearly something we'll in general strive to avoid.

> I was still thinking
> name-based matching most clearly expressed the users intent, so I
> doubt that results in increased errors.

Agreed for those instances where it's compatible.

> Yeah, removing should work following this scheme.  What about changing
> semantics of a parameter though?

That's indeed trickier.  Here's a thought for the example you provide:

	event foo%(is_server: bool%) &deprecated;
	event foo%(is_client: bool%);

This would mean "if the handler is defined using the name is_server as the
parameter, that's the deprecated version".  Any other declaration (that's
for a parameter of type bool, of course) would refer to the new version.

By the way, I didn't follow whether in Vlad's example, the semantics of
the parameter changed, or if his fix was just to give it the correct name
to match its actual semantics.  For the latter, existing handlers are
presumably flat-out broken, and there's no benefit in trying to continue
to support them.

One concern I have with leaning on is-the-name-a-match is not-too-implausible
user coding along the lines of:

	event foo(is_server_orig: bool)
		{
		...
		local is_server = my_twisty_logic(is_server_orig, x, y, z);
		...
		}

i.e., the parameter being renamed by the user anyway because they want to
use the original name for a local variable.  These users will be bitten
by changes in parameter semantics regardless of which approach we use;
name-based matching isn't a panacea.

> The other problem is that if a user is skipping a version, they may
> end up with a handler that treats "is_server" in the original way, but
> the meaning has been changed and we only match events based on type +
> number of parameters.  With name-based parameter matching, we can
> catch this scenario as well.

With the tweak I outline above, this would only bite them once the &deprecated
version is removed.  That could span several releases, depending on the
particular situation.  I don't have a lot of sympathy for users who upgrade
across a number of releases, for which the release notes flag backward
compatibility issues, and they don't take heed to address those.

> Can you maybe break down what you mean by "clean" further so we can
> better compare/contrast solutions ?  I don't recall seeing anything
> that was a showstopper for my original patch (but been a while).

I find an approach that changes the fundamental type-checking used for
event handlers to be more heavy-handed than one that provides control to
the developer to explicitly decide when they've made a change for which
it makes sense to support a deprecated version.

If Zeek had originally used name-based parameters, then I'd be fine with
this being the solution.  However, switching from positional to name-based
strikes me as a conceptually deep change for a relatively modest problem.

		Vern