• Leszek Swirski's avatar
    [parser] Use non-eval decl scope's parent for caching · fffea681
    Leszek Swirski authored
    We use the compilation entry point as a caching scope for deserializing
    lookups, to avoid redundantly iterating over parent scopes when
    accessing the same variable multiple times.
    
    However, this caching scope messes with lookups that are looking for
    lexical name conflicts, as opposed to just resolving variables. In
    particular, it messes with name conflict lookups and sloppy block
    function hoisting checks, when there are other scopes in the way, e.g.
    
        function f() {
          let x;
          try {
            throw 0;
          }
          catch (x) {
            // This catch is the entry scope
    
            // Naive use of caches will find the catch-bound x (which is
            // a VAR), and declare 'no conflict'.
            eval("var x;");
    
            // Naive use of caches will find the catch-bound x (which is
            // a VAR), and determine that this function can be hoisted.
            eval("{ function x() {} }");
          }
        }
    
    Previously, we worked around this by avoiding cache uses for these
    lookups, but this had the issue of instead caching the same variable
    multiple times, on different scopes. In particular, we saw:
    
        function f() {
          with ({}) {
            // This with is the entry scope, any other scope would do
            // though.
    
            // The conflict check on `var f` caches the function name
            // variable on the function scope, the subsequent 'real'
            // lookup of `f` caches the function name variable on the
            // entry i.e. with scope.
            eval("var f; f;");
          }
        }
    
    With this patch, we change the caching behaviour to cache on the first
    non-eval declaration scope above the eval -- in the above examples, this
    becomes the parent function "f". For compilations with no intermediate
    non-decl scopes (no with or catch scopes between the function and eval)
    this becomes equivalent to the existing entry-point-based caching.
    
    This means that normal lookups do have to (sometimes) iterate more scopes,
    and we do have to be careful when using the cache to not use it for
    lookups in these intermediate scopes (a new IsOuterScope DCHECK guards
    against this), but we can now safely ignore the cache scope when doing
    the name-collision lookups, as they only iterate up to the outer
    non-eval declaration scope anyway.
    
    Bug: chromium:1026603
    Bug: chromium:1029461
    Change-Id: I9e7a96ce4b8adbc7ed47a49fba6fba58b526235b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1955731
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65391}
    fffea681
scopes.cc 101 KB