Skip to content

Name implicit removal #1693

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 2 commits into from
Dec 6, 2012
Merged

Name implicit removal #1693

merged 2 commits into from
Dec 6, 2012

Conversation

paulp
Copy link
Contributor

@paulp paulp commented Dec 2, 2012

Review by @xeno-by

paulp added 2 commits December 1, 2012 09:31
And simplify the name implicits.
These implicits were crutches going back to a much Stringier
time. Of course "with great type safety comes great verbosity"
and no doubt this could be cleaned up significantly further.
At least the underpinnings are consistent now - the only
implicits involving name should be String -> TypeName and
String -> TermName.
@@ -143,7 +143,7 @@ trait GenSymbols {
val reification = reificode(sym)
import reification.{name, binding}
val tree = reification.tree updateAttachment ReifyBindingAttachment(binding)
state.symtab += (sym, name, tree)
state.symtab += (sym, name.toTermName, tree)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably name in the Reification case class should be made TermName instead of just Name. Then the necessity of this change will vanish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a lot of that sort of thing; I went with "eliminate the implicits" as first pass. Followup commits encouraged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Allright, will do after this PR is merged.

@xeno-by
Copy link
Contributor

xeno-by commented Dec 2, 2012

lgtm

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1758/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1758/

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1071/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1071/

adriaanm added a commit that referenced this pull request Dec 6, 2012
@adriaanm adriaanm merged commit d64f99f into scala:master Dec 6, 2012
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.

4 participants