Evaluate macro parameters in outer scope

src/lineparser.cpp

@@ -227,30 +227,18 @@ void LineParser::Process( const string& line )
 					cout << "Macro " << macroName << ":" << endl;
 				}
 
-				HandleOpenBrace();
-
+				// Evaluate parameters at outer scope.
+				std::vector<Value> parameterValues;
+				std::vector<bool> parameterDefined;
+				parameterValues.resize( macro->GetNumberOfParameters() );
+				parameterDefined.resize( macro->GetNumberOfParameters() );
 				for ( int i = 0; i < macro->GetNumberOfParameters(); i++ )
 				{
-					ScopedSymbolName paramName = m_sourceCode->GetScopedSymbolName( macro->GetParameter( i ) );
-
 					try
 					{
-						if ( !SymbolTable::Instance().IsSymbolDefined( paramName ) )
-						{
-							Value value = EvaluateExpression();
-							SymbolTable::Instance().AddSymbol( paramName, value );
-						}
-						else if ( GlobalData::Instance().IsSecondPass() )
-						{
-							// We must remove the symbol before evaluating the expression,
-							// otherwise nested macros which share the same parameter name can
-							// evaluate the inner macro parameter using the old value of the inner
-							// macro parameter rather than the new value of the outer macro
-							// parameter. See local-forward-branch-5.6502 for an example.
-							SymbolTable::Instance().RemoveSymbol( paramName );
-							Value value = EvaluateExpression();
-							SymbolTable::Instance().AddSymbol( paramName, value );
-						}
+						Value value = EvaluateExpression();
+						parameterValues[i] = value;
+						parameterDefined[i] = true;
 					}
 					catch ( AsmException_SyntaxError_SymbolNotDefined& )
 					{
@@ -270,15 +258,39 @@ void LineParser::Process( const string& line )
 						m_column++;
 					}
 				}
-				
+
 				if ( AdvanceAndCheckEndOfStatement() )
 				{
 					throw AsmException_SyntaxError_InvalidCharacter( m_line, m_column );
 				}
 
+				// Enter the scope
+				HandleOpenBrace();
+
+				// Create symbols for the parameters
+				for ( int i = 0; i < macro->GetNumberOfParameters(); i++ )
+				{
+					// If the parameter is not defined then don't give it a default value.  We want the
+					// undefinedness to propagate to the macro so that the assembler uses PC as a default.
+					if ( parameterDefined[i] )
+					{
+						ScopedSymbolName paramName = m_sourceCode->GetScopedSymbolName( macro->GetParameter( i ) );
+						if ( !SymbolTable::Instance().IsSymbolDefined( paramName ) )
+						{
+							SymbolTable::Instance().AddSymbol( paramName, parameterValues[i] );
+						}
+						else
+						{
+							// The value may come from an outer scope on the first pass and the current scope on
+							// the second pass so it may need updating.
+							SymbolTable::Instance().ChangeSymbol( paramName, parameterValues[i] );
+						}
+					}
+				}
+
+				// Run the macro and tidy up
 				MacroInstance macroInstance( macro, m_sourceCode );
 				macroInstance.Process();
-
 				HandleCloseBrace();
 
 				if ( m_sourceCode->ShouldOutputAsm() )

src/symboltable.cpp

@@ -316,7 +316,7 @@ void SymbolTable::ChangeBuiltInSymbol( const std::string& symbolName, double val
 	@param		value			Its new value
 */
 /*************************************************************************************************/
-void SymbolTable::ChangeSymbol( const ScopedSymbolName& symbol, double value )
+void SymbolTable::ChangeSymbol( const ScopedSymbolName& symbol, Value value )
 {
 	assert( IsSymbolDefined( symbol ) );
 	m_map.find( symbol )->second.SetValue( value );

src/symboltable.h

@@ -46,7 +46,7 @@ class SymbolTable
 	bool AddCommandLineSymbol( const std::string& expr );
 	bool AddCommandLineStringSymbol( const std::string& expr );
 	void ChangeBuiltInSymbol( const std::string& symbol, double value );
-	void ChangeSymbol( const ScopedSymbolName& symbol, double value );
+	void ChangeSymbol( const ScopedSymbolName& symbol, Value value );
 	Value GetSymbol( const ScopedSymbolName& symbol ) const;
 	bool IsSymbolDefined( const ScopedSymbolName& symbol ) const;
 	void RemoveSymbol( const ScopedSymbolName& symbol );

test/2-expressions/macroParamEval.6502

@@ -0,0 +1,10 @@
+MACRO SHOW x, y
+PRINT x, y
+ASSERT(x=5)
+ASSERT(y=5)
+ENDMACRO
+
+\ Check that macro parameters are evaluated in the outer scope
+x=2
+y=3
+SHOW x+y, x+y

test/2-expressions/macroUndefined.6502

@@ -0,0 +1,23 @@
+MACRO LOAD address
+LDA address
+ENDMACRO
+
+ORG &2000
+\ On the first pass LOAD is called with an undefined symbol.
+\ Test that the undefinedness propagates to the macro so
+\ that the LDA has the correct width (i.e. not zero page).
+LOAD data
+RTS
+.data
+EQUB 2
+
+\ This example fails because zp moves out of zero page between
+\ passes.  It would be better if scopes couldn't override symbols
+\ from outer scopes.
+\zp=&70
+\{
+\    LDA zp
+\    RTS
+\    .zp
+\    EQUB 3
+\}