Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Scala 2.10.0
    • Fix Version/s: Scala 2.11.0-M4
    • Component/s: Type Checker
    • Labels:
      None

      Description

      As noticed while fixing SI-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.

        Issue Links

          Activity

          Hide
          Lukas Rytz added a comment -

          also there are too many ways to construct contexts (5 overloads of `make`)

          Show
          Lukas Rytz added a comment - also there are too many ways to construct contexts (5 overloads of `make`)
          Hide
          Hubert Plociniczak added a comment -

          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.

          Show
          Hubert Plociniczak added a comment - 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.
          Hide
          Miguel Garcia added a comment -

          What about SI-6149

          Show
          Miguel Garcia added a comment - What about SI-6149
          Hide
          Hubert Plociniczak added a comment -

          Yeah, that's one of my favourites.

          Show
          Hubert Plociniczak added a comment - Yeah, that's one of my favourites.
          Hide
          Jason Zaugg added a comment -

          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.).

          Show
          Jason Zaugg added a comment - 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.).
          Hide
          Lukas Rytz added a comment -

          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

          Show
          Lukas Rytz added a comment - 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

            People

            • Assignee:
              Jason Zaugg
              Reporter:
              Jason Zaugg
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development