Various fixes pertaining to State Variable accessors

libsolidity/AST.cpp

@@ -155,7 +155,7 @@ vector<tuple<FixedHash<4>, std::shared_ptr<FunctionType const>, Declaration cons
 				}
 
 			for (ASTPointer<VariableDeclaration> const& v: contract->getStateVariables())
-				if (v->isPublic())
+				if (v->isPublic() && functionsSeen.count(v->getName()) == 0)
 				{
 					FunctionType ftype(*v);
 					functionsSeen.insert(v->getName());
@@ -519,12 +519,12 @@ void Literal::checkTypeRequirements()
 		BOOST_THROW_EXCEPTION(createTypeError("Invalid literal value."));
 }
 
-std::string ParamDescription::getName() const
+std::string const& ParamDescription::getName() const
 {
 	return m_description.first;
 }
 
-std::string ParamDescription::getType() const
+std::string const& ParamDescription::getType() const
 {
 	return m_description.second;
 }

libsolidity/AST.h

@@ -158,16 +158,16 @@ private:
 
 
 /**
-* Generic Parameter description used by @see FunctionDescription to return
-* a descripton of its parameters.
-*/
+ * Generic Parameter description used by @see FunctionDescription to return
+ * a descripton of its parameters.
+ */
 struct ParamDescription
 {
 	ParamDescription(std::string const& _name, std::string const& _type):
 		m_description(_name, _type){}
 
-	std::string getName() const;
-	std::string getType() const;
+	std::string const& getName() const;
+	std::string const& getType() const;
 
 	std::pair<std::string, std::string> m_description;
 };
@@ -456,6 +456,7 @@ private:
 	ASTPointer<TypeName> m_typeName;    ///< can be empty ("var")
 	bool m_isPublic;                    ///< Whether there is an accessor for it or not
 	bool m_isStateVariable;             ///< Whether or not this is a contract state variable
+
 	std::shared_ptr<Type const> m_type; ///< derived type, initially empty
 };
 

libsolidity/Compiler.cpp

@@ -295,15 +295,15 @@ void Compiler::registerStateVariables(ContractDefinition const& _contract)
 void Compiler::generateAccessorCode(VariableDeclaration const& _varDecl)
 {
 	m_context.startNewFunction();
-	m_returnTag = m_context.newTag();
 	m_breakTags.clear();
 	m_continueTags.clear();
 
 	m_context << m_context.getFunctionEntryLabel(_varDecl);
-	ExpressionCompiler::appendStateVariableAccessor(m_context, &_varDecl);
+	ExpressionCompiler::appendStateVariableAccessor(m_context, _varDecl);
 
-	uint64_t foo = uint64_t(_varDecl.getType()->getStorageSize());
-	m_context << eth::dupInstruction(foo + 1);
+	unsigned sizeOnStack = _varDecl.getType()->getSizeOnStack();
+	solAssert(sizeOnStack <= 15, "Illegal variable stack size detected");
+	m_context << eth::dupInstruction(sizeOnStack + 1);
 	m_context << eth::Instruction::JUMP;
 }
 

libsolidity/CompilerContext.h

@@ -117,10 +117,7 @@ private:
 	std::map<Declaration const*, u256> m_stateVariables;
 	/// Offsets of local variables on the stack (relative to stack base).
 	std::map<Declaration const*, unsigned> m_localVariables;
-	/// Sum of stack sizes of local variables
-	unsigned m_localVariablesSize;
 	/// Labels pointing to the entry points of functions.
-
 	std::map<Declaration const*, eth::AssemblyItem> m_functionEntryLabels;
 	/// Labels pointing to the entry points of function overrides.
 	std::map<std::string, eth::AssemblyItem> m_virtualFunctionEntryLabels;

libsolidity/ExpressionCompiler.cpp

@@ -48,7 +48,7 @@ void ExpressionCompiler::appendTypeConversion(CompilerContext& _context, Type co
 	compiler.appendTypeConversion(_typeOnStack, _targetType, _cleanupNeeded);
 }
 
-void ExpressionCompiler::appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize)
+void ExpressionCompiler::appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const& _varDecl, bool _optimize)
 {
 	ExpressionCompiler compiler(_context, _optimize);
 	compiler.appendStateVariableAccessor(_varDecl);
@@ -795,10 +795,11 @@ unsigned ExpressionCompiler::appendArgumentCopyToMemory(TypePointers const& _typ
 	return length;
 }
 
-void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const* _varDecl)
+void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& _varDecl)
 {
-	m_currentLValue.fromStateVariable(*_varDecl, _varDecl->getType());
-	m_currentLValue.retrieveValueFromStorage(_varDecl->getType(), true);
+	m_currentLValue.fromStateVariable(_varDecl, _varDecl.getType());
+	solAssert(m_currentLValue.isInStorage(), "");
+	m_currentLValue.retrieveValueFromStorage(_varDecl.getType(), true);
 }
 
 ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType,
@@ -843,7 +844,7 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo
 	}
 }
 
-void ExpressionCompiler::LValue::retrieveValueFromStorage(std::shared_ptr<Type const> const& _type, bool _remove) const
+void ExpressionCompiler::LValue::retrieveValueFromStorage(TypePointer const& _type, bool _remove) const
 {
 	if (!_type->isValueType())
 		return; // no distinction between value and reference for non-value types
@@ -968,7 +969,7 @@ void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression co
 	}
 }
 
-void ExpressionCompiler::LValue::fromStateVariable(Declaration const& _varDecl, std::shared_ptr<Type const> const& _type)
+void ExpressionCompiler::LValue::fromStateVariable(Declaration const& _varDecl, TypePointer const& _type)
 {
 	m_type = STORAGE;
 	solAssert(_type->getStorageSize() <= numeric_limits<unsigned>::max(), "The storage size of " + _type->toString() + " should fit in an unsigned");

libsolidity/ExpressionCompiler.h

@@ -54,7 +54,7 @@ public:
 	static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack,
 									 Type const& _targetType, bool _cleanupNeeded = false);
 	/// Appends code for a State Variable accessor function
-	static void appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize = false);
+	static void appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const& _varDecl, bool _optimize = false);
 
 private:
 	explicit ExpressionCompiler(CompilerContext& _compilerContext, bool _optimize = false):
@@ -98,7 +98,7 @@ private:
 										unsigned _memoryOffset = 0);
 
 	/// Appends code for a State Variable accessor function
-	void appendStateVariableAccessor(VariableDeclaration const* _varDecl);
+	void appendStateVariableAccessor(VariableDeclaration const& _varDecl);
 
 	/**
 	 * Helper class to store and retrieve lvalues to and from various locations.
@@ -117,7 +117,7 @@ private:
 		/// @a _expression is the current expression
 		void fromIdentifier(Identifier const& _identifier, Declaration const& _declaration);
 		/// Convenience function to set type for a state variable and retrieve the reference
-		void fromStateVariable(Declaration const& _varDecl, std::shared_ptr<Type const> const& _type);
+		void fromStateVariable(Declaration const& _varDecl, TypePointer const& _type);
 		void reset() { m_type = NONE; m_baseStackOffset = 0; m_size = 0; }
 
 		bool isValid() const { return m_type != NONE; }
@@ -133,7 +133,7 @@ private:
 		/// @a _expression is the current expression, used for error reporting.
 		void retrieveValue(Expression const& _expression, bool _remove = false) const;
 		/// Convenience function to retrieve Value from Storage. Specific version of @ref retrieveValue
-		void retrieveValueFromStorage(std::shared_ptr<Type const> const& _type, bool _remove = false) const;
+		void retrieveValueFromStorage(TypePointer const& _type, bool _remove = false) const;
 		/// Stores a value (from the stack directly beneath the reference, which is assumed to
 		/// be on the top of the stack, if any) in the lvalue and removes the reference.
 		/// Also removes the stored value from the stack if @a _move is

test/SolidityNameAndTypeResolution.cpp

@@ -76,7 +76,7 @@ ASTPointer<SourceUnit> parseTextAndResolveNamesWithChecks(std::string const& _so
 	}
 	catch(boost::exception const& _e)
 	{
-		auto msg = std::string("Parsing text and resolving nanes failed with: \n") + boost::diagnostic_information(_e);
+		auto msg = std::string("Parsing text and resolving names failed with: \n") + boost::diagnostic_information(_e);
 		BOOST_FAIL(msg);
 	}
 	return sourceUnit;
@@ -642,7 +642,7 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors)
 	ASTPointer<SourceUnit> source;
 	ContractDefinition const* contract;
 	BOOST_CHECK_NO_THROW(source = parseTextAndResolveNamesWithChecks(text));
-	BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr);
+	BOOST_REQUIRE((contract = retrieveContract(source, 0)) != nullptr);
 	FunctionDescription function = retrieveFunctionBySignature(contract, "foo()");
 	BOOST_CHECK_MESSAGE(function.getDeclaration() != nullptr, "Could not find the accessor function");
 	auto returnParams = function.getReturnParameters();