Skip to content

fix issue #20425#22218

Merged
thewilsonator merged 4 commits intodlang:masterfrom
Emmankoko:tenary
Dec 13, 2025
Merged

fix issue #20425#22218
thewilsonator merged 4 commits intodlang:masterfrom
Emmankoko:tenary

Conversation

@Emmankoko
Copy link
Copy Markdown
Contributor

@Emmankoko Emmankoko commented Dec 11, 2025

in the type merging for tenary operators, no handler exists yet for real to complex type conversion.
complex in C does nor follow Tcomplex family but a _Complex struct with floating types.

This PR handles it!

now we are able to use complex in tenary operators.

Windows disabled in testing since windows does not implement the C99 complex system.

@dkorpel @thewilsonator

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request and interest in making D better, @Emmankoko! 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 coverage diff by visiting the details link of the codecov check)
  • 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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

Testing this PR locally

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

dub run digger -- build "master + dmd#22218"

Comment thread compiler/src/dmd/dcast.d
@ibuclaw
Copy link
Copy Markdown
Member

ibuclaw commented Dec 12, 2025

void foo() {
    double *a;
    double _Complex *b;

It's preferable not to depend on headers coming from the system in tests.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

void foo() {
    double *a;
    double _Complex *b;

It's preferable not to depend on headers coming from the system in tests.

Right.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

void foo() {
    double *a;
    double _Complex *b;

It's preferable not to depend on headers coming from the system in tests.

Right.

Illegal instruction on only LDC on windows x64.

@ibuclaw
Copy link
Copy Markdown
Member

ibuclaw commented Dec 12, 2025

complex in C does nor follow Tcomplex family but a _Complex struct with floating types.

Where does this struct come from? Complex is a fundamental type, anything else is incompatible with gdc/ldc backends.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

Emmankoko commented Dec 12, 2025

complex in C does nor follow Tcomplex family but a _Complex struct with floating types.

Where does this struct come from? Complex is a fundamental type, anything else is incompatible with gdc/ldc backends.

in importC, complex never falls under Tcomplex. that was why it never worked.

and it is actually compatible with ldc and gdc on ubuntu and Mac, except only windows.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

Emmankoko commented Dec 12, 2025

complex in C does nor follow Tcomplex family but a _Complex struct with floating types.

Where does this struct come from? Complex is a fundamental type, anything else is incompatible with gdc/ldc backends.

Error: incompatible types for (*(b + cast(long)0 * 16L)) : (*(a + cast(long)0 * 8L)): _Complex!double and double

this is the error message emitted when trying to use it in importC.

I did a naive writeln on the type of the complex variables and it was always printing Tstruct.

@ibuclaw
Copy link
Copy Markdown
Member

ibuclaw commented Dec 12, 2025

in importC, complex never falls under Tcomplex. that was why it never worked.

I don't see any other mapping except to the fundamental type.

case TKW.xcomplex | TKW.xfloat: t = AST.Type.tcomplex32; break;

@Emmankoko
Copy link
Copy Markdown
Contributor Author

in importC, complex never falls under Tcomplex. that was why it never worked.

I don't see any other mapping except to the fundamental type.

case TKW.xcomplex | TKW.xfloat: t = AST.Type.tcomplex32; break;

in importC, complex never falls under Tcomplex. that was why it never worked.

I don't see any other mapping except to the fundamental type.

case TKW.xcomplex | TKW.xfloat: t = AST.Type.tcomplex32; break;

oh alright!

so I was using the Tcomplex. let me rewrite with tcomplex and see.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

Emmankoko commented Dec 12, 2025

I don't see any other mapping except to the fundamental type.

case TKW.xcomplex | TKW.xfloat: t = AST.Type.tcomplex32; break;

I think I have found something worth looking at.
D was supposed to pass complex in C as a fundamental type as it appears in the parser. but it doesn't appear as fundamental during semantics. here's why.
this is typesem

    Type visitComplex(TypeBasic t)
    {
        import std.stdio;
        writeln(t);
        if (!sc.inCfile) // this is where everything breaks.
            return visitType(t);

        auto tc = getComplexLibraryType(loc, sc, t.ty);
        if (tc.ty == Terror)
            return tc;
        return tc.addMod(t.mod).merge();
    }

printing the type out gives me cdouble for C complex which is fundamental. but then we go on to say, if we are not in a C file, return visitType. and hence C files will getcomplexlibrary type. hence the reason why the C complex is a _Complex struct. I think the if condition should rather be if we are in a C file, go to visitType. Tested locally and that's the only needed change to make it work! I hope it doesn't break any other tests!

@thewilsonator thewilsonator merged commit 244ee63 into dlang:master Dec 13, 2025
42 checks passed
@ibuclaw
Copy link
Copy Markdown
Member

ibuclaw commented Dec 13, 2025

Then Complex is broken on C then. And that should be fixed instead. There is no reason for it to be swapped with a struct. That's not the C ABI type, and passing it across C<->D boundaries won't work.

@Emmankoko
Copy link
Copy Markdown
Contributor Author

Then Complex is broken on C then. And that should be fixed instead. There is no reason for it to be swapped with a struct. That's not the C ABI type, and passing it across C<->D boundaries won't work.

Then Complex is broken on C then. And that should be fixed instead. There is no reason for it to be swapped with a struct. That's not the C ABI type, and passing it across C<->D boundaries won't work.

Yep! it would require a heavy change which I am hoping to look into. even all the tests that are written are written to obey the struct interface.

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