Parser - Some clean-up in previously added code

opparser.cc

@@ -169,6 +169,9 @@ struct T_ParserImpl_
 
 	void main( T_SRDList const& list ) noexcept;
 
+	// ---------------------------------------------------------------------
+
+    private:
 	bool checkRequiredBlocks( T_SRDList const& list ) noexcept;
 	bool checkCalls( ) noexcept;
 	bool checkInstructionRestrictions( ) noexcept;
@@ -177,6 +180,9 @@ struct T_ParserImpl_
 	bool checkArgumentTypes( ) noexcept;
 	bool checkIdentifierExpressions( ) noexcept;
 
+	E_DataType getTypeOf( T_String const& name ,
+			A_FuncNode const* context ) const noexcept;
+
 	// ---------------------------------------------------------------------
 
 	bool parseTopLevel( T_SRDList const& list ) noexcept;
@@ -386,60 +392,68 @@ bool T_ParserImpl_::checkInstructionRestrictions( ) noexcept
 			return true;
 		} );
 	for ( auto i = 0u ; i < output->root.nFunctions( ) ; i ++ ) {
-		visitor.visit( output->root.function( i ) ,
+		visitor.visit( output->root.function( i ) , [&]( A_Node& node , bool exit ) {
-			[&]( A_Node& node , bool exit ) {
+			if ( exit ) {
-				if ( exit ) {
+				return false;
-					return false;
+			}
-				}
+			auto const* instr( dynamic_cast< A_InstructionNode const* >( &node ) );
-				auto const* instr( dynamic_cast< A_InstructionNode const* >( &node ) );
+			if ( instr && ( instr->restriction( ) & callInfo[ i ] ) ) {
-				if ( instr && ( instr->restriction( ) & callInfo[ i ] ) ) {
+				T_StringBuilder sb;
-					T_StringBuilder sb;
+				sb << "instruction not allowed in "
-					sb << "instruction not allowed in "
+					<< ( ( instr->restriction( ) & E_InstrRestriction::INIT )
-						<< ( ( instr->restriction( ) & E_InstrRestriction::INIT )
+						? "initialisation" : "frame function" );
-							? "initialisation" : "frame function" );
+				errors.addNew( std::move( sb ) , instr->location( ) );
-					errors.addNew( std::move( sb ) , instr->location( ) );
+			}
-				}
+			return true;
-				return true;
+		} );
-			} );
 	}
 	return errors.empty( );
 }
 
+/* Find local variable declarations, add them to the function to owns them,
+ * add errors if duplicates or variables with names that match function
+ * arguments are found.
+ */
 bool T_ParserImpl_::checkLocalVariables( ) noexcept
 {
 	uint32_t cfi;
 	T_StringBuilder esb;
 	for ( cfi = 0 ; cfi < output->root.nFunctions( ) ; cfi ++ ) {
 		auto& function( output->root.function( cfi ) );
-		visitor.visit( function ,
+		visitor.visit( function , [&]( A_Node& n , const bool exit ) -> bool {
-			[&]( A_Node& n , const bool exit ) -> bool {
+			if ( exit || n.type( ) != A_Node::OP_LOCALS ) {
-				if ( exit || n.type( ) != A_Node::OP_LOCALS ) {
+				return true;
-					return true;
+			}
+
+			auto& locals( dynamic_cast< T_LocalsInstrNode& >( n ) );
+			for ( auto i = 0u ; i < locals.variables( ) ; i ++ ) {
+				auto prev{ function.addLocalVariable(
+						locals.varName( i ) ,
+						locals.varLocation( i ) ) };
+				if ( !prev ) {
+					continue;
 				}
 
-				auto& locals( dynamic_cast< T_LocalsInstrNode& >( n ) );
+				esb << "duplicate local '" << locals.varName( i )
-				for ( auto i = 0u ; i < locals.variables( ) ; i ++ ) {
+					<< "'; previous declaration at "
-					auto prev{ function.addLocalVariable(
+					<< *prev;
-							locals.varName( i ) ,
+				errors.addNew( std::move( esb ) , locals.varLocation( i ) );
-							locals.varLocation( i ) ) };
+			}
-					if ( !prev ) {
-						continue;
-					}
 
-					esb << "duplicate local '" << locals.varName( i )
+			return false;
-						<< "'; previous declaration at "
+		} );
-						<< *prev;
-					errors.addNew( std::move( esb ) , locals.varLocation( i ) );
-				}
-
-				return false;
-			} );
 	}
 	return errors.empty( );
 }
 
+/* Find and determine the type of all global declarations. This includes
+ * variables and resources. In addition, make sure no resources are
+ * declared as local and that all declarations for a global name have
+ * the same type.
+ */
 bool T_ParserImpl_::collectGlobalTypes( ) noexcept
 {
+	// Temporary table for type / first declaration location
 	struct T_Decl_ {
 		E_DataType type;
 		T_SRDLocation location;
@@ -467,13 +481,12 @@ bool T_ParserImpl_::collectGlobalTypes( ) noexcept
 				return false;
 			}
 
+			// If the node defines or sets something, get its identifier,
+			// location and type.
 			E_DataType dt{ E_DataType::UNKNOWN };
 			T_String id;
 			T_SRDLocation location;
 			if ( node.type( ) == A_Node::OP_SET ) {
-				// When we find a set instruction, we need to check whether
-				// it is affecting a local variable. If it is we'll just skip
-				// it.
 				id = dynamic_cast< T_SetInstrNode& >( node ).id( );
 				dt = E_DataType::VARIABLE;
 				location = dynamic_cast< T_SetInstrNode& >( node ).idLocation( );
@@ -494,6 +507,9 @@ bool T_ParserImpl_::collectGlobalTypes( ) noexcept
 			esb.clear( );
 #endif
 
+			// When we find a set instruction, we need to check whether
+			// it is affecting a local variable. If it is we'll just skip
+			// it.
 			if ( function.hasLocal( id ) ) {
 				if ( function.isArgument( id ) ) {
 					errors.addNew( "trying to override argument",
@@ -505,15 +521,17 @@ bool T_ParserImpl_::collectGlobalTypes( ) noexcept
 				return false;
 			}
 
+			// Add new entries
 			T_Decl_ const* const existing( type.get( id ) );
 			if ( !existing ) {
 				type.add( id , T_Decl_{ dt , location } );
 				return false;
 			}
+
+			// Make sure it matches previous declarations
 			if ( existing->type == dt ) {
 				return false;
 			}
-
 			if ( existing->type == E_DataType::BUILTIN ) {
 				esb << "trying to redefine built-in variable " << id;
 			} else {
@@ -528,6 +546,7 @@ bool T_ParserImpl_::collectGlobalTypes( ) noexcept
 		} );
 	}
 
+	// Copy type table to the output
 	for ( auto const& k : type.keys( ) ) {
 		output->types.add( k , type.get( k )->type );
 	}
@@ -613,64 +632,45 @@ bool T_ParserImpl_::checkArgumentTypes( ) noexcept
 				assert( call->arguments( ) == calledFn.arguments( ) );
 				if ( argsResolved[ calledIdx ] ) {
 					TRACE( "    argument types already resolved, checking" );
-					for ( auto a = 0u ; a < call->arguments( ) ; a ++ ) {
-						auto& arg( call->argument( a ) );
-						E_DataType ndt{ E_DataType::UNKNOWN };
-						if ( arg.type( ) == A_Node::EXPR_ID ) {
-							T_String const& id( dynamic_cast< T_IdentifierExprNode& >( arg ).id( ) );
-							if ( f.hasLocal( id ) ) {
-								ndt = f.getLocalType( id );
-							} else {
-								auto const* const ptr( output->types.get( id ) );
-								if ( ptr ) {
-									ndt = *ptr;
-								} else {
-									errors.addNew( "unknown identifier" ,
-											arg.location( ) );
-								}
-							}
-						} else {
-							ndt = E_DataType::VARIABLE;
-						}
-						TRACE( "     [" << a << "] " << ndt );
-						if ( ndt != E_DataType::UNKNOWN && calledFn.getLocalType( a ) != ndt ) {
-							esb << "argument " << ( a + 1 ) << " of function '"
-								<< called << "' should be a "
-								<< calledFn.getLocalType( a )
-								<< " but a " << ndt << " is being passed";
-							errors.addNew( std::move( esb ) , arg.location( ) );
-						}
-					}
-					changed = true;
 				} else {
 					TRACE( "    resolving arguments" );
-					bool ok = true;
-					for ( auto a = 0u ; a < call->arguments( ) ; a ++ ) {
-						auto& arg( call->argument( a ) );
-						E_DataType ndt{ E_DataType::UNKNOWN };
-						if ( arg.type( ) == A_Node::EXPR_ID ) {
-							T_String const& id( dynamic_cast< T_IdentifierExprNode& >( arg ).id( ) );
-							if ( f.hasLocal( id ) ) {
-								ndt = f.getLocalType( id );
-							} else {
-								auto const* const ptr( output->types.get( id ) );
-								if ( ptr ) {
-									ndt = *ptr;
-								} else {
-									errors.addNew( "unknown identifier" ,
-											arg.location( ) );
-									ok = false;
-								}
-							}
-						} else {
-							ndt = E_DataType::VARIABLE;
-						}
-						TRACE( "     [" << a << "] " << ndt );
-						calledFn.setLocalType( a , ndt );
-					}
-					argsResolved[ calledIdx ] = ok;
-					changed = changed || ok;
 				}
+
+				bool ok = true;
+				for ( auto a = 0u ; a < call->arguments( ) ; a ++ ) {
+					auto& arg( call->argument( a ) );
+					E_DataType ndt{ E_DataType::UNKNOWN };
+					if ( arg.type( ) == A_Node::EXPR_ID ) {
+						auto const& idn( dynamic_cast< T_IdentifierExprNode& >( arg ) );
+						T_String const& id( idn.id( ) );
+						ndt = getTypeOf( id , &f );
+						if ( ndt == E_DataType::UNKNOWN ) {
+							errors.addNew( "unknown identifier" ,
+									arg.location( ) );
+						}
+					} else {
+						ndt = E_DataType::VARIABLE;
+					}
+					TRACE( "     [" << a << "] " << ndt );
+
+					// Arguments not resolved -> try setting the argument's type
+					if ( !argsResolved[ calledIdx ] ) {
+						ok = ( ndt != E_DataType::UNKNOWN );
+						calledFn.setLocalType( a , ndt );
+						continue;
+					}
+
+					// Arguments resolved -> error if types don't match
+					if ( ndt != E_DataType::UNKNOWN && calledFn.getLocalType( a ) != ndt ) {
+						esb << "argument " << ( a + 1 ) << " of function '"
+							<< called << "' should be a "
+							<< calledFn.getLocalType( a )
+							<< " but a " << ndt << " is being passed";
+						errors.addNew( std::move( esb ) , arg.location( ) );
+					}
+				}
+				argsResolved[ calledIdx ] = ok;
+				changed = changed || ok;
 				callInstuctions.get( i , c ) = nullptr;
 			}
 		}
@@ -752,6 +752,24 @@ bool T_ParserImpl_::checkIdentifierExpressions( ) noexcept
 
 /*------------------------------------------------------------------------------*/
 
+E_DataType T_ParserImpl_::getTypeOf(
+		T_String const& name ,
+		A_FuncNode const* context ) const noexcept
+{
+	if ( context && context->hasLocal( name ) ) {
+		return context->getLocalType( name );
+	} else {
+		auto const* const ptr( output->types.get( name ) );
+		if ( ptr ) {
+			return *ptr;
+		} else {
+			return E_DataType::UNKNOWN;
+		}
+	}
+}
+
+/*------------------------------------------------------------------------------*/
+
 bool T_ParserImpl_::parseTopLevel(
 		T_SRDList const& input ) noexcept
 {