Improve &expire_func compatibility type-checking #1505
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes GH-1503
Previously, incompatible &expire_funcs could mistakenly be used, such as
when using that attribute on the unspecified table()/set()
initializations/assignments, resulting in invalid function calls that
eventually crash Zeek.
Ideally, type-checking could catch this at parse-time always, but found that may not generally be possible and a run-time type-check may have to happen in rare scenarios (but the common ones still always catch parse-time). Going from common to rare:
When using unspecified
table()
expression in initializations, the full type/attribute info isn't available to be caught in the previousCheckAttr()
location, have to wait formake_var
logic to propagate from unspecifiedtable()
->ID
->TableVal
ctor ->TableVal::SetAttrs()
If using unspecified
table()
in run-time assignment, theTableCoerceExpr
could likely be expanded to help catch more at parse-time, but eventually it will useTableVal
ctor ->TableVal::SetAttrs()
The
TableVal::SetAttrs()
method is technically there for anyone to call directly (e.g. external plugins).So this patch addresses the common denominator:
TableVal::SetAttrs()
must not be allowed to set an incompatible expire function.It's possible that some checks are now redundant and can give multiple error messages for the same issue, but I find that to be ok: don't want to get rid of the
Attributes::CheckAttr()
check, because I'm not sure if it's still helping find some types incompatible expire functions at parse-time rather than run-time (e.g. when full type info actually is available for assignments).