The Wayback Machine - https://web.archive.org/web/20200908023820/https://github.com/scala-js/scala-js/pull/4124
Skip to content
Sign in Sign up

/ scala-js

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split TopLevelExportDefs away from LinkedClass #4124

Merged
merged 2 commits into from Jul 16, 2020
Merged

Split TopLevelExportDefs away from LinkedClass #4124

merged 2 commits into from Jul 16, 2020

Conversation

@gzm0
Copy link
Contributor

gzm0 commented Jul 15, 2020

Side-effects for TopLevelMethodExportDefs in IRChecker:

  • Allow them on all class kinds.
  • Allow names containing __ for non JS classes.
  • Disallow references to JS class captures (this was a bug).
@gzm0 gzm0 requested a review from sjrd Jul 15, 2020
@gzm0
Copy link
Contributor Author

gzm0 commented Jul 15, 2020

Split away from #4111. To be merged now, or not :)
@sjrd
sjrd approved these changes Jul 15, 2020
View changes
Copy link
Member

sjrd left a comment

Let's merge this now.
@sjrd
sjrd reviewed Jul 15, 2020
View changes
linker/shared/src/main/scala/org/scalajs/linker/analyzer/Infos.scala Outdated
@@ -42,8 +43,6 @@ object Infos {
val methods: List[MethodInfo],
val jsNativeMembers: Map[MethodName, JSNativeLoadSpec],
val exportedMembers: List[ReachabilityInfo],

This comment has been minimized.

Sign in to view
@sjrd

sjrd Jul 15, 2020

Member

The CI says:

Suggested change
val exportedMembers: List[ReachabilityInfo],
val exportedMembers: List[ReachabilityInfo]
@gzm0 gzm0 force-pushed the gzm0:split-top-level-exports branch from 648910e to 770c20b Jul 15, 2020
@sjrd
sjrd approved these changes Jul 15, 2020
View changes
@gzm0 gzm0 force-pushed the gzm0:split-top-level-exports branch from 770c20b to 5f82662 Jul 15, 2020
@sjrd
sjrd reviewed Jul 16, 2020
View changes
linker/shared/src/main/scala/org/scalajs/linker/frontend/IRLoader.scala Outdated
def loadClassDefAndVersion(irFile: IRFileImpl)(
implicit ec: ExecutionContext): Future[(ClassDef, Option[String])] = {
update(irFile).map(s => (s._1, version))
}

private def update(irFile: IRFileImpl)(
implicit ec: ExecutionContext): Future[(ClassDef, Infos.ClassInfo)] = synchronized {
implicit ec: ExecutionContext): Future[(ClassDef, Infos.ClassInfo, List[Infos.TopLevelExportInfo])] = synchronized {

This comment has been minimized.

Sign in to view
@sjrd

sjrd Jul 16, 2020

Member

The CI says that this line is too long (more than 120 characters). Perhaps move the synchronized inside?

This comment has been minimized.

Sign in to view
@gzm0

gzm0 Jul 16, 2020

Author Contributor

I took the chance to simplify this a bit. PTAL
gzm0 added 2 commits May 11, 2020
@gzm0
Split TopLevelExportDefs away from LinkedClass
8d10150 
This will help with organizing modules.

Side-effects for TopLevelMethodExportDefs in IRChecker:
- Allow them on all class kinds.
- Allow names containing `__` for non JS classes.
- Disallow references to JS class captures (this was a bug).
@gzm0
Use ClsName matcher instead of name constants
Loading status checks…
9616e5e
@gzm0 gzm0 force-pushed the gzm0:split-top-level-exports branch from 5f82662 to 9616e5e Jul 16, 2020
@sjrd
sjrd approved these changes Jul 16, 2020
View changes
@gzm0 gzm0 merged commit 430f915 into scala-js:master Jul 16, 2020
3 checks passed
3 checks passed
check
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@gzm0 gzm0 deleted the gzm0:split-top-level-exports branch Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjrd sjrd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@gzm0 @sjrd
You can’t perform that action at this time.
Morty Proxy This is a proxified and sanitized view of the page, visit original site.