Merge pull request ethereum#3790 from ethereum/empty-structs

Disallow empty structs
TGBP · Mar 30, 2018 · 326d656 · 326d656
2 parents 58c57e4 + ebb1275
commit 326d656
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 2 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -4,6 +4,7 @@ Features:
  * Commandline interface: Error when missing or inaccessible file detected. Suppress it with the ``--ignore-missing`` flag.
  * General: Support accessing dynamic return data in post-byzantium EVMs.
  * Interfaces: Allow overriding external functions in interfaces with public in an implementing contract.
+ * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature).
 
 Bugfixes:
  * Code Generator: Allow ``block.blockhash`` without being called.

diff --git a/docs/grammar.txt b/docs/grammar.txt
@@ -19,7 +19,7 @@ InheritanceSpecifier = UserDefinedTypeName ( '(' Expression ( ',' Expression )*
 StateVariableDeclaration = TypeName ( 'public' | 'internal' | 'private' | 'constant' )? Identifier ('=' Expression)? ';'
 UsingForDeclaration = 'using' Identifier 'for' ('*' | TypeName) ';'
 StructDefinition = 'struct' Identifier '{'
-                     ( VariableDeclaration ';' (VariableDeclaration ';')* )? '}'
+                     ( VariableDeclaration ';' (VariableDeclaration ';')* ) '}'
 
 ModifierDefinition = 'modifier' Identifier ParameterList? Block
 ModifierInvocation = Identifier ( '(' ExpressionList? ')' )?

diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
@@ -255,3 +255,17 @@ bool SyntaxChecker::visit(VariableDeclaration const& _declaration)
 	}
 	return true;
 }
+
+bool SyntaxChecker::visit(StructDefinition const& _struct)
+{
+	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
+
+	if (_struct.members().empty())
+	{
+		if (v050)
+			m_errorReporter.syntaxError(_struct.location(), "Defining empty structs is disallowed.");
+		else
+			m_errorReporter.warning(_struct.location(), "Defining empty structs is deprecated.");
+	}
+	return true;
+}
diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h
@@ -71,6 +71,8 @@ class SyntaxChecker: private ASTConstVisitor
 
 	virtual bool visit(VariableDeclaration const& _declaration) override;
 
+	virtual bool visit(StructDefinition const& _struct) override;
+
 	ErrorReporter& m_errorReporter;
 
 	/// Flag that indicates whether a function modifier actually contains '_'.

diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -7289,7 +7289,7 @@ BOOST_AUTO_TEST_CASE(modifiers_access_storage_pointer)
 {
 	char const* text = R"(
 		contract C {
-			struct S { }
+			struct S { uint a; }
 			modifier m(S storage x) {
 				x;
 				_;

diff --git a/test/libsolidity/syntaxTests/empty_struct.sol b/test/libsolidity/syntaxTests/empty_struct.sol
@@ -0,0 +1,5 @@
+contract test {
+	struct A {}
+}
+// ----
+// Warning: Defining empty structs is deprecated.
diff --git a/test/libsolidity/syntaxTests/empty_struct_050.sol b/test/libsolidity/syntaxTests/empty_struct_050.sol
@@ -0,0 +1,6 @@
+pragma experimental "v0.5.0";
+contract test {
+	struct A {}
+}
+// ----
+// SyntaxError: Defining empty structs is disallowed.