Skip to content

Removing global state#600

Merged
marcoeilers merged 9 commits into
masterfrom
meilers_noglobalstate
Sep 1, 2022
Merged

Removing global state#600
marcoeilers merged 9 commits into
masterfrom
meilers_noglobalstate

Conversation

@marcoeilers

Copy link
Copy Markdown
Contributor

Removing global state from Silver in order to be able to run multiple instances of Silicon or Carbon in parallel (which several frontends would like to do).

In particular,

  • we split the FastParser object into a stateless object that supplies constants and defines some parsing primitives, and a class FastParser that contains mutable state (e.g., related to the current file and positions).
  • The ParserExtensions object (which contains mutable state) is moved into the FastParser class, s.t. each parser instance has its own ParserExtensions object.
  • Some fresh-value-counters in ParseAST classes are replaced by AtomicInt objects to allow concurrent accesses.

As a result, all code that used to directly access FastParser is now passed an instance of FastParser instead or has to create one itself. In particular, the interface of Viper plugins is extended s.t. their constructor now gets an additional FastParser argument.

@marcoeilers

Copy link
Copy Markdown
Contributor Author

@JonasAlaif So what do I do here now to get the backend tests to pass (since this PR depends on a Silicon PR viperproject/silicon#635 and vice versa)?

@JonasAlaif

JonasAlaif commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

@marcoeilers merge the silicon PR first (also looks like you'll need to update carbon) and afterwards either a) rerun the tests here or b) just merge it without rerunning the tests since you saw that those tests passed in the silicon PR with the same version of silver

@jcp19 jcp19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not super well-versed in this part of the code, so I would suggest waiting for another review before merging. From my side, I have some minor remarks

Comment thread src/main/scala/viper/silver/parser/FastParser.scala Outdated
Comment thread src/main/scala/viper/silver/ast/utility/Consistency.scala Outdated
Comment thread src/main/scala/viper/silver/parser/ParseAst.scala

@vakaras vakaras left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach sounds good, but I am unfamiliar with the code to review it properly.

@marcoeilers marcoeilers merged commit 3007816 into master Sep 1, 2022
@marcoeilers marcoeilers deleted the meilers_noglobalstate branch September 1, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants