Parser - Improved type checking re: locals

opast.cc

@@ -323,7 +323,9 @@ T_Optional< T_SRDLocation > A_FuncNode::addLocalVariable(
 	if ( pnp ) {
 		return pnp->location;
 	}
-	locals_.add( T_Local_{ name , location , false } );
+	T_Local_ lv{ name , location , false };
+	lv.type = E_DataType::VARIABLE;
+	locals_.add( std::move( lv ) );
 	return {};
 }
 

opast.hh

@@ -239,9 +239,20 @@ class A_FuncNode : public A_Node
 	T_InstrListNode const& instructions( ) const noexcept
 		{ return instructions_; }
 
+	// ---------------------------------------------------------------------
+
 	T_Optional< T_SRDLocation > addLocalVariable(
 			T_String const& name ,
 			T_SRDLocation const& location ) noexcept;
+
+	bool hasLocal( T_String const& id ) const noexcept
+		{ return locals_.contains( id ); }
+
+	bool isArgument( T_String const& id ) const noexcept
+	{
+		auto const* const ptr( locals_.get( id ) );
+		return ptr && ptr->argument;
+	}
 };
 using P_InstrListNode = T_OwnPtr< T_InstrListNode >;
 

opparser.cc

@@ -286,13 +286,16 @@ void T_ParserImpl_::main(
 		&& checkRequiredBlocks( input )
 		&& checkCalls( )
 		&& checkInstructionRestrictions( )
-		&& collectGlobalTypes( )
 		&& checkLocalVariables( )
+		&& collectGlobalTypes( )
 		&& checkIdentifierExpressions( );
 }
 
 /*----------------------------------------------------------------------------*/
 
+/* Check that both the initialisation and frame rendering blocks have been
+ * defined.
+ */
 bool T_ParserImpl_::checkRequiredBlocks(
 		T_SRDList const& input ) noexcept
 {
@@ -311,6 +314,9 @@ bool T_ParserImpl_::checkRequiredBlocks(
 	return errors.empty( );
 }
 
+/* Gather function calls into a call graph and check that function calls
+ * provide the correct quantity of arguments.
+ */
 bool T_ParserImpl_::checkCalls( ) noexcept
 {
 	calls.clear( );
@@ -352,6 +358,10 @@ bool T_ParserImpl_::checkCalls( ) noexcept
 	return errors.empty( );
 }
 
+/* Go through the call graph, determine whether functions are called from the
+ * initialisation block, the frame rendering block, or both, and then enforce
+ * restrictions on instructions.
+ */
 bool T_ParserImpl_::checkInstructionRestrictions( ) noexcept
 {
 	callInfo.clear( );
@@ -393,79 +403,6 @@ bool T_ParserImpl_::checkInstructionRestrictions( ) noexcept
 	return errors.empty( );
 }
 
-bool T_ParserImpl_::collectGlobalTypes( ) noexcept
-{
-	struct T_Decl_ {
-		E_DataType type;
-		T_SRDLocation location;
-
-		T_Decl_( const E_DataType dt ,
-				T_SRDLocation const& loc ) noexcept
-			: type( dt ) , location( loc )
-		{ }
-
-		T_Decl_( const E_DataType dt ) noexcept
-			: type( dt ) , location{}
-		{ }
-	};
-	T_KeyValueTable< T_String , T_Decl_ > type;
-	type.add( T_String::Pooled( "time" ) , T_Decl_{ E_DataType::BUILTIN } );
-	type.add( T_String::Pooled( "width" ) , T_Decl_{ E_DataType::BUILTIN } );
-	type.add( T_String::Pooled( "height" ) , T_Decl_{ E_DataType::BUILTIN } );
-
-	visitor.visit( output->root , [&]( A_Node& node , bool exit ) -> bool {
-		if ( exit ) {
-			return false;
-		}
-
-		E_DataType dt{ E_DataType::UNKNOWN };
-		T_String id;
-		T_SRDLocation location;
-		if ( node.type( ) == A_Node::OP_SET ) {
-			dt = E_DataType::VARIABLE;
-			id = dynamic_cast< T_SetInstrNode& >( node ).id( );
-			location = dynamic_cast< T_SetInstrNode& >( node ).idLocation( );
-		} else {
-			auto* np{ dynamic_cast< A_ResourceDefInstrNode* >( &node ) };
-			if ( !np ) {
-				return !dynamic_cast< A_ExpressionNode* >( &node );
-			}
-			dt = np->dataType( );
-			id = np->id( );
-			location = np->idLocation( );
-		}
-		assert( dt != E_DataType::UNKNOWN );
-
-		T_Decl_ const* const existing( type.get( id ) );
-		if ( !existing ) {
-			type.add( id , T_Decl_{ dt , location } );
-			return false;
-		}
-		if ( existing->type == dt ) {
-			return false;
-		}
-
-		T_StringBuilder sb;
-		if ( existing->type == E_DataType::BUILTIN ) {
-			sb << "trying to redefine built-in variable " << id;
-		} else {
-			sb << "'" << id << "' redeclared as " << dt
-				<< "; previous declaration as "
-				<< existing->type << " at "
-				<< existing->location;
-		}
-		errors.addNew( std::move( sb ) , location );
-
-		return false;
-	} );
-
-	for ( auto const& k : type.keys( ) ) {
-		output->types.add( k , type.get( k )->type );
-	}
-
-	return errors.empty( );
-}
-
 bool T_ParserImpl_::checkLocalVariables( ) noexcept
 {
 	uint32_t cfi;
@@ -499,6 +436,97 @@ bool T_ParserImpl_::checkLocalVariables( ) noexcept
 	return errors.empty( );
 }
 
+bool T_ParserImpl_::collectGlobalTypes( ) noexcept
+{
+	struct T_Decl_ {
+		E_DataType type;
+		T_SRDLocation location;
+
+		T_Decl_( const E_DataType dt ,
+				T_SRDLocation const& loc ) noexcept
+			: type( dt ) , location( loc )
+		{ }
+
+		T_Decl_( const E_DataType dt ) noexcept
+			: type( dt ) , location{}
+		{ }
+	};
+	T_KeyValueTable< T_String , T_Decl_ > type;
+	type.add( T_String::Pooled( "time" ) , T_Decl_{ E_DataType::BUILTIN } );
+	type.add( T_String::Pooled( "width" ) , T_Decl_{ E_DataType::BUILTIN } );
+	type.add( T_String::Pooled( "height" ) , T_Decl_{ E_DataType::BUILTIN } );
+
+	uint32_t cfi;
+	T_StringBuilder esb;
+	for ( cfi = 0 ; cfi < output->root.nFunctions( ) ; cfi ++ ) {
+		auto& function( output->root.function( cfi ) );
+		visitor.visit( function , [&]( A_Node& node , bool exit ) -> bool {
+			if ( exit ) {
+				return false;
+			}
+
+			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( );
+			} else {
+				auto* np{ dynamic_cast< A_ResourceDefInstrNode* >( &node ) };
+				if ( !np ) {
+					return !dynamic_cast< A_ExpressionNode* >( &node );
+				}
+				dt = np->dataType( );
+				id = np->id( );
+				location = np->idLocation( );
+			}
+			assert( dt != E_DataType::UNKNOWN );
+
+			if ( function.hasLocal( id ) ) {
+				if ( function.isArgument( id ) ) {
+					errors.addNew( "trying to override argument",
+							node.location( ) );
+				} else if ( dt != E_DataType::VARIABLE ) {
+					esb << "cannot define a local " << dt;
+					errors.addNew( std::move( esb ) , node.location( ) );
+				}
+				return false;
+			}
+
+			T_Decl_ const* const existing( type.get( id ) );
+			if ( !existing ) {
+				type.add( id , T_Decl_{ dt , location } );
+				return false;
+			}
+			if ( existing->type == dt ) {
+				return false;
+			}
+
+			if ( existing->type == E_DataType::BUILTIN ) {
+				esb << "trying to redefine built-in variable " << id;
+			} else {
+				esb << "'" << id << "' redeclared as " << dt
+					<< "; previous declaration as "
+					<< existing->type << " at "
+					<< existing->location;
+			}
+			errors.addNew( std::move( esb ) , location );
+
+			return false;
+		} );
+	}
+
+	for ( auto const& k : type.keys( ) ) {
+		output->types.add( k , type.get( k )->type );
+	}
+
+	return errors.empty( );
+}
+
 bool T_ParserImpl_::checkIdentifierExpressions( ) noexcept
 {
 	uint32_t cfi;