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

Multiple declarations of the same method in the same object should be rejected #6035

Closed
vicuna opened this issue Jun 7, 2013 · 6 comments
Closed
Assignees

Comments

@vicuna
Copy link

vicuna commented Jun 7, 2013

Original bug ID: 6035
Reporter: @alainfrisch
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2015-12-11T18:19:41Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.01.0+dev
Category: typing

Bug description

let c = object
method x = 0
method x = 0
end

is currently accepted, and trigger the warning 7 (method x is overridden) on the second occurrence. I propose to consider it an error when a method overrides another method defined in the same object (which is thus useless), even the method is marked with "!".

We have been bitten by not having the compiler complain on a code like:

let c = object
inherit super_class

method! init = ...

...
...

method! init = ...

end

The proposed change is not difficult to implement, but it could potentially break existing (bad) code. We could also create a new warning, but since it will report code which is clearly wrong and trivial to fix, it would make more sense to have an error.

@vicuna
Copy link
Author

vicuna commented Jun 7, 2013

Comment author: @alainfrisch

Commit 13756 in trunk.

@vicuna
Copy link
Author

vicuna commented Jun 7, 2013

Comment author: @alainfrisch

Keeping this open for a few days, in case anyone wants to complain.

@vicuna
Copy link
Author

vicuna commented Jun 12, 2013

Comment author: @alainfrisch

Nobody complained. The new behavior is documented in the manual.

@vicuna
Copy link
Author

vicuna commented Jun 13, 2013

Comment author: dsheets

Personally, I believe this error is a good idea. With that said, this will cause breakage in some libraries with (benign) method duplication. For instance, ocamlnet 3.6.0, 3.6.3, and 3.6.5 have identical 'stat' methods in netpop. See https://github.com/OCamlPro/opam-repository/pull/796 for more info and the OPAM patch.

I'm not really sure what you'll do with this information but gasche told me to report here for your edification. Also, there was some speculation about this sort of problem being caused by a silent merge bug.

Gerd has also been notified via ocamlnet-devel.

@vicuna
Copy link
Author

vicuna commented Jun 13, 2013

Comment author: dsheets

Bah! Clearly that URL should not end with ">" as that's an invalid character for a URI, Mantis regex.

@vicuna
Copy link
Author

vicuna commented Jun 13, 2013

Comment author: @alainfrisch

Thanks for the information. I see it as a good think that the compiler spots the problem, especially since the fix is trivial.

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