-
Notifications
You must be signed in to change notification settings - Fork 369
Mono engine using code segments to better parse scripts #771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
TBH, I would have liked this PR to be part of the 0.10 release as IMO the mono engine is still restricted without it. However, this is a big change so it’s also a bit risky. That’s why I haven’t been pushing for it. Unfortunately I created it after the code freeze. The current engine isn’t really user friendly as NRefactory can screw up the parsing if code contains blocks and give unexpected messages. Also often the line error messages are incorrect and exceptions not passed to be handled by ScriptCs.Core. This will be annoying for users scripting on the engine. As code speaks loader then words here are some code snippets. Block parsing failure class A { public string X { get { return "Class A"; } } }
if(true) { var x = 10; }
class B { public string X { get { return "Class B"; } } }
Console.WriteLine(new A().X);
Console.WriteLine(new B().X);Issue #713 var x = 123;
Action a = () => x++;
Console.WriteLine(x);REPL multiline Errors var a = 1;
var b = 2;
Foo(a,b);
public void Foo(int a, int b) { return a + b; }Current engine will give error message at line 2,2 While the new engine will return more correctly Also this PR will build bau scripts … checked both bau and bau-msbuild Just my 5 cent for now! |
|
this is awesome. I would suggest that I would suggest the same for REPL multiline, but this looks like it's too coupled to the new code. Makes sense to you? |
|
@filipw ... yes I know it's a big block! There are many issues interconnected here but I tried to keep he commits self contained. I can't see why I couldn't extract both MonoPrinter and MuliLine REPL into a separate PR . But now I'm off to watch the game :) |
|
@ztone hey man, do you mind picking this up like we discussed? It would be good to have 3 separate PRs here:
This way it would be easier to digest and error-proof. Thanks again, since this work is extremely valuable. If you need any help let me know. |
a80d687 to
a048e5f
Compare
|
Hi @filipw I've changed the PR to just include the lexer, which is the bulk of my former PR anyway. Cheers, |
|
@filipw are you looking into this? |
|
@filipw @adamralph I rebased this PR. Once I took a closer look I remembered that this PR has some needed features like keeping track of line numbers (as scripts are being re-ordered before evaluation) and maintains code format as classes and methods are re-formatted by NRefactory in our current solution. This is needed when we want to add our custom mono reporting class. Also because of this reformatting, implementing multiline support for classes is going to be awkward in our current solution. For example, NRefactory will add a missing curly bracket for classes which will mean that the Mono.CSharp will not think it’s an incomplete submission. Pros:
Cons:
|
|
great. IMHO this is good to merge. I know this is a massive changeset, but it was expected to be, since it introduces a lexer. I wouldn't worry about introducing bugs that much. Firstly, it has a bunch of tests like you mentioned. Secondly, I tested it out when it was brought forward originally and it worked like a charm. I say this should go in immediately, as it's definitely an improvement in our Mono support. unless someone wants to chime in? @scriptcs/core |
|
|
Mono engine using code segments to better parse scripts
|
Thanks @ztone! This will go out in 0.13. |
update by @adamralph
As merged, this fixes #713 only.
This PR is a proposed solution to get the Mono engine close to be at par with Roslyn and solve most of the Mono issues.
Why this approach
NRefactory failed to parse correctly if script contained blocks
{ }Needed a way to fix #713
What this PR does
So the approach I've taken is to split the script into code segments.
For example
Console.WriteLine(“hello”);is a region andif(true) { doStuff(); }is also a region. The lexer and parser don't care about the syntax or semantics, it's just used to do a logical split of the code.Segments are also ordered as we want to evaluate classes first, then method prototypes. After that we evaluate method expressions and lastly the evaluation.
If you look at ScriptCs code this approach does make some sense.
Here we identify three regions/segments. The Mono.CSharp evaluator will handle all the compiler errors that are inside each segment (and NRefactory for methods).
Other
error.didn't cause any failure (as the evaluator regards this as a partial, because of the dot). Handling partial loose functions is going to be a bit tricker and needs to be in another PR.Please test drive.
Stone