Added support to indicate if function redifitinion is supported. (#342)

* For LazyJIT re-definition is NOT supported.
    - If it is possible to do, the solution , using only LLVM-C API is far from obvious. The asynchronous nature of resolution makes it a VERY complex problem. Fortunately, redefinition is a feature of dynamic runtimes that don't normally use lazy code execution so it's mostly an acedemic excercise. This implementation deals with it as an error at the AST generation with an error. The `DynamicRuntimeState` is used to indicate if the runtime supports re-definitions.
* Minor doc comment updates

Author: smaillet

src/Samples/Kaleidoscope/Chapter7.1/ReplEngine.cs

@@ -14,8 +14,9 @@ namespace Kaleidoscope.Chapter71
     internal class ReplEngine
         : ReadEvaluatePrintLoopBase<Value>
     {
+        // This uses Lazy JIT evaluation so redefinition is not supported.
         public ReplEngine( )
-            : base( LanguageLevel.MutableVariables )
+            : base( LanguageLevel.MutableVariables, functionRedfinitionIsAnError: true )
         {
         }
 

src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/AstBuilder.cs

@@ -133,16 +133,28 @@ public override IAstNode VisitFunctionDefinition( FunctionDefinitionContext cont
                                                );
 
             // only add valid definitions to the runtime state.
-            if(errors.Length == 0)
-            {
-                RuntimeState.FunctionDefinitions.AddOrReplaceItem( retVal );
-            }
-            else
+            if(errors.Length > 0)
             {
                 // remove the prototype implicitly added for this definition
                 // as the definition has errors
                 RuntimeState.FunctionDeclarations.Remove( sig );
             }
+            else
+            {
+                if(RuntimeState.SupportsRedefinition)
+                {
+                    RuntimeState.FunctionDefinitions.AddOrReplaceItem( retVal );
+                }
+                else
+                {
+                    if(RuntimeState.FunctionDefinitions.Contains(retVal.Name))
+                    {
+                        return new ErrorNode(retVal.Location, (int)DiagnosticCode.RedclarationNotSupported, "Duplicate function name, redefinitions not allowed." );
+                    }
+
+                    RuntimeState.FunctionDefinitions.Add( retVal );
+                }
+            }
 
             return retVal;
         }

src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/DiagnosticCode.cs

@@ -42,5 +42,8 @@ public enum DiagnosticCode
 
         /// <summary>Parse was cancelled</summary>
         ParseCanceled = 2000,
+
+        /// <summary>Re-declaration is not supported in the runtime</summary>
+        RedclarationNotSupported,
     }
 }

src/Samples/Kaleidoscope/Kaleidoscope.Grammar/DynamicRuntimeState.cs

@@ -34,16 +34,40 @@ namespace Kaleidoscope.Grammar
     /// </remarks>
     public class DynamicRuntimeState
     {
+        /// <remarks>This overload supports function re-definition for the normal case</remarks>
+        /// <inheritdoc cref="DynamicRuntimeState.DynamicRuntimeState(LanguageLevel, bool)"/>
+        public DynamicRuntimeState( LanguageLevel languageLevel )
+            : this(languageLevel, functionRedefinitionIsAnError: false)
+        {
+        }
+
         /// <summary>Initializes a new instance of the <see cref="DynamicRuntimeState"/> class.</summary>
+        /// <param name="functionRedefinitionIsAnError">Flag to indicate if function definitions are an error</param>
         /// <param name="languageLevel">Language level supported for this instance</param>
-        public DynamicRuntimeState( LanguageLevel languageLevel )
+        /// <remarks>
+        /// <see cref="SupportsRedefinition"/> is used to indicate if this language implementation supports
+        /// redefinition of functions. This is ordinarily allowed for interactive languages but if an extreme
+        /// lazy JIT is used, the asynchronous nature of materialization makes it a very difficult (maybe
+        /// impossible) task to accomplish. So Such runtimes will generally not support re-definition.
+        /// </remarks>
+        public DynamicRuntimeState( LanguageLevel languageLevel, bool functionRedefinitionIsAnError = false )
         {
             LanguageLevel = languageLevel;
+            SupportsRedefinition = !functionRedefinitionIsAnError;
         }
 
         /// <summary>Gets or sets the Language level the application supports</summary>
         public LanguageLevel LanguageLevel { get; set; }
 
+        /// <summary>Gets a value indicating whether this runtime supports redefinition of functions</summary>
+        /// <remarks>
+        /// Extreme Lazy JIT realization does not support redifinition at this point. It's s complicated problem
+        /// that is not entirely clear how to solve using the LLVM-C API. Thus, it is currently not supported. This
+        /// is generally not an issue as such lazy JIT execution is ordinarily not used for an interactive runtime
+        /// where function re-definition is used.
+        /// </remarks>
+        public bool SupportsRedefinition { get; init; }
+
         /// <summary>Gets a collection of function definitions parsed but not yet generated</summary>
         /// <remarks>
         /// This is a potentially dynamic set as parsing can add new entries. Also, when fully lazy

src/Samples/Kaleidoscope/Kaleidoscope.Grammar/Parser.cs

@@ -30,7 +30,16 @@ public class Parser
         /// <param name="level"><see cref="LanguageLevel"/> for the parser</param>
         /// <param name="visualizer">Visualizer to use for the parse (Includes possible error nodes)</param>
         public Parser( LanguageLevel level, IVisualizer? visualizer = null )
-            : this( new DynamicRuntimeState( level ), visualizer )
+            : this( new DynamicRuntimeState( level, functionRedefinitionIsAnError: false ), visualizer )
+        {
+        }
+
+        /// <summary>Initializes a new instance of the <see cref="Parser"/> class.</summary>
+        /// <param name="level"><see cref="LanguageLevel"/> for the parser</param>
+        /// <param name="functionRedefinitionIsAnError">flag to indicate if redefinition of a function is an error</param>
+        /// <param name="visualizer">Visualizer to use for the parse (Includes possible error nodes)</param>
+        public Parser( LanguageLevel level, bool functionRedefinitionIsAnError, IVisualizer? visualizer = null )
+            : this( new DynamicRuntimeState( level, functionRedefinitionIsAnError ), visualizer )
         {
         }
 

src/Samples/Kaleidoscope/Kaleidoscope.Runtime/ReadEvaluatePrintLoopBase.cs

@@ -21,6 +21,9 @@ public abstract class ReadEvaluatePrintLoopBase<T>
         /// <summary>Gets the Kaleidoscope language level for this instance</summary>
         public LanguageLevel LanguageFeatureLevel { get; }
 
+        /// <summary>Gets a value indicating whether this variant of the language runtime supports function re-definition</summary>
+        public bool SupportsRedefinition { get; }
+
         /// <summary>Gets the <see cref="TextWriter"/> to use of output from the loop</summary>
         public TextWriter Output { get; init; } = Console.Out;
 
@@ -59,31 +62,34 @@ public sealed override void ShowPrompt( ReadyState state )
         /// <returns><see cref="Task"/>for the REPL operation</returns>
         public async Task Run( TextReader input, IVisualizer? visualizer, CancellationToken cancelToken = default )
         {
-            var parser = new Kaleidoscope.Grammar.Parser(LanguageFeatureLevel, visualizer);
+            var parser = new Kaleidoscope.Grammar.Parser(LanguageFeatureLevel, functionRedefinitionIsAnError: !SupportsRedefinition, visualizer);
             using ICodeGenerator<T> generator = CreateGenerator( parser.GlobalState );
             await Run( input, parser, generator, cancelToken );
         }
 
         /// <summary>Initializes a new instance of the <see cref="ReadEvaluatePrintLoopBase{T}"/> class.</summary>
         /// <param name="level">Language level supported by this REPL instance</param>
+        /// <param name="functionRedfinitionIsAnError">Flag to indicate whether function redefinition is an error or allowed</param>
         /// <remarks>
         /// This is protected to prevent use by anything other than a derived type.
         /// </remarks>
-        protected ReadEvaluatePrintLoopBase( LanguageLevel level )
-            : this(level, new ParseErrorDiagnosticAdapter(new ColoredConsoleReporter(), "KLS"))
+        protected ReadEvaluatePrintLoopBase( LanguageLevel level, bool functionRedfinitionIsAnError = false )
+            : this(level, new ParseErrorDiagnosticAdapter(new ColoredConsoleReporter(), "KLS"), functionRedfinitionIsAnError )
         {
         }
 
         /// <summary>Initializes a new instance of the <see cref="ReadEvaluatePrintLoopBase{T}"/> class.</summary>
         /// <param name="level">Language level supported by this REPL instance</param>
         /// <param name="logger">Logger to report any issues parsing the input.</param>
+        /// <param name="functionRedfinitionIsAnError">Flag to indicate whether function redefinition is an error or allowed</param>
         /// <remarks>
         /// This is protected to prevent use by anything other than a derived type.
         /// </remarks>
-        protected ReadEvaluatePrintLoopBase( LanguageLevel level, IParseErrorReporter logger )
+        protected ReadEvaluatePrintLoopBase( LanguageLevel level, IParseErrorReporter logger, bool functionRedfinitionIsAnError = false )
             : base( logger )
         {
             LanguageFeatureLevel = level;
+            SupportsRedefinition = !functionRedfinitionIsAnError;
         }
     }
 }

src/Samples/Kaleidoscope/Kaleidoscope.Tests/BasicTests.cs

@@ -2,12 +2,14 @@
 // Licensed under the Apache-2.0 WITH LLVM-exception license. See the LICENSE.md file in the project root for full license information.
 
 using System;
+using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
 
 using Kaleidoscope.Grammar;
+using Kaleidoscope.Grammar.AST;
 
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -50,74 +52,108 @@ public async Task Chapter2( )
         public async Task Chapter3( )
         {
             using var input = File.OpenText( "simpleExpressions.kls" );
-            await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter3.CodeGenerator( state ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter3.CodeGenerator( state ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter3_5( )
         {
             using var input = File.OpenText( "simpleExpressions.kls" );
-            await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter3_5.CodeGenerator( state ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter3_5.CodeGenerator( state ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter4( )
         {
             using var input = File.OpenText( "simpleExpressions.kls" );
-            await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter4.CodeGenerator( state, writer ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.SimpleExpressions, input, ( state, writer ) => new Chapter4.CodeGenerator( state, writer ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter5( )
         {
             using var input = File.OpenText( "ControlFlow.kls" );
-            await RunBasicReplLoop( LanguageLevel.ControlFlow, input, ( state, writer ) => new Chapter5.CodeGenerator( state, writer ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.ControlFlow, input, ( state, writer ) => new Chapter5.CodeGenerator( state, writer ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter6( )
         {
             using var input = File.OpenText( "mandel.kls" );
-            await RunBasicReplLoop( LanguageLevel.UserDefinedOperators, input, ( state, writer ) => new Chapter6.CodeGenerator( state, writer ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.UserDefinedOperators, input, ( state, writer ) => new Chapter6.CodeGenerator( state, writer ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter7( )
         {
             using var input = File.OpenText( "fibi.kls" );
-            await RunBasicReplLoop( LanguageLevel.MutableVariables, input, ( state, writer ) => new Chapter7.CodeGenerator( state, writer ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.MutableVariables, input, ( state, writer ) => new Chapter7.CodeGenerator( state, writer ) );
+            Assert.IsEmpty( errors );
         }
 
         [TestMethod]
         [Description( "Basic test of Chapter parsing and code generation to ensure it doesn't crash on well-known good input [output is not validated in this test]" )]
         public async Task Chapter71( )
         {
             using var input = File.OpenText( "fibi.kls" );
-            await RunBasicReplLoop( LanguageLevel.MutableVariables, input, ( state, writer ) => new Chapter71.CodeGenerator( state, writer ) );
+            var errors = await RunBasicReplLoop( LanguageLevel.MutableVariables, input, ( state, writer ) => new Chapter71.CodeGenerator( state, writer ) );
+            Assert.IsEmpty( errors );
         }
 
-        private async Task RunBasicReplLoop( LanguageLevel level
-                                           , TextReader input
-                                           , Func<DynamicRuntimeState, TextWriter, ICodeGenerator<Value>> generatorFactory
-                                           )
+        [TestMethod]
+        [Description( "Test of redefinition not supported with lazy JIT [output is not validated in this test]" )]
+        public async Task Chapter71_with_redefinition( )
+        {
+            using var input = File.OpenText( "Redefinition.kls" );
+            var errors = await RunBasicReplLoop(
+                                   LanguageLevel.MutableVariables,
+                                   functionRedefinitionIsAnError: true,
+                                   input,
+                                   ( state, writer ) => new Chapter71.CodeGenerator( state, writer )
+                               );
+            Assert.HasCount( 1, errors );
+            Assert.AreEqual( (int)DiagnosticCode.RedclarationNotSupported, errors[ 0 ].Code );
+        }
+
+        private Task<ImmutableArray<ErrorNode>> RunBasicReplLoop( LanguageLevel level
+                                   , TextReader input
+                                   , Func<DynamicRuntimeState, TextWriter, ICodeGenerator<Value>> generatorFactory
+                                   )
+        {
+            return RunBasicReplLoop( level, functionRedefinitionIsAnError: false, input, generatorFactory );
+        }
+
+        private async Task<ImmutableArray<ErrorNode>> RunBasicReplLoop( LanguageLevel level
+                                                                      , bool functionRedefinitionIsAnError
+                                                                      , TextReader input
+                                                                      , Func<DynamicRuntimeState, TextWriter, ICodeGenerator<Value>> generatorFactory
+                                                                      )
         {
-            var parser = new Parser( level );
+            var parser = new Parser( level, functionRedefinitionIsAnError );
             using var outputWriter = new TestContextTextWriter( RuntimeContext );
             using var generator = generatorFactory( parser.GlobalState, outputWriter );
 
             // Create sequence of parsed AST RootNodes to feed the 'REPL' loop
-            var replSeq = from stmt in input.ToStatements( _=>{ }, RuntimeContext.CancellationTokenSource.Token )
+            var replSeq = from stmt in input.ToStatements( _=>{ }, RuntimeContext.CancellationToken )
                           select parser.Parse( stmt );
 
             await foreach(IAstNode node in replSeq)
             {
                 var errors = node.CollectErrors();
-                Assert.IsEmpty( errors );
+                if(errors.Length > 0)
+                {
+                    return errors;
+                }
 
                 var result = generator.Generate( node );
 
@@ -139,6 +175,8 @@ private async Task RunBasicReplLoop( LanguageLevel level
                     }
                 }
             }
+
+            return [];
         }
 
         private readonly TestContext RuntimeContext;

src/Samples/Kaleidoscope/Kaleidoscope.Tests/Kaleidoscope.Tests.csproj

@@ -34,6 +34,9 @@
         <None Update="mandel.kls">
             <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
         </None>
+        <None Update="Redefinition.kls">
+          <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+        </None>
         <None Update="SimpleExpressions.kls">
             <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
         </None>

src/Samples/Kaleidoscope/Kaleidoscope.Tests/Redefinition.kls

@@ -0,0 +1,9 @@
+# This is intended as a test case to identify issues with redefinition of
+# symbols in a LazyJIT scenario. (Either it works or is at least flagged
+# as an error before triggering an error deep in LLVM.
+
+def foo(a b) a*a + 2*a*b + b*b;
+
+# Ideally, this second definition should replace the first, or produce
+# an error not crash with a non-string LLVMErrorRef...
+def foo(a b) a*a + 2*a*b;

src/Samples/Kaleidoscope/Kaleidoscope.Tests/TestContextTextWriter.cs

@@ -8,7 +8,7 @@
 
 namespace Kaleidoscope.Tests
 {
-    // Used as a target to output text of the runs to the test context
+    // Adapter - Used as a target to output text of the runs to the test context
     internal class TestContextTextWriter
         : TextWriter
     {