Redesign Names. #366
Merged
Redesign Names. #366
Conversation
This file contains 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
A name is almost never empty. The only known case is for a given import, which already has a dedicated accessor `isGiven` to present to the public API.
Then remove `Name.tag`.
This means that `Printers` do not have to specifically print it itself. It also means that the `SignatureSuite` becomes more precise, as we can actually make the difference between class names and module class names in the string representations that we use for testing.
Previously, there was an invariant that the last name in a signature name was always a `TypeName`, and all the other ones were `TermName`s. Instead, we now declare that they must all be `TermName`s. This removes the need for a number of `mapLast`s, and simplifies manipulations and invariants.
…ullName`. `fullName` is not meaningful on arbitrary symbols. Having it return a supposedly structured `FullyQualifiedName` is misleading. We move it `PackageSymbol`, for which it is actually meaningful and used. In `Symbol`, we introduce `displayFullName` instead, which does the same as what the previous `fullName` did but returned as a `String`. This is meant to be used for display purposes only, such as debugging or error messages.
bishabosha
reviewed
Oct 20, 2023
| @@ -1689,6 +1696,12 @@ object Symbols { | |||
| this.withFlags(EmptyFlagSet, None) | |||
| this.setAnnotations(Nil) | |||
|
|
|||
| private lazy val _fullName: FullyQualifiedName = | |||
| if owner == null || name == nme.EmptyPackageName then FullyQualifiedName.rootPackageName | |||
There was a problem hiding this comment.
why isn't this emptyPackageName for the empty package? I guess consistency in that <empty> does not appear in the prefix, but just checking
There was a problem hiding this comment.
I can see its just a refactoring, so not really important
bishabosha
reviewed
Oct 20, 2023
|
|
||
| object Loader: | ||
| private[Loaders] case class Root private[Loaders] (pkg: PackageSymbol, rootName: SimpleName): | ||
| def fullName: FullyQualifiedName = | ||
| pkg.fullName.select(rootName) | ||
| override def toString(): String = pkg.displayFullName + "." + rootName.toString |
There was a problem hiding this comment.
this will now print <empty>.Foo rather than Foo
A `PackageFullName` only contains `SimpleName`s. A `SignatureName` contains `SimpleName`s and `ObjectClassName`s.
Also directly use `tpnme.Wildcard` instead of `TypeName(nme.Wildcard)`.
Use alternatives from `tpnme` instead.
So that it makes more sense.
`TypeName`s do not fundamentally wrap `TermName`s anymore. They form their separate hierarchy. They are much simpler than term names: there are only `SimpleTypeName`, `ObjectClassTypeName` and `UniqueTypeName`. All `TypeName`s can be converted to `TermName`s, but only `SignatureNameItem`s can be converted to `ClassTypeName`s.
sjrd
force-pushed
the
redesign-names-2
branch
from
cf1ffd1
to
fc96252
Compare
bishabosha
approved these changes
Oct 20, 2023
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.
No description provided.