Addressing issues with Enums in Solidity

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

AST.h

@@ -165,21 +165,6 @@ private:
 	Declaration const* m_scope;
 };
 
-/**
- * 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.
  */
@@ -359,6 +344,21 @@ private:
 	std::vector<ASTPointer<EnumValue>> m_members;
 };
 
+/**
+ * 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

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())
-				ContractDefinition const& contract = contractType->getContractDefinition();
+				if (function->getName() == member)
-				for (ASTPointer<FunctionDefinition> const& function: contract.getDefinedFunctions())
+				{
-					if (function->getName() == member)
+					m_context << m_context.getFunctionEntryLabel(*function).pushTag();
-					{
+					return;
-						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;
-			}
 		}
-		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:
 	{

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());

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();
 	///@}
 

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; }