Merge remote-tracking branch 'origin/topic/jsiwek/no-switch-fallthrough'

* origin/topic/jsiwek/no-switch-fallthrough:
  Add "fallthrough" keyword, require a flow statement to end case blocks.
  Disable automatic case fallthrough in switch stmts.  Addresses #754.

I've added a test for the error case where no break/fallthrough/return
is given.

Closes #754.

src/SerialTypes.h

@@ -171,6 +171,7 @@ SERIAL_STMT(EVENT_BODY_LIST, 16)
 SERIAL_STMT(INIT_STMT, 17)
 SERIAL_STMT(NULL_STMT, 18)
 SERIAL_STMT(WHEN_STMT, 19)
+SERIAL_STMT(FALLTHROUGH_STMT, 20)
 
 #define SERIAL_TYPE(name, val) SERIAL_CONST(name, val, BRO_TYPE)
 SERIAL_TYPE(BRO_TYPE, 1)

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>",
+		"<init>", "fallthrough",
 		"null",
 	};
 
@@ -584,6 +584,32 @@ bool IfStmt::DoUnserialize(UnserialInfo* info)
 	return s2 != 0;
 	}
 
+static BroStmtTag get_last_stmt_tag(const Stmt* stmt)
+	{
+	if ( ! stmt )
+		return STMT_NULL;
+
+	if ( stmt->Tag() != STMT_LIST )
+		return stmt->Tag();
+
+	const StmtList* stmts = stmt->AsStmtList();
+	int len = stmts->Stmts().length();
+
+	if ( len == 0 )
+		return STMT_LIST;
+
+	return get_last_stmt_tag(stmts->Stmts()[len - 1]);
+	}
+
+Case::Case(ListExpr* c, Stmt* arg_s)
+    : cases(simplify_expr_list(c, SIMPLIFY_GENERAL)), s(arg_s)
+	{
+	BroStmtTag t = get_last_stmt_tag(Body());
+
+	if ( t != STMT_BREAK && t != STMT_FALLTHROUGH && t != STMT_RETURN )
+		Error("case block must end in break/fallthrough/return statement");
+	}
+
 Case::~Case()
 	{
 	Unref(cases);
@@ -802,15 +828,12 @@ Val* SwitchStmt::DoExec(Frame* f, Val* v, stmt_flow_type& flow) const
 		flow = FLOW_NEXT;
 		rval = c->Body()->Exec(f, flow);
 
-		if ( flow == FLOW_BREAK )
+		if ( flow == FLOW_BREAK  || flow == FLOW_RETURN )
-			{
-			flow = FLOW_NEXT;
 			break;
 		}
 
-		if ( flow == FLOW_RETURN )
+	if ( flow != FLOW_RETURN )
-			break;
+		flow = FLOW_NEXT;
-		}
 
 	return rval;
 	}
@@ -1467,6 +1490,47 @@ bool BreakStmt::DoUnserialize(UnserialInfo* info)
 	return true;
 	}
 
+Val* FallthroughStmt::Exec(Frame* /* f */, stmt_flow_type& flow) const
+	{
+	RegisterAccess();
+	flow = FLOW_FALLTHROUGH;
+	return 0;
+	}
+
+int FallthroughStmt::IsPure() const
+	{
+	return 1;
+	}
+
+void FallthroughStmt::Describe(ODesc* d) const
+	{
+	Stmt::Describe(d);
+	Stmt::DescribeDone(d);
+	}
+
+TraversalCode FallthroughStmt::Traverse(TraversalCallback* cb) const
+	{
+	TraversalCode tc = cb->PreStmt(this);
+	HANDLE_TC_STMT_PRE(tc);
+
+	tc = cb->PostStmt(this);
+	HANDLE_TC_STMT_POST(tc);
+	}
+
+IMPLEMENT_SERIAL(FallthroughStmt, SER_FALLTHROUGH_STMT);
+
+bool FallthroughStmt::DoSerialize(SerialInfo* info) const
+	{
+	DO_SERIALIZE(SER_FALLTHROUGH_STMT, Stmt);
+	return true;
+	}
+
+bool FallthroughStmt::DoUnserialize(UnserialInfo* info)
+	{
+	DO_UNSERIALIZE(Stmt);
+	return true;
+	}
+
 ReturnStmt::ReturnStmt(Expr* arg_e) : ExprStmt(STMT_RETURN, arg_e)
 	{
 	Scope* s = current_scope();

src/Stmt.h

@@ -46,6 +46,12 @@ public:
 		return (StmtList*) this;
 		}
 
+	const StmtList* AsStmtList() const
+		{
+		CHECK_TAG(tag, STMT_LIST, "Stmt::AsStmtList", stmt_name)
+		return (const StmtList*) this;
+		}
+
 	ForStmt* AsForStmt()
 		{
 		CHECK_TAG(tag, STMT_FOR, "Stmt::AsForStmt", stmt_name)
@@ -189,8 +195,7 @@ protected:
 
 class Case : public BroObj {
 public:
-	Case(ListExpr* c, Stmt* arg_s) :
+	Case(ListExpr* c, Stmt* arg_s);
-	    cases(simplify_expr_list(c,SIMPLIFY_GENERAL)), s(arg_s) { }
 	~Case();
 
 	const ListExpr* Cases() const	{ return cases; }
@@ -365,6 +370,21 @@ protected:
 	DECLARE_SERIAL(BreakStmt);
 };
 
+class FallthroughStmt : public Stmt {
+public:
+	FallthroughStmt() : Stmt(STMT_FALLTHROUGH)	{ }
+
+	Val* Exec(Frame* f, stmt_flow_type& flow) const;
+	int IsPure() const;
+
+	void Describe(ODesc* d) const;
+
+	TraversalCode Traverse(TraversalCallback* cb) const;
+
+protected:
+	DECLARE_SERIAL(FallthroughStmt);
+};
+
 class ReturnStmt : public ExprStmt {
 public:
 	ReturnStmt(Expr* e);

src/StmtEnums.h

@@ -16,6 +16,7 @@ typedef enum {
 	STMT_ADD, STMT_DELETE,
 	STMT_LIST, STMT_EVENT_BODY_LIST,
 	STMT_INIT,
+	STMT_FALLTHROUGH,
 	STMT_NULL
 #define NUM_STMTS (int(STMT_NULL) + 1)
 } BroStmtTag;
@@ -24,7 +25,8 @@ typedef enum {
 	FLOW_NEXT,		// continue on to next statement
 	FLOW_LOOP,		// go to top of loop
 	FLOW_BREAK,		// break out of loop
-	FLOW_RETURN		// return from function
+	FLOW_RETURN,		// return from function
+	FLOW_FALLTHROUGH	// fall through to next switch case
 } stmt_flow_type;
 
 extern const char* stmt_name(BroStmtTag t);

src/parse.y

@@ -8,8 +8,8 @@
 %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF
 %token TOK_BOOL TOK_BREAK TOK_CASE TOK_CONST
 %token TOK_CONSTANT TOK_COPY TOK_COUNT TOK_COUNTER TOK_DEFAULT TOK_DELETE
-%token TOK_DOUBLE TOK_ELSE TOK_ENUM TOK_EVENT TOK_EXPORT TOK_FILE TOK_FOR
+%token TOK_DOUBLE TOK_ELSE TOK_ENUM TOK_EVENT TOK_EXPORT TOK_FALLTHROUGH
-%token TOK_FUNCTION TOK_GLOBAL TOK_HOOK TOK_ID TOK_IF TOK_INT
+%token TOK_FILE TOK_FOR TOK_FUNCTION TOK_GLOBAL TOK_HOOK TOK_ID TOK_IF TOK_INT
 %token TOK_INTERVAL TOK_LIST TOK_LOCAL TOK_MODULE
 %token TOK_NEXT TOK_OF TOK_OPAQUE TOK_PATTERN TOK_PATTERN_TEXT
 %token TOK_PORT TOK_PRINT TOK_RECORD TOK_REDEF
@@ -1436,6 +1436,14 @@ stmt:
 			    brofiler.AddStmt($$);
 			}
 
+	|	TOK_FALLTHROUGH ';' opt_no_test
+			{
+			set_location(@1, @2);
+			$$ = new FallthroughStmt;
+			if ( ! $3 )
+				brofiler.AddStmt($$);
+			}
+
 	|	TOK_RETURN ';' opt_no_test
 			{
 			set_location(@1, @2);

src/scan.l

@@ -282,6 +282,7 @@ else	return TOK_ELSE;
 enum	return TOK_ENUM;
 event	return TOK_EVENT;
 export	return TOK_EXPORT;
+fallthrough	return TOK_FALLTHROUGH;
 file	return TOK_FILE;
 for	return TOK_FOR;
 function	return TOK_FUNCTION;

testing/btest/Baseline/language.switch-incomplete/out

@@ -0,0 +1 @@
+error in /home/robin/bro/master/testing/btest/.tmp/language.switch-incomplete/switch-incomplete.bro, lines 7-8: case block must end in break/fallthrough/return statement (case 1:{ print 1})

testing/btest/core/leaks/switch-statement.bro

@@ -148,17 +148,19 @@ function switch_empty(v: count): string
 	return "n/a";
 	}
 
-function switch_break(v: count): string
+function switch_fallthrough(v: count): string
 	{
 	local rval = "";
 	switch ( v ) {
 	case 1:
 		rval += "test";
+		fallthrough;
 	case 2:
 		rval += "testing";
-		break;
+		fallthrough;
 	case 3:
 		rval += "tested";
+		break;
 	}
 	return rval + "return";
 	}
@@ -169,12 +171,16 @@ function switch_default(v: count): string
 	switch ( v ) {
 	case 1:
 		rval += "1";
+		fallthrough;
 	case 2:
 		rval += "2";
+		break;
 	case 3:
 		rval += "3";
+		fallthrough;
 	default:
 		rval += "d";
+		break;
 	}
 	return rval + "r";
 	}
@@ -185,13 +191,16 @@ function switch_default_placement(v: count): string
 	switch ( v ) {
 	case 1:
 		rval += "1";
+		fallthrough;
 	default:
 		rval += "d";
+		fallthrough;
 	case 2:
 		rval += "2";
 		break;
 	case 3:
 		rval += "3";
+		break;
 	}
 	return rval + "r";
 	}
@@ -251,11 +260,11 @@ event new_connection(c: connection)
 	test_switch( switch_subnet([fe80::1]/96) , "[fe80::0]" );
 	test_switch( switch_subnet(192.168.1.100/16) , "192.168.0.0/16" );
 	test_switch( switch_empty(2) , "n/a" );
-	test_switch( switch_break(1) , "testtestingreturn" );
+	test_switch( switch_fallthrough(1) , "testtestingtestedreturn" );
-	test_switch( switch_break(2) , "testingreturn" );
+	test_switch( switch_fallthrough(2) , "testingtestedreturn" );
-	test_switch( switch_break(3) , "testedreturn" );
+	test_switch( switch_fallthrough(3) , "testedreturn" );
-	test_switch( switch_default(1) , "123dr" );
+	test_switch( switch_default(1) , "12r" );
-	test_switch( switch_default(2) , "23dr" );
+	test_switch( switch_default(2) , "2r" );
 	test_switch( switch_default(3) , "3dr" );
 	test_switch( switch_default(4) , "dr" );
 	test_switch( switch_default_placement(1) , "1d2r" );

testing/btest/language/switch-incomplete.bro

@@ -0,0 +1,12 @@
+# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1
+# @TEST-EXEC: btest-diff out
+
+event bro_init()
+	{
+	switch ( 1 ) {
+	case 1:
+		print 1;
+		# error: neither break/fallthrough/return here.
+	}
+	}
+

testing/btest/language/switch-statement.bro

@@ -143,17 +143,19 @@ function switch_empty(v: count): string
 	return "n/a";
 	}
 
-function switch_break(v: count): string
+function switch_fallthrough(v: count): string
 	{
 	local rval = "";
 	switch ( v ) {
 	case 1:
 		rval += "test";
+		fallthrough;
 	case 2:
 		rval += "testing";
-		break;
+		fallthrough;
 	case 3:
 		rval += "tested";
+		break;
 	}
 	return rval + "return";
 	}
@@ -164,12 +166,16 @@ function switch_default(v: count): string
 	switch ( v ) {
 	case 1:
 		rval += "1";
+		fallthrough;
 	case 2:
 		rval += "2";
+		break;
 	case 3:
 		rval += "3";
+		fallthrough;
 	default:
 		rval += "d";
+		break;
 	}
 	return rval + "r";
 	}
@@ -180,13 +186,16 @@ function switch_default_placement(v: count): string
 	switch ( v ) {
 	case 1:
 		rval += "1";
+		fallthrough;
 	default:
 		rval += "d";
+		fallthrough;
 	case 2:
 		rval += "2";
 		break;
 	case 3:
 		rval += "3";
+		break;
 	}
 	return rval + "r";
 	}
@@ -246,11 +255,11 @@ event bro_init()
 	test_switch( switch_subnet([fe80::1]/96) , "[fe80::0]" );
 	test_switch( switch_subnet(192.168.1.100/16) , "192.168.0.0/16" );
 	test_switch( switch_empty(2) , "n/a" );
-	test_switch( switch_break(1) , "testtestingreturn" );
+	test_switch( switch_fallthrough(1) , "testtestingtestedreturn" );
-	test_switch( switch_break(2) , "testingreturn" );
+	test_switch( switch_fallthrough(2) , "testingtestedreturn" );
-	test_switch( switch_break(3) , "testedreturn" );
+	test_switch( switch_fallthrough(3) , "testedreturn" );
-	test_switch( switch_default(1) , "123dr" );
+	test_switch( switch_default(1) , "12r" );
-	test_switch( switch_default(2) , "23dr" );
+	test_switch( switch_default(2) , "2r" );
 	test_switch( switch_default(3) , "3dr" );
 	test_switch( switch_default(4) , "dr" );
 	test_switch( switch_default_placement(1) , "1d2r" );