Skip to content

Avoid loading compilation units twice #3461

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

Merged
merged 1 commit into from
Nov 15, 2017

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki force-pushed the fix-case-class-from-tasty branch from 81202ad to efbe537 Compare November 12, 2017 13:30
@nicolasstucki nicolasstucki force-pushed the fix-case-class-from-tasty branch 2 times, most recently from 37587f3 to a05eda7 Compare November 13, 2017 17:28
@nicolasstucki nicolasstucki changed the title Fix case class from tasty Avoid loading compilation units twice Nov 13, 2017
@nicolasstucki nicolasstucki force-pushed the fix-case-class-from-tasty branch from a05eda7 to 957694c Compare November 13, 2017 17:31
}
compilationUnits
// Try to load it from the class, or else if the class is not present try to load it from the object with that name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment could be improved. I would say something like:

The TASTY section in a/b/C.class may either contain a class a.b.C, an object a.b.C, or both.
We first try to load the class and fallback to loading the object if the class doesn't exist.
Note that if both the class and the object are present, then loading the class will also load the object,
this is why we use orElse here, otherwise we could load the object twice!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
compilationUnits
// Try to load it from the class, or else if the class is not present try to load it from the object with that name
compilationUnit(className).orElse(compilationUnit(className.moduleClassName)).toList
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return type should be an Option and not a List now I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. It was only IntelliJ that complained about the Option in the flat map.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM.

For some class name foo.Foo we need to try to load the tasty from the its class.
If the class is not definded, we try load it from the obeject.

Previously, if both the class and the object extists we were loading the compilation unit twice.
Which would duplicate symbols and then fail with duplicated symbol definitions.
@nicolasstucki nicolasstucki force-pushed the fix-case-class-from-tasty branch from 957694c to 723993e Compare November 15, 2017 06:41
@nicolasstucki nicolasstucki merged commit 7759e00 into scala:master Nov 15, 2017
@allanrenucci allanrenucci deleted the fix-case-class-from-tasty branch December 14, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants