From 2927e5ab0c91587033da7b31e7c6d963ef356c3b Mon Sep 17 00:00:00 2001 From: Steven Maillet Date: Wed, 29 Oct 2025 15:02:24 -0700 Subject: [PATCH] Added support to indicate if function redifitinion is supported. * 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 --- .../Kaleidoscope/Chapter7.1/ReplEngine.cs | 3 +- .../Kaleidoscope.Grammar/AST/AstBuilder.cs | 22 +++++-- .../AST/DiagnosticCode.cs | 3 + .../DynamicRuntimeState.cs | 26 +++++++- .../Kaleidoscope.Grammar/Parser.cs | 11 +++- .../ReadEvaluatePrintLoopBase.cs | 14 ++-- .../Kaleidoscope.Tests/BasicTests.cs | 66 +++++++++++++++---- .../Kaleidoscope.Tests.csproj | 3 + .../Kaleidoscope.Tests/Redefinition.kls | 9 +++ .../TestContextTextWriter.cs | 2 +- .../Kaleidoscope/Kaleidoscope.Tests/foo.bar | 0 .../OrcJITv2/MaterializationUnit.cs | 8 +++ 12 files changed, 140 insertions(+), 27 deletions(-) create mode 100644 src/Samples/Kaleidoscope/Kaleidoscope.Tests/Redefinition.kls create mode 100644 src/Samples/Kaleidoscope/Kaleidoscope.Tests/foo.bar diff --git a/src/Samples/Kaleidoscope/Chapter7.1/ReplEngine.cs b/src/Samples/Kaleidoscope/Chapter7.1/ReplEngine.cs index c6d54cb1e5..b5dec44249 100644 --- a/src/Samples/Kaleidoscope/Chapter7.1/ReplEngine.cs +++ b/src/Samples/Kaleidoscope/Chapter7.1/ReplEngine.cs @@ -14,8 +14,9 @@ namespace Kaleidoscope.Chapter71 internal class ReplEngine : ReadEvaluatePrintLoopBase { + // This uses Lazy JIT evaluation so redefinition is not supported. public ReplEngine( ) - : base( LanguageLevel.MutableVariables ) + : base( LanguageLevel.MutableVariables, functionRedfinitionIsAnError: true ) { } diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/AstBuilder.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/AstBuilder.cs index 8043434058..5af3ed7a3b 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/AstBuilder.cs +++ b/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; } diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/DiagnosticCode.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/DiagnosticCode.cs index 5deddd8293..097cd4bbc2 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/DiagnosticCode.cs +++ b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/AST/DiagnosticCode.cs @@ -42,5 +42,8 @@ public enum DiagnosticCode /// Parse was cancelled ParseCanceled = 2000, + + /// Re-declaration is not supported in the runtime + RedclarationNotSupported, } } diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/DynamicRuntimeState.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/DynamicRuntimeState.cs index 081570caad..df26d8af9c 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/DynamicRuntimeState.cs +++ b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/DynamicRuntimeState.cs @@ -34,16 +34,40 @@ namespace Kaleidoscope.Grammar /// public class DynamicRuntimeState { + /// This overload supports function re-definition for the normal case + /// + public DynamicRuntimeState( LanguageLevel languageLevel ) + : this(languageLevel, functionRedefinitionIsAnError: false) + { + } + /// Initializes a new instance of the class. + /// Flag to indicate if function definitions are an error /// Language level supported for this instance - public DynamicRuntimeState( LanguageLevel languageLevel ) + /// + /// 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. + /// + public DynamicRuntimeState( LanguageLevel languageLevel, bool functionRedefinitionIsAnError = false ) { LanguageLevel = languageLevel; + SupportsRedefinition = !functionRedefinitionIsAnError; } /// Gets or sets the Language level the application supports public LanguageLevel LanguageLevel { get; set; } + /// Gets a value indicating whether this runtime supports redefinition of functions + /// + /// 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. + /// + public bool SupportsRedefinition { get; init; } + /// Gets a collection of function definitions parsed but not yet generated /// /// This is a potentially dynamic set as parsing can add new entries. Also, when fully lazy diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/Parser.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/Parser.cs index 397385bdc4..cb52d58a88 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/Parser.cs +++ b/src/Samples/Kaleidoscope/Kaleidoscope.Grammar/Parser.cs @@ -30,7 +30,16 @@ public class Parser /// for the parser /// Visualizer to use for the parse (Includes possible error nodes) public Parser( LanguageLevel level, IVisualizer? visualizer = null ) - : this( new DynamicRuntimeState( level ), visualizer ) + : this( new DynamicRuntimeState( level, functionRedefinitionIsAnError: false ), visualizer ) + { + } + + /// Initializes a new instance of the class. + /// for the parser + /// flag to indicate if redefinition of a function is an error + /// Visualizer to use for the parse (Includes possible error nodes) + public Parser( LanguageLevel level, bool functionRedefinitionIsAnError, IVisualizer? visualizer = null ) + : this( new DynamicRuntimeState( level, functionRedefinitionIsAnError ), visualizer ) { } diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Runtime/ReadEvaluatePrintLoopBase.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Runtime/ReadEvaluatePrintLoopBase.cs index bf511cb8af..a34cc0a4c8 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Runtime/ReadEvaluatePrintLoopBase.cs +++ b/src/Samples/Kaleidoscope/Kaleidoscope.Runtime/ReadEvaluatePrintLoopBase.cs @@ -21,6 +21,9 @@ public abstract class ReadEvaluatePrintLoopBase /// Gets the Kaleidoscope language level for this instance public LanguageLevel LanguageFeatureLevel { get; } + /// Gets a value indicating whether this variant of the language runtime supports function re-definition + public bool SupportsRedefinition { get; } + /// Gets the to use of output from the loop public TextWriter Output { get; init; } = Console.Out; @@ -59,31 +62,34 @@ public sealed override void ShowPrompt( ReadyState state ) /// for the REPL operation 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 generator = CreateGenerator( parser.GlobalState ); await Run( input, parser, generator, cancelToken ); } /// Initializes a new instance of the class. /// Language level supported by this REPL instance + /// Flag to indicate whether function redefinition is an error or allowed /// /// This is protected to prevent use by anything other than a derived type. /// - 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 ) { } /// Initializes a new instance of the class. /// Language level supported by this REPL instance /// Logger to report any issues parsing the input. + /// Flag to indicate whether function redefinition is an error or allowed /// /// This is protected to prevent use by anything other than a derived type. /// - protected ReadEvaluatePrintLoopBase( LanguageLevel level, IParseErrorReporter logger ) + protected ReadEvaluatePrintLoopBase( LanguageLevel level, IParseErrorReporter logger, bool functionRedfinitionIsAnError = false ) : base( logger ) { LanguageFeatureLevel = level; + SupportsRedefinition = !functionRedfinitionIsAnError; } } } diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/BasicTests.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/BasicTests.cs index c750d0ca16..8ad032ec78 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/BasicTests.cs +++ b/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,7 +52,8 @@ 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] @@ -58,7 +61,8 @@ public async Task Chapter3( ) 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] @@ -66,7 +70,8 @@ public async Task Chapter3_5( ) 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] @@ -74,7 +79,8 @@ public async Task Chapter4( ) 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] @@ -82,7 +88,8 @@ public async Task Chapter5( ) 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] @@ -90,7 +97,8 @@ public async Task Chapter6( ) 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] @@ -98,26 +106,54 @@ public async Task Chapter7( ) 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> 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> RunBasicReplLoop( LanguageLevel level + , TextReader input + , Func> generatorFactory + ) + { + return RunBasicReplLoop( level, functionRedefinitionIsAnError: false, input, generatorFactory ); + } + + private async Task> RunBasicReplLoop( LanguageLevel level + , bool functionRedefinitionIsAnError + , TextReader input + , Func> 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; diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Kaleidoscope.Tests.csproj b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Kaleidoscope.Tests.csproj index eb188b74d3..453064468b 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Kaleidoscope.Tests.csproj +++ b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Kaleidoscope.Tests.csproj @@ -34,6 +34,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Redefinition.kls b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/Redefinition.kls new file mode 100644 index 0000000000..d2c0bc56c5 --- /dev/null +++ b/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; diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/TestContextTextWriter.cs b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/TestContextTextWriter.cs index 2cf7f1428e..d306934e31 100644 --- a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/TestContextTextWriter.cs +++ 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 { diff --git a/src/Samples/Kaleidoscope/Kaleidoscope.Tests/foo.bar b/src/Samples/Kaleidoscope/Kaleidoscope.Tests/foo.bar new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/Ubiquity.NET.Llvm/OrcJITv2/MaterializationUnit.cs b/src/Ubiquity.NET.Llvm/OrcJITv2/MaterializationUnit.cs index e6a89e42b5..aa1afadac5 100644 --- a/src/Ubiquity.NET.Llvm/OrcJITv2/MaterializationUnit.cs +++ b/src/Ubiquity.NET.Llvm/OrcJITv2/MaterializationUnit.cs @@ -4,12 +4,19 @@ // Elements ARE ordered correctly, analyzer has dumb defaults and doesn't allow override of order #pragma warning disable SA1202 // Elements should be ordered by access +using static Ubiquity.NET.Llvm.Interop.ABI.llvm_c.Orc; + namespace Ubiquity.NET.Llvm.OrcJITv2 { /// Abstract base class for an LLVM ORC JIT v2 Materialization Unit public abstract class MaterializationUnit : DisposableObject { + /// + /// For this class, this is an idempotent method. This allows MOVE semantics for native + /// code to function and callers remain oblivious. Callers should always call this for + /// correctness if it was succesfully moved to native code then such a call is a NOP. + /// /// protected override void Dispose( bool disposing ) { @@ -22,6 +29,7 @@ protected override void Dispose( bool disposing ) base.Dispose( disposing ); } + /// This will set the internal handle to a default state; makeing a NOP [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void InvalidateAfterMove( ) {