Do not compute PackageDef members in extractor #4578
Conversation
nicolasstucki
force-pushed
the
add-missing-tasty-implemetations
branch
2 times, most recently
from
8d02417 to
e5ad4af
Compare
nicolasstucki
changed the title
| if (x.symbol.is(core.Flags.JavaDefined)) Nil // FIXME should also support java packages | ||
| else x.symbol.info.decls.iterator.map(FromSymbol.definition).toList | ||
| Some(x.symbol.name.toString, definitions) | ||
| Some((x.symbol.name.toString, FromSymbol.packageDef(x.symbol.owner))) |
There was a problem hiding this comment.
Pretty hard to guess here: the returned PackageDef is the owner. Lacking constructors that's hard to document.
nicolasstucki
force-pushed
the
add-missing-tasty-implemetations
branch
from
e5ad4af to
7378bc2
Compare
nicolasstucki
force-pushed
the
add-missing-tasty-implemetations
branch
from
7378bc2 to
b90ff00
Compare
|
Rebased |
There was a problem hiding this comment.
As far as I can tell this is mostly a refactoring (that optimizes things by making them lazier), which LGTM.
However, this changes PackageDef destruction to not match TastyFormat anymore — since that defines
TopLevelStat = PACKAGE Length Path TopLevelStat*
Stat
I'm fine with this format being inspired by Tasty, but that seems potentially problematic in general.
To ensure we keep a correspondence between this format and tasty, I suspect we should add tests comparing the output from this API with the results of reading pickled Tasty trees, verifying the two are isomorphic. Such a test could be updated to withstand this change, but that would limit more invasive misalignments.
|
The confusion here is that in tasty reflet we call the PackageDef tree PackageClause. The PackageDef in tasty reflect represents the symbol of the package. |
|
@Blaisorblade any other comment or can we merge this one? |
|
Merging.
Now I see what you mean, but I don't understand what the distinction is supposed to represent conceptually. As far as I can tell, |
No description provided.