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 19661 - DMD 2.084.0 SIGSEGV in std.traits.isFunction #9652

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

RazvanN7
Copy link
Contributor

module test;

public immutable bool testModule = testFunctionMembers!"test";    // line 3

public void testFunctionMembers(string module_)() {
    import std.traits : isFunction;
    mixin(`import dmodule = ` ~ module_ ~ `;`);
    foreach(member; __traits(allMembers, dmodule)) {
        const bool isfunc = isFunction!(__traits(getMember, dmodule, member));
    }
}

Fixing the regression for this code led to outputting the error message : "Error: cannot implicitly convert expression of type void to immutable(bool)" ; the line number is missing, but it refers to line 3.
I wanted to fix that also by adding the line number to the error, but noticed that this code compiles fine:

immutable bool b = void;

In light of this example, I modified the code so that when an ExpInitializer that is evaluated to a VoidExpression is encountered, a VoidInitializer is returned. This made the code in the original bug post compile successfully, which I believe is the right fix.

@dlang-bot
Copy link
Contributor

dlang-bot commented Apr 19, 2019

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
19661 regression DMD 2.084.0 SIGSEGV in std.traits.isFunction

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#9652"

@wilzbach
Copy link
Member

stable?

@RazvanN7
Copy link
Contributor Author

Tried to, but I get conflicts.

@ghost
Copy link

ghost commented Apr 19, 2019

In light of this example, I modified the code so that when an ExpInitializer that is evaluated to a VoidExpression is encountered, a VoidInitializer is returned. This made the code in the original bug post compile successfully, which I believe is the right fix.

No error message is not good at all. Also this is not like a void init, the test case is more like

immutable bool b = test();
void test(){}

which outputs the impl. conv error I managed to get in the previous fix attempt.

Geod24
Geod24 previously requested changes Apr 20, 2019
Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

void initialization != initialization with an expression of type void

test/compilable/test19661.d Outdated Show resolved Hide resolved
@RazvanN7
Copy link
Contributor Author

@Geod24 @Basile-z I updated the patch. Now if any expressions are evaluated to void after ctfe an error will be issued.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I see a small progress compared to my fix, i.e the line numbers, so this is good to my eyes

@RazvanN7
Copy link
Contributor Author

ping @Geod24

@thewilsonator thewilsonator dismissed Geod24’s stale review April 24, 2019 07:53

void init problem fixed.

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Apr 24, 2019

The Out of memory error is unrelated to this PR, although this should be a sign that dmd is using waaaaaaay too much memory; I just compiled the vibe.d unittests and my computer froze for 1-2 minutes; I have 4GB, I know it's not that much, but still...

@RazvanN7 RazvanN7 closed this Apr 24, 2019
@RazvanN7 RazvanN7 reopened this Apr 24, 2019
@dlang-bot dlang-bot merged commit 9487191 into dlang:master Apr 24, 2019
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.

5 participants