ScriptNode : Support multi-line if in tolerantExec()

Although we don't make use of any compound statements in our own serialisations, third parties have done so using custom serialisers. Parsing for those was broken by 9d15bd2 when we
stopped using an AST for parsing. Here we do the absolute bare minimum necessary to get these legacy files loading. As noted in the comments, we do not intend to do more : rather our future intention is likely to be constrain the serialisation format further for improved performance.

Changes.md

@@ -27,6 +27,7 @@ Fixes
   - Fixed loading of OpenEXR images with 32 bit float data and DWA compression [^1].
   - Fixed loading of secondary layers in OpenEXR images with DWA compression [^1].
 - GUI : Fixed potential crashes during shutdown [^1].
+- ScriptNode : Fixed execution of multi-line `if :` statements in `.gfr` files [^1].
 
 API
 ---

python/GafferTest/ScriptNodeTest.py

@@ -1763,5 +1763,79 @@ def testDeletingNodeRemovesFocus( self ) :
 		self.assertEqual( memberRemovals, [ ( s.focusSet(), n ) ] )
 		self.assertEqual( memberAdditions, [] )
 
+	def testExecuteWithConditionals( self ) :
+
+		script = Gaffer.ScriptNode()
+
+		script.execute(
+			inspect.cleandoc(
+				"""
+				if True :
+					script.addChild( Gaffer.Node( "True1" ) )
+				script.addChild( Gaffer.Node( "Unconditional1" ) )
+
+				if False :
+					script.addChild( Gaffer.Node( "False" ) )
+
+				if True :
+					script.addChild( Gaffer.Node( "True2" ) )
+					script.addChild( Gaffer.Node( "True3" ) )
+				script.addChild( Gaffer.Node( "Unconditional2" ) )
+
+				if(True):
+					script.addChild( Gaffer.Node( "True4" ) )
+
+				script.addChild( Gaffer.Node( "Unconditional3" ) )
+
+				if(True) :
+					script.addChild( Gaffer.Node( "True5" ) )
+
+				if (True):
+					script.addChild( Gaffer.Node( "True6" ) )
+
+				if True : script.addChild( Gaffer.Node( "True7" ) )
+				if False : script.addChild( Gaffer.Node( "False" ) )
+
+				"""
+			),
+			continueOnError = True
+		)
+
+		self.assertIn( "True1", script )
+		self.assertIn( "True2", script )
+		self.assertIn( "True3", script )
+		self.assertIn( "True4", script )
+		self.assertIn( "True5", script )
+		self.assertIn( "True6", script )
+		self.assertIn( "True7", script )
+		self.assertIn( "Unconditional1", script )
+		self.assertIn( "Unconditional2", script )
+		self.assertIn( "Unconditional3", script )
+		self.assertNotIn( "False", script )
+		self.assertNotIn( "True8", script )
+
+	def testLineNumbersAfterConditionalExecution( self ) :
+
+		script = Gaffer.ScriptNode()
+
+		with IECore.CapturingMessageHandler() as mh :
+			script.execute(
+				inspect.cleandoc(
+					"""
+					if True :
+						script.addChild( Gaffer.Node( "True" ) )
+					a = 10 * b
+					"""
+				),
+				continueOnError = True
+			)
+
+		self.assertIn( "True", script )
+
+		self.assertEqual( len( mh.messages ), 1 )
+		self.assertEqual( mh.messages[0].level, IECore.Msg.Level.Error )
+		self.assertTrue( "Line 3" in mh.messages[0].context )
+		self.assertTrue( "name 'b' is not defined" in mh.messages[0].message )
+
 if __name__ == "__main__":
 	unittest.main()

src/GafferModule/ScriptNodeBinding.cpp

@@ -65,6 +65,7 @@
 #include "fmt/format.h"
 
 #include <memory>
+#include <regex>
 
 using namespace boost;
 using namespace Gaffer;
@@ -85,12 +86,15 @@ const std::string formattedErrorContext( int lineNumber, const std::string &cont
 	);
 }
 
+const std::regex g_blockStartRegex( R"(^if[ \t(])" );
+const std::regex g_blockContinuationRegex( R"(^[ \t]+)" );
+
 // Execute the script one line at a time, reporting errors that occur,
 // but otherwise continuing with execution.
 bool tolerantExec( const std::string &pythonScript, boost::python::object globals, boost::python::object locals, const std::string &context )
 {
 	bool result = false;
-	int lineNumber = 1;
+	int lineNumber = 0;
 
 	const IECore::Canceller *canceller = Context::current()->canceller();
 
@@ -99,18 +103,56 @@ bool tolerantExec( const std::string &pythonScript, boost::python::object global
 	{
 		IECore::Canceller::check( canceller );
 
-		const std::string line( it->begin(), it->end() );
+		std::string toExecute( it->begin(), it->end() );
+		++it; ++lineNumber;
+
+		// Our serialisations have always been in a form that can be executed
+		// line-by-line. But certain third parties have used custom serialisers
+		// that output multi-line `if` statements that must be executed in a
+		// single call. Here we detect them and group them together.
+		//
+		// Notes :
+		//
+		// - Historically, we used a Python C AST API to support _any_ compound
+		//   statements here. But that is [no longer available](9d15bd21c4049d89118faf3ac27d01c5799b8264),
+		//   and using the `ast` module instead would be a significant performance
+		//   regression.
+		// - While using Python as our serialisation format is great from an
+		//   educational and reuse perspective, it has never been good from a
+		//   performance perspective. A likely future direction is to constrain
+		//   the serialisation syntax further such that it is still valid
+		//   Python, but we can parse and execute the majority of it directly in C++
+		//   for improvement performance.
+		// - We are therefore deliberately supporting only the absolute minimum of syntax
+		//   needed for the legacy third-party serialisations here, to give us
+		//   more flexibility in optimising the parsing in future.
+		if( std::regex_search( toExecute, g_blockStartRegex )  )
+		{
+			while( it != split_iterator<std::string::const_iterator>() )
+			{
+				const std::string line( it->begin(), it->end() );
+				if( std::regex_search( line, g_blockContinuationRegex ) )
+				{
+					toExecute += "\n" + line;
+					++it; ++lineNumber;
+				}
+				else
+				{
+					break;
+				}
+			}
+		}
+
 		try
 		{
-			exec( line.c_str(), globals, locals );
+			exec( toExecute.c_str(), globals, locals );
 		}
 		catch( const boost::python::error_already_set & )
 		{
 			const std::string message = IECorePython::ExceptionAlgo::formatPythonException( /* withTraceback = */ false );
 			IECore::msg( IECore::Msg::Error, formattedErrorContext( lineNumber, context ), message );
 			result = true;
 		}
-		++it; ++lineNumber;
 	}
 
 	return result;