Merge pull request #5526 from ethereum/fixGetterNewCoder

Fix: Disallow structs in getters for old encoder.

6 files changed, 65 insertions, 11 deletions

diff --git a/Changelog.md b/Changelog.md
index 7fd95566..b67bc592 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -18,8 +18,9 @@ Bugfixes:
  * Assembly output: Do not mix in/out jump annotations with arguments.
  * Commandline interface: Fix crash when using ``--ast`` on empty runtime code.
  * Code Generator: Annotate jump from calldata decoder to function as "jump in".
- * Type Checker: Properly detect different return types when overriding an external interface function with a public contract function.
  * Optimizer: Fix nondeterminism bug related to the boost version and constants representation. The bug only resulted in less optimal but still correct code because the generated routine is always verified to be correct.
+ * Type Checker: Properly detect different return types when overriding an external interface function with a public contract function.
+ * Type Checker: Disallow struct return types for getters of public state variables unless the new ABI encoder is active.
 
 Build System:
  * Emscripten: Upgrade to Emscripten SDK 1.37.21 and boost 1.67.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index c41c372b..e1a89dd4 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -21,17 +21,24 @@
  */
 
 #include <libsolidity/analysis/TypeChecker.h>
-#include <memory>
-#include <boost/algorithm/cxx11/all_of.hpp>
-#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/join.hpp>
-#include <boost/range/adaptor/reversed.hpp>
 #include <libsolidity/ast/AST.h>
+
 #include <libyul/AsmAnalysis.h>
 #include <libyul/AsmAnalysisInfo.h>
 #include <libyul/AsmData.h>
+
 #include <liblangutil/ErrorReporter.h>
+
 #include <libdevcore/Algorithms.h>
+#include <libdevcore/StringUtils.h>
+
+#include <boost/algorithm/cxx11/all_of.hpp>
+#include <boost/algorithm/string/predicate.hpp>
+#include <boost/algorithm/string/join.hpp>
+#include <boost/range/adaptor/reversed.hpp>
+
+#include <memory>
+#include <vector>
 
 using namespace std;
 using namespace dev;
@@ -809,11 +816,25 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 			if (!varType->canLiveOutsideStorage())
 				m_errorReporter.typeError(_variable.location(), "Type " + varType->toString() + " is only valid in storage.");
 	}
-	else if (
-		_variable.visibility() >= VariableDeclaration::Visibility::Public &&
-		!FunctionType(_variable).interfaceFunctionType()
-	)
-		m_errorReporter.typeError(_variable.location(), "Internal or recursive type is not allowed for public state variables.");
+	else if (_variable.visibility() >= VariableDeclaration::Visibility::Public)
+	{
+		FunctionType getter(_variable);
+		if (!_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2))
+		{
+			vector<string> unsupportedTypes;
+			for (auto const& param: getter.parameterTypes() + getter.returnParameterTypes())
+				if (!typeSupportedByOldABIEncoder(*param))
+					unsupportedTypes.emplace_back(param->toString());
+			if (!unsupportedTypes.empty())
+				m_errorReporter.typeError(_variable.location(),
+					"The following types are only supported for getters in the new experimental ABI encoder: " +
+					joinHumanReadable(unsupportedTypes) +
+					". Either remove \"public\" or use \"pragma experimental ABIEncoderV2;\" to enable the feature."
+				);
+		}
+		if (!getter.interfaceFunctionType())
+			m_errorReporter.typeError(_variable.location(), "Internal or recursive type is not allowed for public state variables.");
+	}
 
 	switch (varType->category())
 	{
diff --git a/test/libsolidity/syntaxTests/getter/complex_struct.sol b/test/libsolidity/syntaxTests/getter/complex_struct.sol
new file mode 100644
index 00000000..3fa8111c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/getter/complex_struct.sol
@@ -0,0 +1,7 @@
+contract C {
+    struct Y {
+        uint a;
+        uint b;
+    }
+    mapping(uint256 => Y) public m;
+}
diff --git a/test/libsolidity/syntaxTests/getter/nested_structs.sol b/test/libsolidity/syntaxTests/getter/nested_structs.sol
new file mode 100644
index 00000000..1068f287
--- /dev/null
+++ b/test/libsolidity/syntaxTests/getter/nested_structs.sol
@@ -0,0 +1,11 @@
+contract C {
+    struct Y {
+        uint b;
+    }
+    struct X {
+        Y a;
+    }
+    mapping(uint256 => X) public m;
+}
+// ----
+// TypeError: (88-118): The following types are only supported for getters in the new experimental ABI encoder: struct C.Y memory. Either remove "public" or use "pragma experimental ABIEncoderV2;" to enable the feature.
diff --git a/test/libsolidity/syntaxTests/getter/recursive_struct.sol b/test/libsolidity/syntaxTests/getter/recursive_struct.sol
new file mode 100644
index 00000000..d81cac60
--- /dev/null
+++ b/test/libsolidity/syntaxTests/getter/recursive_struct.sol
@@ -0,0 +1,8 @@
+contract C {
+    struct Y {
+        Y[] x;
+    }
+    mapping(uint256 => Y) public m;
+}
+// ----
+// TypeError: (53-83): Internal or recursive type is not allowed for public state variables.
diff --git a/test/libsolidity/syntaxTests/getter/simple_struct.sol b/test/libsolidity/syntaxTests/getter/simple_struct.sol
new file mode 100644
index 00000000..c7a23ae9
--- /dev/null
+++ b/test/libsolidity/syntaxTests/getter/simple_struct.sol
@@ -0,0 +1,6 @@
+contract C {
+    struct Y {
+        uint b;
+    }
+    mapping(uint256 => Y) public m;
+}