Skip to content
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

Refactor Contexts #7345

Closed
scabug opened this issue Apr 8, 2013 · 7 comments
Closed

Refactor Contexts #7345

scabug opened this issue Apr 8, 2013 · 7 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Apr 8, 2013

As noticed while fixing #7319:

 - encapsulate the error/warnings buffers in Contexts, and
   only expose the minimal representation necessary to propagate
   these between contexts, filter, and issue. This should fix
   a number of places where buffered warnings are not treated
   correctly (these were added recently, but places like `withSavedContext`
   were not updated.)
 - bring order to other menagerie of state within Context.
   It might be simpler and safer to carry it around in a nested
   case class, so we could use the 'copy' method to selectively
   modify one or two fields.
@scabug
Copy link
Author

scabug commented Apr 8, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7345?orig=1
Reporter: @retronym
Affected Versions: 2.10.0

@scabug
Copy link
Author

scabug commented Apr 8, 2013

@lrytz said:
also there are too many ways to construct contexts (5 overloads of make)

@scabug
Copy link
Author

scabug commented Apr 8, 2013

@hubertp said:
Depending on how far do you want to go with this refactoring we might think about ways to delay error message generation in the compiler.
Currently we deal mostly with strings, ideally we would like to deal with symbols and types. That is something javac people rewrote in the past 2-3 years as they were dealing with the same mess as we do now. It also enabled them to provide custom error message formatters that better describe problems. Unfortunately for us we modify the info a lot in some cases, so reference to the symbol or type might be pretty useless by the time you want to generate the error message.

@scabug
Copy link
Author

scabug commented Apr 8, 2013

@magarciaEPFL said:
What about SI-6149

@scabug
Copy link
Author

scabug commented Apr 8, 2013

@hubertp said:
Yeah, that's one of my favourites.

@scabug
Copy link
Author

scabug commented Apr 9, 2013

@retronym said:
I don't plan to tackle structured errors / delayed error message formatting as part of this ticket. Maybe next time...

As for the other points, here's my WIP:

https://github.com/retronym/scala/compare/ticket/7345-2

  • rationalized the overloads for context creation (three cheers for default params).
  • better encapsulation of the error/warning buffers
  • store all the boolean flags in a bitset ContextMode.

@hubert: do you have a sense as to which parts of Contexts is particularly performance sensitive (if any)? My profiling results suggest that the only part of interest (wrt CPU usage) is rootContext takes ~ 1% of the time generating the automatic imports (Predef._ et al.).

@scabug
Copy link
Author

scabug commented May 8, 2013

@lrytz said:
related question:

Most of the time, Typers uses context.makeNewScope, which calls newNestedScope(scope). Howeve, the context created to type check a template (in typedClassDef and typedModuleDef) is created using context.make(.., .., newScope), i.e. a non-nested scope. (Namers is equivalent).

I have no idea why this distinction is made, anybody who has could add a comment to the source code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants