Skip to content
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

the scanner is now less eager about deprecations #1763

Closed

Conversation

xeno-by
Copy link
Contributor

@xeno-by xeno-by commented Dec 12, 2012

When healing braces it isn't very useful to report deprecation warnings,
especially since this process is just simple context-free skimming, which
can't know about what positions can accept what identifiers.

When healing braces it isn't very useful to report deprecation warnings,
especially since this process is just simple context-free skimming, which
can't know about what positions can accept what identifiers.
@xeno-by
Copy link
Contributor Author

xeno-by commented Dec 12, 2012

review @paulp

@xeno-by
Copy link
Contributor Author

xeno-by commented Dec 12, 2012

In the last 15 minutes I haven't managed to find a test case that would exhibit a spurious warning. I did manage to trip the warning in my playground branch, but that required changes to the parser.

@scala-jenkins
Copy link

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

@scala-jenkins
Copy link

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

@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/1873/

@scala-jenkins
Copy link

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

@paulp
Copy link
Contributor

paulp commented Dec 13, 2012

I must say this feels over-engineered. Culling all this code lately has made me more conscious than ever of the cost of dragging something like "protected def emitIdentifierDeprecationWarnings" around with us until the end of time. When you can't even demonstrate the behavior without modifying the parser, and the behavior is a pretty minor issue (which can easily be evaded by not running with -deprecation - you don't have to use it full time - why do you want deprecation warnings on code which doesn't compile?) I think this belongs in xeno-by's private home-branch.

@retronym
Copy link
Member

I should note that I often see "macro is now reserved" error when I have a syntax error elsewhere in a file containing macro decls. Is that what we're fixing here? I never managed to reproduce this outside of my SBT build, but will try again.

@xeno-by
Copy link
Contributor Author

xeno-by commented Dec 13, 2012

@retronym Yes, this is exactly what I am talking about.

@paulp Would you change your mind if Jason and/or I conjured a testcase?

@paulp
Copy link
Contributor

paulp commented Dec 15, 2012

@xeno-by I think a test case is necessary but insufficient; can we not find some more generally applicable formulation of the quality being considered than "emitIdentifierDeprecationWarnings" ?

@xeno-by
Copy link
Contributor Author

xeno-by commented Dec 24, 2012

Will reopen against master

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