Merge pull request #5526 from ethereum/fixGetterNewCoder

Fix: Disallow structs in getters for old encoder.

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.

libsolidity/analysis/TypeChecker.cpp

@@ -21,17 +21,24 @@
  */
 
 #include <libsolidity/analysis/TypeChecker.h>
-#include <memory>
+#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 <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 <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())
 	{

test/libsolidity/syntaxTests/getter/complex_struct.sol

@@ -0,0 +1,7 @@
+contract C {
+    struct Y {
+        uint a;
+        uint b;
+    }
+    mapping(uint256 => Y) public m;
+}

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.

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.

test/libsolidity/syntaxTests/getter/simple_struct.sol

@@ -0,0 +1,6 @@
+contract C {
+    struct Y {
+        uint b;
+    }
+    mapping(uint256 => Y) public m;
+}