Skip to content

Add requiredXYZ symbols to TASTy reflect context #7903

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

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Jan 6, 2020
@nicolasstucki nicolasstucki marked this pull request as ready for review January 6, 2020 17:45
Copy link
Contributor

@liufengyun liufengyun 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

def Context_requiredClass(self: Context)(path: String): Symbol

/** Get module symbol if module is either defined in current compilation run or present on classpath. */
def Context_requiredModule(self: Context)(path: String): Symbol
Copy link
Contributor

Choose a reason for hiding this comment

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

The meta-programmer may not know what's a module. Maybe rename to requiredObject?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better to keep names consistent. It will be simpler for documentation. We also have moduleClass and companionModule that use the same naming scheme.

Copy link
Contributor

Choose a reason for hiding this comment

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

I totally agree, for advanced meta-programming, keeping the concepts closer to the compiler will help both meta-programmers who want to get into the compiler and maintenance of the framework.

@nicolasstucki nicolasstucki merged commit a8ea206 into scala:master Jan 7, 2020
@nicolasstucki nicolasstucki deleted the add-required-symbols-to-tasty-reflect branch January 7, 2020 09:39
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