Refactor lookupCompanion #5700
Merged
Refactor lookupCompanion #5700
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d230729 to
eaa4a47
Compare
|
@lrytz There was a merge conflict with a typo fix that needed rebase. |
|
Needs another rebase due to due to |
eaa4a47 to
b1484ee
Compare
- Check for module class up front to use sourceModule - Consolidate most of the logic in Contexts
b1484ee to
06eee79
Compare
SethTisue
added a commit
to SethTisue/scala
that referenced
this pull request
Mar 6, 2017
only trivial merge conflict involving a method renaming * d1d700e - (origin/HEAD, origin/2.12.x) Merge pull request scala#5754 from Philippus/issue/html-tag-in-hover (2 days ago) <Lukas Rytz> |\ | * cf64718 - pattern for entitylink was too narrow, cleaned up the tests (3 days ago) <Philippus Baalman> * | f771395 - Merge pull request scala#5671 from retronym/topic/stubby-2 (3 days ago) <Lukas Rytz> |\ \ | * | ad13063 - Remove non-essential fix for stub symbol failure (4 days ago) <Jason Zaugg> | * | 7f2d1fa - Avoid forcing info transforms of primitive methods (2 weeks ago) <Jason Zaugg> | * | 37a0eb7 - Avoid stub symbol related crash in backend (2 weeks ago) <Jason Zaugg> * | | 96a7617 - Merge pull request scala#5622 from edmundnoble/extra-errs (4 days ago) <Adriaan Moors> |\ \ \ | * | | 466e52b - Match error lengths (4 weeks ago) <Edmund Noble> | * | | d1fc983 - Improved error messages for identically named, differently prefixed types (9 weeks ago) <Edmund Noble> | / / * | | f2e05c2 - Merge pull request scala#5728 from Philippus/issue/html-tag-in-hover (4 days ago) <Lukas Rytz> |\ \ \ | | |/ | |/| | * | e3c5df8 - added missing Inline matches to inlineToStr so it is now exhaustive scala.xml.XML.loadString(tag).text will remove all html tags inside the HtmlTag (9 days ago) <Philippus Baalman> * | | 920bc4e - Merge pull request scala#5743 from som-snytt/issue/10207-bad-update (7 days ago) <Lukas Rytz> |\ \ \ | * | | 094f7f9 - SI-10207 Error before update conversion (8 days ago) <Som Snytt> * | | | 1b4d36f - Merge pull request scala#5746 from paulp/pr/partest (7 days ago) <Lukas Rytz> |\ \ \ \ | |/ / / |/| | | | * | | eac00e1 - Add partest paths to the list of watched sources. (8 days ago) <Paul Phillips> |/ / / * | | 5f1a638 - Merge pull request scala#5732 from retronym/topic/build-info-malarkey (10 days ago) <Adriaan Moors> |\ \ \ | * | | 5e9acac - More predictable performance of SBT build startup, reload (11 days ago) <Jason Zaugg> | / / * | | 759a7b7 - Merge pull request scala#5735 from SethTisue/sd-313 (10 days ago) <Adriaan Moors> |\ \ \ | * | | ed4ddf5 - increase timeouts on some sys.process tests (11 days ago) <Seth Tisue> * | | | e2aaf2c - Merge pull request scala#5723 from dragos/issue/regression-assert-ide (11 days ago) <Lukas Rytz> |\ \ \ \ | |/ / / |/| | | | * | | e5c957e - Fix regression in 5751763 (12 days ago) <Iulian Dragos> * | | | f174bfb - Merge pull request scala#5731 from janekdb/issue/scalaGH-644/fix-spec-latex-rendering (11 days ago) <Seth Tisue> |\ \ \ \ | * | | | ba4c6d4 - scalaGH-644: Remove static html styling of spec code blocks (11 days ago) <Janek Bogucki> |/ / / / * | | | 8e40bef - Merge pull request scala#5729 from scala/revert-5658-topic/hashhash (12 days ago) <Adriaan Moors> |\ \ \ \ | |_|/ / |/| | | | * | | 86cd70f - (origin/revert-5658-topic/hashhash) Revert "Fix erasure of the qualifier of ##" (12 days ago) <Adriaan Moors> |/ / / * | | cbf7daa - Merge pull request scala#5681 from Philippus/issue/9704 (13 days ago) <Lukas Rytz> |\ \ \ | * | | b8a8ac1 - moved Pattern and TagsNotToClose to a HtmlTag companion object (13 days ago) <Philippus Baalman> | * | | a019082 - SI-9704 don't add a closed HtmlTag if it is already closed (4 weeks ago) <Philippus Baalman> | / / * | | effde0c - Merge pull request scala#5726 from scala/revert-5629-issue/10120-quote-err (13 days ago) <Adriaan Moors> |\ \ \ | * | | d60f6e3 - (origin/revert-5629-issue/10120-quote-err) Revert "SI-10133 Require escaped single quote char lit" (13 days ago) <Adriaan Moors> |/ / / * | | a8c4a54 - Merge pull request scala#5663 from gourlaysama/ticket/sd-256-enable-repl-colors-unix-2 (13 days ago) <Adriaan Moors> |\ \ \ | * | | 6411170 - SD-256 enable colored output by default on unix (13 days ago) <Antoine Gourlay> * | | | c96a977 - Merge pull request scala#5658 from retronym/topic/hashhash (13 days ago) <Lukas Rytz> |\ \ \ \ | |/ / / |/| | | | * | | f85c62e - Fix erasure of the qualifier of ## (6 weeks ago) <Jason Zaugg> | / / * | | 76bfb9e - Merge pull request scala#5708 from szeiger/issue/si10194 (13 days ago) <Lukas Rytz> |\ \ \ | * | | 1d22ee4 - SI-10194: Fix abstract type resolution for overloaded HOFs (13 days ago) <Stefan Zeiger> | / / * | | dabec1a - Merge pull request scala#5700 from retronym/ticket/10154-refactor (13 days ago) <Lukas Rytz> |\ \ \ | * | | 06eee79 - Refactor implementation of lookupCompanion (2 weeks ago) <Jason Zaugg> | / / * | | 2f1e0c2 - Merge pull request scala#5704 from som-snytt/issue/10190-elide-string (13 days ago) <Lukas Rytz> |\ \ \ | * | | 6fb3825 - SI-10190 Elide string to empty instead of null (3 weeks ago) <Som Snytt> | / / * | | 13f7b2a - Merge pull request scala#5640 from optimizely/repl-import-handler (2 weeks ago) <Adriaan Moors> |\ \ \ | * | | aa7e335 - Fix ImportHandler's reporting of importedNames and importedSymbols (8 weeks ago) <Hao Xia> | * | | c89d821 - Fix SIOOBE in Name#pos for substrings of length 1 (8 weeks ago) <Jason Zaugg> | / / * | | 023a96a - Merge pull request scala#5629 from som-snytt/issue/10120-quote-err (2 weeks ago) <Adriaan Moors> |\ \ \ | * | | 05cc3e2 - SI-10120 ReplReporter handles message indent (7 weeks ago) <Som Snytt> | * | | 939abf1 - SI-10120 Extra advice on unclosed char literal (8 weeks ago) <Som Snytt> | * | | 855492c - SI-10133 Require escaped single quote char lit (8 weeks ago) <Som Snytt> | / / * | | e21ab42 - Merge pull request scala#5660 from som-snytt/issue/9464-spec (2 weeks ago) <Adriaan Moors> |\ \ \ | * | | a6dcceb - SI-9464 Clarify spec on no final trait (6 weeks ago) <Som Snytt> | / / * | | e87a436 - Merge pull request scala#5659 from retronym/ticket/10026 (2 weeks ago) <Adriaan Moors> |\ \ \ | |/ / |/| | | * | 777a0e5 - SI-10026 Fix endless cycle in runtime reflection (2 weeks ago) <Jason Zaugg> | |/ * | 23e5ed9 - Merge pull request scala#5707 from retronym/topic/java9-prepare (2 weeks ago) <Lukas Rytz> |\ \ | * | 96e8e97 - Workaround bug in Scala runtime reflection on JDK 9 (3 weeks ago) <Jason Zaugg> | * | fe2d9a4 - Avoid ambiguous overload on JDK 9 (3 weeks ago) <Jason Zaugg> | * | 6bba8f7 - Adapt to change in ClassLoader in JDK 9 (3 weeks ago) <Jason Zaugg> | * | 8136057 - Bump scala-asm version (3 weeks ago) <Jason Zaugg> | / * | cad3c3d - Merge pull request scala#5709 from adriaanm/sam_wild_bound (2 weeks ago) <Lukas Rytz> |\ \ | * | c396e96 - Ignore BoundedWildcardType in erasure type map (2 weeks ago) <Adriaan Moors> * | | 3e9df41 - Merge pull request scala#5711 from retronym/ticket/jrt (2 weeks ago) <Lukas Rytz> |\ \ \ | * | | 09c7edc - Faster and simpler Java 9 classpath implementation (2 weeks ago) <Jason Zaugg> | / / * | | 7b9d3cc - Merge pull request scala#5713 from janekdb/issue/scalaGH-644/sync-jekyll-README-to-Gemfile (2 weeks ago) <Lukas Rytz> |\ \ \ | * | | 5e5ec9a - scalaGH-644: Expand note regarding Jekyll versions (2 weeks ago) <Janek Bogucki> | / / * | | 144f7e0 - Merge pull request scala#5714 from dragos/issue/usage-sterr-SI-10178 (2 weeks ago) <Lukas Rytz> |\ \ \ | |/ / |/| | | * | 640c85e - SI-10178 Route reporter.echo to stdout (2 weeks ago) <Iulian Dragos> | / * | 2fec08b - Merge pull request scala#5717 from som-snytt/issue/10148-followup (2 weeks ago) <Adriaan Moors> |\ \ | |/ |/| | * f3d271b - SI-10148 Accept verbose zero (2 weeks ago) <Som Snytt> * 147e5dd - Merge pull request scala#5716 from adriaanm/i296 (2 weeks ago) <Jason Zaugg> * 12437a0 - Ensure ordering for args to `choose` in DurationTest (2 weeks ago) <Adriaan Moors>
retronym
added a commit
to retronym/scala
that referenced
this pull request
Jan 25, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created.
retronym
added a commit
to retronym/scala
that referenced
this pull request
Jan 25, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created.
lrytz
pushed a commit
to retronym/scala
that referenced
this pull request
Jan 25, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created.
retronym
added a commit
to retronym/scala
that referenced
this pull request
Jan 29, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created.
retronym
added a commit
to retronym/scala
that referenced
this pull request
Sep 12, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created. (cherry picked from commit da14e9c)
adriaanm
pushed a commit
to retronym/scala
that referenced
this pull request
Oct 16, 2018
In scala#5700, I fixed a bug in the companion lookup, which ensured they were defined in the same scope. The same approach applies well to the lookup of default getters. You may ask, we can't just use: ``` context.lookupSymbol(name, _.owner == expectedOwner) ``` That doesn't individually lookup the entry in each enclosing nested scopes, but rather relies on the outer scope delegation in `Scope.lookupEntry` itself. This in turn relies on the way that nested scopes share the `elems` table with the enclosing scope: ``` final def newNestedScope(outer: Scope): Scope = { val nested = newScope nested.elems = outer.elems nested.nestinglevel = outer.nestinglevel + 1 ... } ``` If the outer scope is later mutated, in our case by lazily adding the default getter, the inner scope won't see the new elems. Context.lookupSymbol will jump immediately jump to search of the enclosing prefix. Perhaps a better design would be for the inner scope to retain a reference to the outer one, rather than just to the head of its elems linked list at the time the nested scope was created. (cherry picked from commit da14e9c)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followup for #5654