pharo icon indicating copy to clipboard operation
pharo copied to clipboard

[Compiler] Stateless OpalCompiler

Open dionisiydk opened this issue 2 years ago • 1 comments

  1. PR makes OpalCompiler stateless (with temp state deprecation):

With the recent changes to the compiler it is now clear that we can easily remove #source and #ast variables and reuse the compiler instance for parsing/compilation of multiple sources. This way we can get rid of CompilationContext moving all its state to the compiler. So that we will have logical reference from the method/AST to the producer compiler instance (currently it is the reference to the CompilationContext). Then many tools can be simplified by reusing a same compiler instance for own needs (highlighting for example).

Here PR performs main changes to the users of #compiler:

  • Variables #source and #ast are removed (source is temporally deprecated for integration).
  • API is changed to be #compile:/parse:/#evaluate: with source code as a parameter:
OpalCompiler new noPattern: true; compile: '1+2'.

Versus old style:

OpalCompiler new source: '1+2'; noPattern: true; compile.

The new style API was always there but it is now implemented without instance variables #source and #ast. Most of the changes in PR is such a rewrite from old style to new one.

  • #source:, #compile, #evaluate methods are deprecated (will be removed after NewTools and Spec fixes).
  • All users of compiler are adopted accordingly

The deprecation of #source variable is required to get safe independent adoption of NewTools and Spec. After that the variables will be removed.

The inline of CompilerContext will be done in separate PR.

  1. Environment parameter is moved from CompilationContext into OCSemanticScope as #targetEnvironment which uses the environment of #targetClass by default.

    • it makes the #environment configuration really working. Previously it did nothing. See the changes in #testCompileWithEnvironment.
  2. Introduce compilation option #optionEmbeddAST to remove #compileDoItFromAST method. Now #doIt compilation simply enables #optionEmbeddAST option (just as #optionEmbeddSources option) and a scope indirection is not needed anymore. See #generateMethod.

dionisiydk avatar Jul 27 '22 22:07 dionisiydk

failing tests not related

  • #skipOnPharoCITestingEnvironment seems to not work on WIN and MAC #11425
  • Failing on CI (random): CoNarrowHistoryFetcherTest>>testNarrowingReturnsSameElementsThatCallingDirectly #11368

MarcusDenker avatar Aug 17 '22 09:08 MarcusDenker

What's the next step here? I see many merge conflicts.

guillep avatar Jan 27 '23 10:01 guillep

What's the next step here? I see many merge conflicts.

I am waiting @MarcusDenker review for this idea.

Conflicts are resolved

dionisiydk avatar Jan 28 '23 11:01 dionisiydk

conflicts were badly fixed . I will revisit it. But it does not prevent the review

dionisiydk avatar Jan 28 '23 12:01 dionisiydk

The PR https://github.com/pharo-project/pharo/pull/13152 now adds some more state.. (see discussion)

I need to think about this more (stateless compiler, CompilationContext is for sure now holding on to too much state...)

MarcusDenker avatar Mar 28 '23 07:03 MarcusDenker

I added the cleaning of the state in CompilationContext and OpalCompiler to #12883, I will think about that.

privat avatar Mar 28 '23 11:03 privat

So what is the status? I have the impression that the issue is not going to be integratable.

Ducasse avatar Nov 25 '23 13:11 Ducasse