Addressing issues with Enums in Solidity

6 files changed, 41 insertions, 50 deletions

diff --git a/AST.cpp b/AST.cpp
index 9d7d2169..5aa67218 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -209,6 +209,7 @@ vector<pair<FixedHash<4>, FunctionTypePointer>> const& ContractDefinition::getIn
 TypePointer EnumValue::getType(ContractDefinition const*) const
 {
 	EnumDefinition const* parentDef = dynamic_cast<EnumDefinition const*>(getScope());
+	solAssert(parentDef, "Enclosing Scope of EnumValue was not set");
 	return make_shared<EnumType>(*parentDef);
 }
 
@@ -271,7 +272,7 @@ void EnumDefinition::checkValidityOfMembers() const
 	sort(begin(members), end(members), compareDecls);
 	for (size_t i = 0; i < members.size() - 1; ++i)
 		if (members[i]->getName() == members[i + 1]->getName())
-			BOOST_THROW_EXCEPTION(createTypeError("Duplicate member detected in Enum"));
+			BOOST_THROW_EXCEPTION(members[i]->createTypeError("Duplicate member detected in Enum"));
 }
 
 TypePointer EnumDefinition::getType(ContractDefinition const*) const
diff --git a/AST.h b/AST.h
index 3485631b..25c18f4f 100644
--- a/AST.h
+++ b/AST.h
@@ -166,21 +166,6 @@ private:
 };
 
 /**
- * Declaration of an Enum Value
- */
-class EnumValue: public Declaration
-{
-  public:
-	EnumValue(Location const& _location,
-			  ASTPointer<ASTString> const& _name):
-		Declaration(_location, _name) {}
-
-	virtual void accept(ASTVisitor& _visitor) override;
-	virtual void accept(ASTConstVisitor& _visitor) const override;
-	TypePointer getType(ContractDefinition const* = nullptr) const;
-};
-
-/**
  * Abstract class that is added to each AST node that can store local variables.
  */
 class VariableScope
@@ -360,6 +345,21 @@ private:
 };
 
 /**
+ * Declaration of an Enum Value
+ */
+class EnumValue: public Declaration
+{
+  public:
+	EnumValue(Location const& _location,
+			  ASTPointer<ASTString> const& _name):
+		Declaration(_location, _name) {}
+
+	virtual void accept(ASTVisitor& _visitor) override;
+	virtual void accept(ASTConstVisitor& _visitor) const override;
+	TypePointer getType(ContractDefinition const* = nullptr) const;
+};
+
+/**
  * Parameter list, used as function parameter list and return list.
  * None of the parameters is allowed to contain mappings (not even recursively
  * inside structs).
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 4e72aca5..00c2dafd 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -501,26 +501,26 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		TypeType const& type = dynamic_cast<TypeType const&>(*_memberAccess.getExpression().getType());
 		ContractType const* contractType;
 		EnumType const* enumType;
-		if (type.getMembers().getMemberType(member))
+		if (!type.getMembers().getMemberType(member))
+			BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Invalid member access to " + type.toString()));			
+
+		if ((contractType = dynamic_cast<ContractType const*>(type.getActualType().get())))
 		{
-			if ((contractType = dynamic_cast<ContractType const*>(type.getActualType().get())))
-			{
-				ContractDefinition const& contract = contractType->getContractDefinition();
-				for (ASTPointer<FunctionDefinition> const& function: contract.getDefinedFunctions())
-					if (function->getName() == member)
-					{
-						m_context << m_context.getFunctionEntryLabel(*function).pushTag();
-						return;
-					}
-			}
-			else if ((enumType = dynamic_cast<EnumType const*>(type.getActualType().get())))
-			{
-				EnumDefinition const &enumDef = enumType->getEnumDefinition();
-				m_context << enumDef.getMemberValue(_memberAccess.getMemberName());
-				return;
-			}
+			ContractDefinition const& contract = contractType->getContractDefinition();
+			for (ASTPointer<FunctionDefinition> const& function: contract.getDefinedFunctions())
+				if (function->getName() == member)
+				{
+					m_context << m_context.getFunctionEntryLabel(*function).pushTag();
+					return;
+				}
 		}
-		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Invalid member access to " + type.toString()));
+		else if ((enumType = dynamic_cast<EnumType const*>(type.getActualType().get())))
+		{
+			EnumDefinition const &enumDef = enumType->getEnumDefinition();
+			m_context << enumDef.getMemberValue(_memberAccess.getMemberName());
+			return;
+		}
+
 	}
 	case Type::Category::ByteArray:
 	{
diff --git a/Parser.cpp b/Parser.cpp
index 28339a46..4277ef54 100644
--- a/Parser.cpp
+++ b/Parser.cpp
@@ -266,12 +266,11 @@ ASTPointer<StructDefinition> Parser::parseStructDefinition()
 	return nodeFactory.createNode<StructDefinition>(name, members);
 }
 
-ASTPointer<EnumValue> Parser::parseEnumDeclaration()
+ASTPointer<EnumValue> Parser::parseEnumValue()
 {
 	ASTNodeFactory nodeFactory(*this);
-	ASTPointer<ASTString> name = expectIdentifierToken();
 	nodeFactory.markEndPosition();
-	return nodeFactory.createNode<EnumValue>(name);
+	return nodeFactory.createNode<EnumValue>(expectIdentifierToken());
 }
 
 ASTPointer<EnumDefinition> Parser::parseEnumDefinition()
@@ -284,13 +283,12 @@ ASTPointer<EnumDefinition> Parser::parseEnumDefinition()
 
 	while (m_scanner->getCurrentToken() != Token::RBrace)
 	{
-		members.push_back(parseEnumDeclaration());
+		members.push_back(parseEnumValue());
 		if (m_scanner->getCurrentToken() == Token::RBrace)
 			break;
 		expectToken(Token::Comma);
-		if (m_scanner->getCurrentToken() != Token::Identifier) {
+		if (m_scanner->getCurrentToken() != Token::Identifier)
 			BOOST_THROW_EXCEPTION(createParserError("Expected Identifier after ','"));
-		}
 	}
 
 	nodeFactory.markEndPosition();
@@ -859,13 +857,6 @@ ASTPointer<ASTString> Parser::expectIdentifierToken()
 	return getLiteralAndAdvance();
 }
 
-ASTPointer<ASTString> Parser::peekIdentifierToken()
-{
-	if (m_scanner->getCurrentToken() != Token::Identifier)
-		return nullptr;
-	return getLiteralAndAdvance();
-}
-
 ASTPointer<ASTString> Parser::getLiteralAndAdvance()
 {
 	ASTPointer<ASTString> identifier = make_shared<ASTString>(m_scanner->getCurrentLiteral());
diff --git a/Parser.h b/Parser.h
index 172b45dd..1bb4ea97 100644
--- a/Parser.h
+++ b/Parser.h
@@ -62,7 +62,7 @@ private:
 	ASTPointer<FunctionDefinition> parseFunctionDefinition(ASTString const* _contractName);
 	ASTPointer<StructDefinition> parseStructDefinition();
 	ASTPointer<EnumDefinition> parseEnumDefinition();
-	ASTPointer<EnumValue> parseEnumDeclaration();
+	ASTPointer<EnumValue> parseEnumValue();
 	ASTPointer<VariableDeclaration> parseVariableDeclaration(VarDeclParserOptions const& _options = VarDeclParserOptions());
 	ASTPointer<ModifierDefinition> parseModifierDefinition();
 	ASTPointer<EventDefinition> parseEventDefinition();
@@ -98,7 +98,6 @@ private:
 	void expectToken(Token::Value _value);
 	Token::Value expectAssignmentOperator();
 	ASTPointer<ASTString> expectIdentifierToken();
-	ASTPointer<ASTString> peekIdentifierToken();
 	ASTPointer<ASTString> getLiteralAndAdvance();
 	///@}
 
diff --git a/Types.h b/Types.h
index 15bd86b1..d8b1b6b7 100644
--- a/Types.h
+++ b/Types.h
@@ -377,7 +377,7 @@ public:
 	explicit EnumType(EnumDefinition const& _enum): m_enum(_enum) {}
 	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual bool operator==(Type const& _other) const override;
-	virtual unsigned getSizeOnStack() const override { return 1; /*@todo*/ }
+	virtual unsigned getSizeOnStack() const override { return 1; }
 	virtual std::string toString() const override;
 	virtual bool isValueType() const override { return true; }