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

Fix Issue 17373 - traits getOverloads + multiple interface inheritan… #7959

Merged
merged 1 commit into from
Apr 24, 2018

Conversation

RazvanN7
Copy link
Contributor

…ce only see one of the interfaces' overloads

Interface inheritance does not create an OverloadSet for overloaded methods in different inherited interfaces. This caused traits(getOverloads) to report incorrect answers. In order to fix this I added a test so that if the symbol passed to traits is a interface declaration, all the inherited interfaces are checked to see if they add a new overload. In order to avoid counting functions with the same signature, but located in different inherited interfaces, I am using a hashtable which stores the function mangled signature.

@dlang-bot
Copy link
Contributor

dlang-bot commented Feb 26, 2018

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
17373 normal traits getOverloads + multiple interface inheritance only see one of the interfaces' overloads

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#7959"

@RazvanN7 RazvanN7 force-pushed the Issue_17373 branch 4 times, most recently from 7115c5d to e5dc60e Compare February 26, 2018 15:13
src/dmd/traits.d Outdated
auto funcType = fd.type.toChars();
auto len = strlen(funcType);
//printf("%s - %s\n", fd.toChars, fd.type.toChars());
if (!funcTypeHash.lookup(funcType, len))
Copy link
Member

Choose a reason for hiding this comment

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

  1. Why don't you use D strings and a normal hashmap?
  2. toChars is a bit of an anti-pattern. Why don't you just insert the type directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type is a class that does not have opEquals and toHash defined.

@JinShil
Copy link
Contributor

JinShil commented Mar 8, 2018

@RazvanN7 Please look into the Jenkins failures.

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Apr 20, 2018

@JinShil Jenkins is green now.

Later edit : and so are the other tests

@wilzbach
Copy link
Member

wilzbach commented Jul 5, 2018

This PR broke Vibe.d's complex InterfaceProxy generation and thus breaks Vibe.d in general.

vibe-d/vibe.d#2181

It's a shame no one noticed this before 2.081 (Vibe.d was disabled on the project tester due to issues with the dub registry at the time of this PR).

@wilzbach
Copy link
Member

wilzbach commented Jul 5, 2018

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

Successfully merging this pull request may close these issues.

4 participants