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

Dynamic is broken. #4536

Closed
scabug opened this issue May 2, 2011 · 11 comments
Closed

Dynamic is broken. #4536

scabug opened this issue May 2, 2011 · 11 comments

Comments

@scabug
Copy link

scabug commented May 2, 2011

applyDynamic goes into an infinite loop, calling "applyDynamic" on itself. For instance, try the pastie at http://pastie.org/pastes/1469174/text . But my guess is anything will be the same.

@scabug
Copy link
Author

scabug commented May 2, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4536?orig=1
Reporter: @paulp
Attachments:

@scabug
Copy link
Author

scabug commented May 3, 2011

@dragos said:
I guess Martin earned a new ticket.

@scabug
Copy link
Author

scabug commented Aug 3, 2011

@dcsobral said:
Let's be clear about what is happening here: there's no "applyDynamic" defined on the class in the example. So, when the compiler sees that a method does not exist, it tries to find the applyDynamic method to handle it. Since it doesn't exist, it tries to find the applyDynamic method to handle the applyDynamic method, and on and on.

The compiler should not do that. Furthermore, IMHO, any non-abstract class extending Dynamic should implement applyDynamic or an error should be returned.

@scabug
Copy link
Author

scabug commented Sep 29, 2011

@jsalvata said:
The attached patch implements a solution by:

(1) Preventing the compiler running away by stopping it dynamically applying a call to "applyDynamic". This is sufficient to stop this bug proper, but puts the weight on the client, as the error only shows up at the point of call.

(2) Forbidding any subclassing of scala.Dynamic without declaring an applyDynamic member -- even traits or abstract classes. Forbidding it only for concrete classes is not sufficient because intermediate abstract classes would cause the error in (1) to come up, which could be pretty mysterious if it's a library class and the user doesn't even know about Dynamic.

Note that with change (2), the only way to run into (1) will be to call a dynamic method on scala.Dynamic directly or some class file compiled with an older version of the compiler.

Please review the patch carefully both in concept and in implementation, as I am not a computer language theorist and it's my first attempt at changing the scala compiler. Tomorrow I'll try to find out how to add test cases and create some for this change.

@scabug
Copy link
Author

scabug commented Sep 29, 2011

@jsalvata said (edited on Sep 29, 2011 6:51:21 AM UTC):
I've realized that condition (2) can be applied without loss of generality as there is no reason to mix the Dynamic trait into the hierarchy until applyDynamic is first declared, possibly abstract.

@scabug
Copy link
Author

scabug commented Sep 29, 2011

@jsalvata said:
Updated patch with better error reporting + some tests.

@scabug
Copy link
Author

scabug commented Oct 1, 2011

@jsalvata said (edited on Oct 2, 2011 5:19:50 PM UTC):
Following recommendation from Paul Phillips at scala-internals (thanks, Paul!), I've proposed the patch through github: scala/scala#98

@scabug
Copy link
Author

scabug commented May 3, 2012

@paulp said:
I don't know what happened, but that link to github is to an xml equality patch. Does the patch still exist somewhere?

@scabug
Copy link
Author

scabug commented May 3, 2012

@paulp said:
It looks like Alex closed this yesterday, but it must have been a mistake.

@scabug
Copy link
Author

scabug commented May 3, 2012

@jsalvata said:
FWIW, the above-mentioned patch is now in this URL: Apparently repositories were renamed: scala/legacy-svn-scala#98

"U" stands for "Universal", but apparently the time dimension is not included.

@scabug
Copy link
Author

scabug commented May 4, 2012

@paulp said:
As of M3, this is no longer an issue; however you can't call dynamic methods on anything unless it can find the implementation, i.e. you can't call anything on Dynamic. I don't know if this is intentional - I started a thread on scala-language. For the moment I will close this as fixed, but will reopen if events say otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants