From 378ee699ffc058f368a06691e4237c7f1e791462 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@ncsa.illinois.edu>
Date: Mon, 26 Nov 2012 16:54:36 -0600
Subject: [PATCH 1/3] Hook functions now directly callable instead of w/ "hook"
 statements.

The return value of the call is an implicit boolean value of T if all
hook handlers ran, or F if one hook handler exited as a result of a
break statement and potentially prevented other handlers from running.

Scripts don't need to declare hooks with an explicit return type of bool
(internally, that's assumed), and any values given to (optional) return
statements in handler definitions are just ignored.

Addresses #918.
---
 doc/scripts/builtins.rst                      | 29 +++++-----
 src/Expr.cc                                   | 11 ++--
 src/Expr.h                                    |  2 +-
 src/Func.cc                                   | 23 ++++++--
 src/SerialTypes.h                             |  1 -
 src/Stmt.cc                                   | 54 ++-----------------
 src/Stmt.h                                    | 18 -------
 src/StmtEnums.h                               |  2 +-
 src/Var.cc                                    |  4 +-
 src/parse.y                                   | 24 ++-------
 testing/btest/Baseline/language.hook/out      |  8 +++
 .../btest/Baseline/language.invalid_hook/out  |  1 -
 testing/btest/core/leaks/hook.bro             | 30 ++++++++---
 testing/btest/language/hook.bro               | 30 ++++++++---
 testing/btest/language/invalid_hook.bro       | 16 ------
 15 files changed, 108 insertions(+), 145 deletions(-)
 delete mode 100644 testing/btest/Baseline/language.invalid_hook/out
 delete mode 100644 testing/btest/language/invalid_hook.bro

diff --git a/doc/scripts/builtins.rst b/doc/scripts/builtins.rst
index ba786ba0d2..cf62299783 100644
--- a/doc/scripts/builtins.rst
+++ b/doc/scripts/builtins.rst
@@ -505,15 +505,14 @@ The Bro scripting language supports the following built-in types.
     A hook is another flavor of function that shares characteristics of
     both a :bro:type:`function` and a :bro:type:`event`.  They are like
     events in that many handler bodies can be defined for the same hook
-    identifier, they have no return vale, and the order of execution
-    can be enforced with :bro:attr:`&priority`.  They are more like
-    functions in the way they are invoked/called, because, unlike
-    events, their execution is immediate and they do not get scheduled
-    through an event queue.  Also, a unique feature of a hook is that
-    a given hook handler body can short-circuit the execution of
-    remaining hook handlers simply by exiting from the body as a result
-    of a ``break`` statement (as opposed to a ``return`` or just
-    reaching the end of the body).
+    identifier and the order of execution can be enforced with
+    :bro:attr:`&priority`.  They are more like functions in the way they
+    are invoked/called, because, unlike events, their execution is
+    immediate and they do not get scheduled through an event queue.
+    Also, a unique feature of a hook is that a given hook handler body
+    can short-circuit the execution of remaining hook handlers simply by
+    exiting from the body as a result of a ``break`` statement (as
+    opposed to a ``return`` or just reaching the end of the body).
 
     A hook type is declared like::
 
@@ -553,12 +552,12 @@ The Bro scripting language supports the following built-in types.
     a hook type isn't strictly required, when it is provided, the
     argument types must match.
 
-    To invoke immediate execution of all hook handler bodies, a ``hook``
-    statement must be used:
+    To invoke immediate execution of all hook handler bodies, they
+    can be called just like a function:
 
     .. code:: bro
 
-        hook myhook("hi");
+        myhook("hi");
 
     And the output would like like::
 
@@ -568,6 +567,12 @@ The Bro scripting language supports the following built-in types.
     Note how the modification to arguments can be seen by remaining
     hook handlers.
 
+    The return value of a hook call is an implicit :bro:type:`bool`
+    value with ``T`` meaning that all handlers for the hook were
+    executed and ``F`` meaning that only some of the handlers may have
+    executed due to one handler body exiting as a result of a ``break``
+    statement.
+
 Attributes
 ----------
 
diff --git a/src/Expr.cc b/src/Expr.cc
index 733e0fe9a5..a7075566aa 100644
--- a/src/Expr.cc
+++ b/src/Expr.cc
@@ -4374,7 +4374,7 @@ bool InExpr::DoUnserialize(UnserialInfo* info)
 	return true;
 	}
 
-CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args, bool in_hook)
+CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args)
 : Expr(EXPR_CALL)
 	{
 	func = arg_func;
@@ -4415,13 +4415,8 @@ CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args, bool in_hook)
 				break;
 
 			case FUNC_FLAVOR_HOOK:
-				// It's fine to not have a yield if it's known that the call
-				// is being done from a hook statement.
-				if ( ! in_hook )
-					{
-					Error("hook called in expression, use hook statement instead");
-					SetError();
-					}
+				Error("hook has no yield type");
+				SetError();
 				break;
 
 			default:
diff --git a/src/Expr.h b/src/Expr.h
index bd4824f8ee..c16cf86612 100644
--- a/src/Expr.h
+++ b/src/Expr.h
@@ -959,7 +959,7 @@ protected:
 
 class CallExpr : public Expr {
 public:
-	CallExpr(Expr* func, ListExpr* args, bool in_hook = false);
+	CallExpr(Expr* func, ListExpr* args);
 	~CallExpr();
 
 	Expr* Func() const	{ return func; }
diff --git a/src/Func.cc b/src/Func.cc
index 27acce4f04..e3675ebc2c 100644
--- a/src/Func.cc
+++ b/src/Func.cc
@@ -349,16 +349,31 @@ Val* BroFunc::Call(val_list* args, Frame* parent) const
 			break;
 			}
 
-		if ( flow == FLOW_BREAK && Flavor() == FUNC_FLAVOR_HOOK )
+		if ( Flavor() == FUNC_FLAVOR_HOOK )
 			{
-			// short-circuit execution of remaining hook handler bodies
-			break;
+			// Ignore any return values of hook bodies, final return value
+			// depends on whether a body returns as a result of break statement
+			Unref(result);
+			result = 0;
+
+			if ( flow == FLOW_BREAK )
+				{
+				// short-circuit execution of remaining hook handler bodies
+				result = new Val(false, TYPE_BOOL);
+				break;
+				}
 			}
 		}
 
+	if ( Flavor() == FUNC_FLAVOR_HOOK )
+		{
+		if ( ! result )
+			result = new Val(true, TYPE_BOOL);
+		}
+
 	// Warn if the function returns something, but we returned from
 	// the function without an explicit return, or without a value.
-	if ( FType()->YieldType() && FType()->YieldType()->Tag() != TYPE_VOID &&
+	else if ( FType()->YieldType() && FType()->YieldType()->Tag() != TYPE_VOID &&
 	     (flow != FLOW_RETURN /* we fell off the end */ ||
 	      ! result /* explicit return with no result */) &&
 	     ! f->HasDelayed() )
diff --git a/src/SerialTypes.h b/src/SerialTypes.h
index a18c9bcc65..c47ff19298 100644
--- a/src/SerialTypes.h
+++ b/src/SerialTypes.h
@@ -165,7 +165,6 @@ SERIAL_STMT(EVENT_BODY_LIST, 16)
 SERIAL_STMT(INIT_STMT, 17)
 SERIAL_STMT(NULL_STMT, 18)
 SERIAL_STMT(WHEN_STMT, 19)
-SERIAL_STMT(HOOK_STMT, 20)
 
 #define SERIAL_TYPE(name, val) SERIAL_CONST(name, val, BRO_TYPE)
 SERIAL_TYPE(BRO_TYPE, 1)
diff --git a/src/Stmt.cc b/src/Stmt.cc
index 0a5ae16ef6..c65b44a9bd 100644
--- a/src/Stmt.cc
+++ b/src/Stmt.cc
@@ -23,7 +23,7 @@ const char* stmt_name(BroStmtTag t)
 		"print", "event", "expr", "if", "when", "switch",
 		"for", "next", "break", "return", "add", "delete",
 		"list", "bodylist",
-		"<init>", "hook",
+		"<init>",
 		"null",
 	};
 
@@ -933,52 +933,6 @@ bool EventStmt::DoUnserialize(UnserialInfo* info)
 	return event_expr != 0;
 	}
 
-HookStmt::HookStmt(CallExpr* arg_e) : ExprStmt(STMT_HOOK, arg_e)
-	{
-	call_expr = arg_e;
-	}
-
-Val* HookStmt::Exec(Frame* f, stmt_flow_type& flow) const
-	{
-	RegisterAccess();
-
-	Val* ret = call_expr->Eval(f);
-	Unref(ret);
-
-	flow = FLOW_NEXT;
-
-	return 0;
-	}
-
-TraversalCode HookStmt::Traverse(TraversalCallback* cb) const
-	{
-	TraversalCode tc = cb->PreStmt(this);
-	HANDLE_TC_STMT_PRE(tc);
-
-	// call expr is stored in base class's "e" field.
-	tc = e->Traverse(cb);
-	HANDLE_TC_STMT_PRE(tc);
-
-	tc = cb->PostStmt(this);
-	HANDLE_TC_STMT_POST(tc);
-	}
-
-IMPLEMENT_SERIAL(HookStmt, SER_HOOK_STMT);
-
-bool HookStmt::DoSerialize(SerialInfo* info) const
-	{
-	DO_SERIALIZE(SER_HOOK_STMT, ExprStmt);
-	return call_expr->Serialize(info);
-	}
-
-bool HookStmt::DoUnserialize(UnserialInfo* info)
-	{
-	DO_UNSERIALIZE(ExprStmt);
-
-	call_expr = (CallExpr*) Expr::Unserialize(info, EXPR_CALL);
-	return call_expr != 0;
-	}
-
 ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr)
 : ExprStmt(STMT_FOR, loop_expr)
 	{
@@ -1378,7 +1332,10 @@ ReturnStmt::ReturnStmt(Expr* arg_e) : ExprStmt(STMT_RETURN, arg_e)
 		}
 
 	else if ( ! e )
-		Error("return statement needs expression");
+		{
+		if ( ft->Flavor() != FUNC_FLAVOR_HOOK )
+			Error("return statement needs expression");
+		}
 
 	else
 		(void) check_and_promote_expr(e, yt);
@@ -1990,7 +1947,6 @@ int same_stmt(const Stmt* s1, const Stmt* s2)
 	case STMT_RETURN:
 	case STMT_EXPR:
 	case STMT_EVENT:
-	case STMT_HOOK:
 		{
 		const ExprStmt* e1 = (const ExprStmt*) s1;
 		const ExprStmt* e2 = (const ExprStmt*) s2;
diff --git a/src/Stmt.h b/src/Stmt.h
index 68bb8d6425..7c3b42609b 100644
--- a/src/Stmt.h
+++ b/src/Stmt.h
@@ -286,24 +286,6 @@ protected:
 	EventExpr* event_expr;
 };
 
-class HookStmt : public ExprStmt {
-public:
-	HookStmt(CallExpr* e);
-
-	Val* Exec(Frame* f, stmt_flow_type& flow) const;
-
-	TraversalCode Traverse(TraversalCallback* cb) const;
-
-protected:
-	friend class Stmt;
-
-	HookStmt() { call_expr = 0; }
-
-	DECLARE_SERIAL(HookStmt);
-
-	CallExpr* call_expr;
-};
-
 class ForStmt : public ExprStmt {
 public:
 	ForStmt(id_list* loop_vars, Expr* loop_expr);
diff --git a/src/StmtEnums.h b/src/StmtEnums.h
index fa5b70389d..f431e3fea1 100644
--- a/src/StmtEnums.h
+++ b/src/StmtEnums.h
@@ -15,7 +15,7 @@ typedef enum {
 	STMT_RETURN,
 	STMT_ADD, STMT_DELETE,
 	STMT_LIST, STMT_EVENT_BODY_LIST,
-	STMT_INIT, STMT_HOOK,
+	STMT_INIT,
 	STMT_NULL
 #define NUM_STMTS (int(STMT_NULL) + 1)
 } BroStmtTag;
diff --git a/src/Var.cc b/src/Var.cc
index 9c4fb5b978..68be18b55e 100644
--- a/src/Var.cc
+++ b/src/Var.cc
@@ -294,12 +294,12 @@ void add_type(ID* id, BroType* t, attr_list* attr, int /* is_event */)
 void begin_func(ID* id, const char* module_name, function_flavor flavor,
 		int is_redef, FuncType* t)
 	{
-	if ( flavor == FUNC_FLAVOR_EVENT || flavor == FUNC_FLAVOR_HOOK )
+	if ( flavor == FUNC_FLAVOR_EVENT )
 		{
 		const BroType* yt = t->YieldType();
 
 		if ( yt && yt->Tag() != TYPE_VOID )
-			id->Error("event/hook cannot yield a value", t);
+			id->Error("event cannot yield a value", t);
 
 		t->ClearYieldType(flavor);
 		}
diff --git a/src/parse.y b/src/parse.y
index b4eee1a56c..39673cacc2 100644
--- a/src/parse.y
+++ b/src/parse.y
@@ -2,7 +2,7 @@
 // See the file "COPYING" in the main distribution directory for copyright.
 %}
 
-%expect 88
+%expect 87
 
 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY
 %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF
@@ -56,7 +56,6 @@
 %type <re> pattern
 %type <expr> expr init anonymous_function
 %type <event_expr> event
-%type <call_expr> hook
 %type <stmt> stmt stmt_list func_body for_head
 %type <type> type opt_type enum_body
 %type <func_type> func_hdr func_params
@@ -212,7 +211,6 @@ static std::list<std::string>* concat_opt_docs (std::list<std::string>* pre,
 	Val* val;
 	RE_Matcher* re;
 	Expr* expr;
-	CallExpr* call_expr;
 	EventExpr* event_expr;
 	Stmt* stmt;
 	ListExpr* list;
@@ -875,7 +873,7 @@ type:
 	|	TOK_HOOK '(' formal_args ')'
 				{
 				set_location(@1, @3);
-				$$ = new FuncType($3, 0, FUNC_FLAVOR_HOOK);
+				$$ = new FuncType($3, base_type(TYPE_BOOL), FUNC_FLAVOR_HOOK);
 				}
 
 	|	TOK_FILE TOK_OF type
@@ -1209,6 +1207,8 @@ func_hdr:
 			}
 	|	TOK_HOOK def_global_id func_params
 			{
+			$3->ClearYieldType(FUNC_FLAVOR_HOOK);
+			$3->SetYieldType(base_type(TYPE_BOOL));
 			begin_func($2, current_module.c_str(),
 				   FUNC_FLAVOR_HOOK, 0, $3);
 			$$ = $3;
@@ -1372,14 +1372,6 @@ stmt:
 			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_HOOK hook ';' opt_no_test
-			{
-			set_location(@1, @4);
-			$$ = new HookStmt($2);
-			if ( ! $4 )
-			    brofiler.AddStmt($$);
-			}
-
 	|	TOK_IF '(' expr ')' stmt
 			{
 			set_location(@1, @4);
@@ -1533,14 +1525,6 @@ event:
 			}
 	;
 
-hook:
-		expr '(' opt_expr_list ')'
-			{
-			set_location(@1, @4);
-			$$ = new CallExpr($1, $3, true);
-			}
-	;
-
 case_list:
 		case_list case
 			{ $1->append($2); }
diff --git a/testing/btest/Baseline/language.hook/out b/testing/btest/Baseline/language.hook/out
index 10688acc3b..18828ad15f 100644
--- a/testing/btest/Baseline/language.hook/out
+++ b/testing/btest/Baseline/language.hook/out
@@ -1,7 +1,15 @@
 myhook, &priority=10, [a=1156, b=hello world]
+myhook return F
+myhook return T
 myhook, &priority=5, [a=37, b=goobye world]
+F
 myhook3, 8
+T
 myhook4, 2
 myhook4, 1
+T
 myhook, &priority=10, [a=2, b=it works]
+myhook return F
+myhook return T
 myhook, &priority=5, [a=37, b=goobye world]
+F
diff --git a/testing/btest/Baseline/language.invalid_hook/out b/testing/btest/Baseline/language.invalid_hook/out
deleted file mode 100644
index 167d62ccc8..0000000000
--- a/testing/btest/Baseline/language.invalid_hook/out
+++ /dev/null
@@ -1 +0,0 @@
-error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.invalid_hook/invalid_hook.bro, line 15: hook called in expression, use hook statement instead (myhook(nope))
diff --git a/testing/btest/core/leaks/hook.bro b/testing/btest/core/leaks/hook.bro
index eadb406e71..d9dcbff369 100644
--- a/testing/btest/core/leaks/hook.bro
+++ b/testing/btest/core/leaks/hook.bro
@@ -39,7 +39,25 @@ hook myhook(r: rec) &priority=10
 	r$b = "goobye world";
 	# returning from the handler early, is fine, remaining handlers still run.
 	return;
-	print "ERROR: break statement should return from hook handler body";
+	print "ERROR: return statement should return from hook handler body";
+	}
+
+hook myhook(r: rec) &priority=9
+	{
+	print "myhook return F";
+	# return value is ignored, remaining handlers still run, final return
+	# value is whether any hook body returned via break statement
+	return F;
+	print "ERROR: return statement should return from hook handler body";
+	}
+
+hook myhook(r: rec) &priority=8
+	{
+	print "myhook return T";
+	# return value is ignored, remaining handlers still run, final return
+	# value is whether any hook body returned via break statement
+	return T;
+	print "ERROR: return statement should return from hook handler body";
 	}
 
 # hook function doesn't need a declaration, we can go straight to defining
@@ -63,16 +81,16 @@ event new_connection(c: connection)
 	{
 	print "new_connection", c$id;
 
-	hook myhook([$a=1156, $b="hello world"]);
+	print myhook([$a=1156, $b="hello world"]);
 
 	# A hook with no handlers is fine, it's just a no-op.
-	hook myhook2("nope");
+	print myhook2("nope");
 
-	hook myhook3(8);
-	hook myhook4();
+	print myhook3(8);
+	print myhook4();
 
 	# A hook can be treated like other data types and doesn't have to be
 	# invoked directly by name.
 	local h = myhook;
-	hook h([$a=2, $b="it works"]);
+	print h([$a=2, $b="it works"]);
 	}
diff --git a/testing/btest/language/hook.bro b/testing/btest/language/hook.bro
index 8f7a85ce95..eedd2ff056 100644
--- a/testing/btest/language/hook.bro
+++ b/testing/btest/language/hook.bro
@@ -34,7 +34,25 @@ hook myhook(r: rec) &priority=10
 	r$b = "goobye world";
 	# returning from the handler early, is fine, remaining handlers still run.
 	return;
-	print "ERROR: break statement should return from hook handler body";
+	print "ERROR: return statement should return from hook handler body";
+	}
+
+hook myhook(r: rec) &priority=9
+	{
+	print "myhook return F";
+	# return value is ignored, remaining handlers still run, final return
+	# value is whether any hook body returned via break statement
+	return F;
+	print "ERROR: return statement should return from hook handler body";
+	}
+
+hook myhook(r: rec) &priority=8
+	{
+	print "myhook return T";
+	# return value is ignored, remaining handlers still run, final return
+	# value is whether any hook body returned via break statement
+	return T;
+	print "ERROR: return statement should return from hook handler body";
 	}
 
 # hook function doesn't need a declaration, we can go straight to defining
@@ -56,16 +74,16 @@ hook myhook4() &priority=2
 
 event bro_init()
 	{
-	hook myhook([$a=1156, $b="hello world"]);
+	print myhook([$a=1156, $b="hello world"]);
 
 	# A hook with no handlers is fine, it's just a no-op.
-	hook myhook2("nope");
+	print myhook2("nope");
 
-	hook myhook3(8);
-	hook myhook4();
+	print myhook3(8);
+	print myhook4();
 
 	# A hook can be treated like other data types and doesn't have to be
 	# invoked directly by name.
 	local h = myhook;
-	hook h([$a=2, $b="it works"]);
+	print h([$a=2, $b="it works"]);
 	}
diff --git a/testing/btest/language/invalid_hook.bro b/testing/btest/language/invalid_hook.bro
deleted file mode 100644
index 0dbbfd1b6f..0000000000
--- a/testing/btest/language/invalid_hook.bro
+++ /dev/null
@@ -1,16 +0,0 @@
-# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1
-# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
-
-global myhook: hook(s: string);
-
-hook myhook(s: string)
-	{
-	print "myhook", s;
-	}
-
-event bro_init()
-	{
-	# hooks must be invoked with a "hook", statement.  They have no return
-	# value and don't make sense to evaluate as arbitrary expressions.
-	local r = myhook("nope");
-	}

From 3b3b05fbbb87f938399f86cd9e75fe636ced421e Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@ncsa.illinois.edu>
Date: Thu, 29 Nov 2012 10:07:38 -0600
Subject: [PATCH 2/3] Clarification in hook documentation.

---
 doc/scripts/builtins.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/scripts/builtins.rst b/doc/scripts/builtins.rst
index cf62299783..0e474e9677 100644
--- a/doc/scripts/builtins.rst
+++ b/doc/scripts/builtins.rst
@@ -548,9 +548,9 @@ The Bro scripting language supports the following built-in types.
             print "not going to happen", s;
             }
 
-    Note that, although the first (forward) declaration of ``myhook`` as
-    a hook type isn't strictly required, when it is provided, the
-    argument types must match.
+    Note that the first (forward) declaration of ``myhook`` as a hook
+    type isn't strictly required.  Argument types must match for all
+    hook handlers and any forward declaration of a given hook.
 
     To invoke immediate execution of all hook handler bodies, they
     can be called just like a function:

From 4e85fe0454c853ea6bab90064c45de32ae35efe4 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@ncsa.illinois.edu>
Date: Fri, 30 Nov 2012 15:39:00 -0600
Subject: [PATCH 3/3] Change hook calls to only be allowed when preceded by
 "hook" keyword.

---
 doc/scripts/builtins.rst                      | 14 +++-
 src/Expr.cc                                   |  9 +-
 src/Expr.h                                    |  2 +-
 src/parse.y                                   | 15 +++-
 testing/btest/Baseline/language.hook/out      |  3 +
 .../Baseline/language.hook_calls/invalid.out  | 10 +++
 .../Baseline/language.hook_calls/valid.out    | 42 ++++++++++
 testing/btest/core/leaks/hook.bro             | 14 ++--
 testing/btest/language/hook.bro               | 14 ++--
 testing/btest/language/hook_calls.bro         | 82 +++++++++++++++++++
 10 files changed, 188 insertions(+), 17 deletions(-)
 create mode 100644 testing/btest/Baseline/language.hook_calls/invalid.out
 create mode 100644 testing/btest/Baseline/language.hook_calls/valid.out
 create mode 100644 testing/btest/language/hook_calls.bro

diff --git a/doc/scripts/builtins.rst b/doc/scripts/builtins.rst
index 0e474e9677..322aafc2cf 100644
--- a/doc/scripts/builtins.rst
+++ b/doc/scripts/builtins.rst
@@ -553,13 +553,21 @@ The Bro scripting language supports the following built-in types.
     hook handlers and any forward declaration of a given hook.
 
     To invoke immediate execution of all hook handler bodies, they
-    can be called just like a function:
+    are called similarly to a function, except preceded by the ``hook``
+    keyword:
 
     .. code:: bro
 
-        myhook("hi");
+        hook myhook("hi");
 
-    And the output would like like::
+    or
+
+    .. code:: bro
+
+        if ( hook myhook("hi") )
+            print "all handlers ran";
+
+    And the output would look like::
 
         priority 10 myhook handler, hi
         break out of myhook handling, bye
diff --git a/src/Expr.cc b/src/Expr.cc
index a7075566aa..a910e0d2fb 100644
--- a/src/Expr.cc
+++ b/src/Expr.cc
@@ -4374,7 +4374,7 @@ bool InExpr::DoUnserialize(UnserialInfo* info)
 	return true;
 	}
 
-CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args)
+CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args, bool in_hook)
 : Expr(EXPR_CALL)
 	{
 	func = arg_func;
@@ -4394,6 +4394,13 @@ CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args)
 		return;
 		}
 
+	if ( func_type->AsFuncType()->Flavor() == FUNC_FLAVOR_HOOK && ! in_hook )
+		{
+		func->Error("hook cannot be called directly, use hook operator");
+		SetError();
+		return;
+		}
+
 	if ( ! func_type->MatchesIndex(args) )
 		SetError("argument type mismatch in function call");
 	else
diff --git a/src/Expr.h b/src/Expr.h
index c16cf86612..bd4824f8ee 100644
--- a/src/Expr.h
+++ b/src/Expr.h
@@ -959,7 +959,7 @@ protected:
 
 class CallExpr : public Expr {
 public:
-	CallExpr(Expr* func, ListExpr* args);
+	CallExpr(Expr* func, ListExpr* args, bool in_hook = false);
 	~CallExpr();
 
 	Expr* Func() const	{ return func; }
diff --git a/src/parse.y b/src/parse.y
index 39673cacc2..0385c1c4e1 100644
--- a/src/parse.y
+++ b/src/parse.y
@@ -2,7 +2,7 @@
 // See the file "COPYING" in the main distribution directory for copyright.
 %}
 
-%expect 87
+%expect 88
 
 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY
 %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF
@@ -32,6 +32,7 @@
 
 %token TOK_NO_TEST
 
+%nonassoc TOK_HOOK
 %left ',' '|'
 %right '=' TOK_ADD_TO TOK_REMOVE_FROM
 %right '?' ':'
@@ -118,6 +119,7 @@ extern const char* g_curr_debug_error;
 
 #define YYLTYPE yyltype
 
+static int in_hook = 0;
 int in_init = 0;
 int in_record = 0;
 bool resolving_global_ID = false;
@@ -515,7 +517,16 @@ expr:
 	|	expr '(' opt_expr_list ')'
 			{
 			set_location(@1, @4);
-			$$ = new CallExpr($1, $3);
+			$$ = new CallExpr($1, $3, in_hook > 0);
+			}
+
+	|	TOK_HOOK { ++in_hook; } expr
+			{
+			--in_hook;
+			set_location(@1, @3);
+			if ( $3->Tag() != EXPR_CALL )
+				$3->Error("not a valid hook call expression");
+			$$ = $3;
 			}
 
 	|	expr TOK_HAS_FIELD TOK_ID
diff --git a/testing/btest/Baseline/language.hook/out b/testing/btest/Baseline/language.hook/out
index 18828ad15f..bef25193b8 100644
--- a/testing/btest/Baseline/language.hook/out
+++ b/testing/btest/Baseline/language.hook/out
@@ -8,6 +8,9 @@ T
 myhook4, 2
 myhook4, 1
 T
+myhook4, 2
+myhook4, 1
+myhook4 all handlers ran
 myhook, &priority=10, [a=2, b=it works]
 myhook return F
 myhook return T
diff --git a/testing/btest/Baseline/language.hook_calls/invalid.out b/testing/btest/Baseline/language.hook_calls/invalid.out
new file mode 100644
index 0000000000..3412c1900e
--- /dev/null
+++ b/testing/btest/Baseline/language.hook_calls/invalid.out
@@ -0,0 +1,10 @@
+error in ./invalid.bro, line 9: hook cannot be called directly, use hook operator (myhook)
+warning in ./invalid.bro, line 9: expression value ignored (myhook(3))
+error in ./invalid.bro, line 10: hook cannot be called directly, use hook operator (myhook)
+error in ./invalid.bro, line 11: hook cannot be called directly, use hook operator (myhook)
+error in ./invalid.bro, line 12: not a valid hook call expression (2 + 2)
+warning in ./invalid.bro, line 12: expression value ignored (2 + 2)
+error in ./invalid.bro, line 13: not a valid hook call expression (2 + 2)
+error in ./invalid.bro, line 15: hook cannot be called directly, use hook operator (h)
+warning in ./invalid.bro, line 15: expression value ignored (h(3))
+error in ./invalid.bro, line 16: hook cannot be called directly, use hook operator (h)
diff --git a/testing/btest/Baseline/language.hook_calls/valid.out b/testing/btest/Baseline/language.hook_calls/valid.out
new file mode 100644
index 0000000000..c711115315
--- /dev/null
+++ b/testing/btest/Baseline/language.hook_calls/valid.out
@@ -0,0 +1,42 @@
+myhook(), 3
+other myhook(), 3
+myhook(), 3
+other myhook(), 3
+T
+myhook(), 0
+F
+-----------
+indirect()
+myhook(), 3
+other myhook(), 3
+indirect()
+myhook(), 3
+other myhook(), 3
+T
+-----------
+really_indirect()
+indirect()
+myhook(), 3
+other myhook(), 3
+really_indirect()
+indirect()
+myhook(), 3
+other myhook(), 3
+T
+-----------
+myhook(), 3
+other myhook(), 3
+myhook(), 3
+other myhook(), 3
+T
+myhook(), 3
+other myhook(), 3
+yes
+myhook(), 0
+double yes
+-----------
+myhook(), 3
+other myhook(), 3
+myhook(), 3
+other myhook(), 3
+T
diff --git a/testing/btest/core/leaks/hook.bro b/testing/btest/core/leaks/hook.bro
index d9dcbff369..9234184317 100644
--- a/testing/btest/core/leaks/hook.bro
+++ b/testing/btest/core/leaks/hook.bro
@@ -81,16 +81,20 @@ event new_connection(c: connection)
 	{
 	print "new_connection", c$id;
 
-	print myhook([$a=1156, $b="hello world"]);
+	print hook myhook([$a=1156, $b="hello world"]);
 
 	# A hook with no handlers is fine, it's just a no-op.
-	print myhook2("nope");
+	print hook myhook2("nope");
 
-	print myhook3(8);
-	print myhook4();
+	print hook myhook3(8);
+	print hook myhook4();
+	if ( hook myhook4() )
+		{
+		print "myhook4 all handlers ran";
+		}
 
 	# A hook can be treated like other data types and doesn't have to be
 	# invoked directly by name.
 	local h = myhook;
-	print h([$a=2, $b="it works"]);
+	print hook h([$a=2, $b="it works"]);
 	}
diff --git a/testing/btest/language/hook.bro b/testing/btest/language/hook.bro
index eedd2ff056..9c9ab30c18 100644
--- a/testing/btest/language/hook.bro
+++ b/testing/btest/language/hook.bro
@@ -74,16 +74,20 @@ hook myhook4() &priority=2
 
 event bro_init()
 	{
-	print myhook([$a=1156, $b="hello world"]);
+	print hook myhook([$a=1156, $b="hello world"]);
 
 	# A hook with no handlers is fine, it's just a no-op.
-	print myhook2("nope");
+	print hook myhook2("nope");
 
-	print myhook3(8);
-	print myhook4();
+	print hook myhook3(8);
+	print hook myhook4();
+	if ( hook myhook4() )
+		{
+		print "myhook4 all handlers ran";
+		}
 
 	# A hook can be treated like other data types and doesn't have to be
 	# invoked directly by name.
 	local h = myhook;
-	print h([$a=2, $b="it works"]);
+	print hook h([$a=2, $b="it works"]);
 	}
diff --git a/testing/btest/language/hook_calls.bro b/testing/btest/language/hook_calls.bro
new file mode 100644
index 0000000000..41ef6f52ae
--- /dev/null
+++ b/testing/btest/language/hook_calls.bro
@@ -0,0 +1,82 @@
+# @TEST-EXEC: bro -b valid.bro >valid.out
+# @TEST-EXEC: btest-diff valid.out
+# @TEST-EXEC-FAIL: bro -b invalid.bro > invalid.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff invalid.out
+
+# hook functions must be called using the "hook" keyword as an operator...
+
+@TEST-START-FILE valid.bro
+hook myhook(i: count)
+    {
+    print "myhook()", i;
+	if ( i == 0 ) break;
+    }
+
+hook myhook(i: count) &priority=-1
+    {
+    print "other myhook()", i;
+	}
+
+function indirect(): hook(i: count)
+    {
+    print "indirect()";
+    return myhook;
+    }
+
+function really_indirect(): function(): hook(i: count)
+    {
+    print "really_indirect()";
+    return indirect;
+    }
+
+global t: table[count] of hook(i: count) = {
+    [0] = myhook,
+};
+
+event bro_init()
+	{
+	hook myhook(3);
+	print hook myhook(3);
+	print hook myhook(0);
+	print "-----------";
+	hook indirect()(3);
+	print hook indirect()(3);
+	print "-----------";
+	hook really_indirect()()(3);
+	print hook really_indirect()()(3);
+	print "-----------";
+	local h = t[0];
+	hook h(3);
+	print hook h(3);
+	if ( hook h(3) )
+		print "yes";
+	if ( ! hook h(0) )
+		print "double yes";
+	print "-----------";
+	hook t[0](3);
+	print hook t[0](3);
+	}
+
+@TEST-END-FILE
+
+@TEST-START-FILE invalid.bro
+hook myhook(i: count)
+	{
+	print "myhook()", i;
+	if ( i == 0 ) break;
+	}
+
+event bro_init()
+	{
+	myhook(3);
+	print myhook(3);
+	print myhook(0);
+	hook 2+2;
+	print hook 2+2;
+	local h = myhook;
+	h(3);
+	if ( h(3) )
+		print "hmm";
+	print "done";
+	}
+@TEST-END-FILE