Skip to content

Fix 23767 - ImportC: ternary with null constant has wrong pointer type#14980

Merged
RazvanN7 merged 1 commit intodlang:stablefrom
dkorpel:importc-ternary-null
Mar 13, 2023
Merged

Fix 23767 - ImportC: ternary with null constant has wrong pointer type#14980
RazvanN7 merged 1 commit intodlang:stablefrom
dkorpel:importc-ternary-null

Conversation

@dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Mar 10, 2023

No description provided.

@dkorpel dkorpel added the Feature:ImportC Pertaining to ImportC support label Mar 10, 2023
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! 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

Auto-close Bugzilla Severity Description
23767 normal ImportC: ternary with null constant has wrong pointer type

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 "stable + dmd#14980"

@dkorpel dkorpel marked this pull request as ready for review March 10, 2023 16:25
// https://issues.dlang.org/show_bug.cgi?id=23767
// `cast(void*) 0` should be treated as `null` so the ternary expression
// gets the pointer type of the other branch
if (sc.flags & SCOPE.Cfile)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to change the way the types are merged rather than rewriting them? Not sure if you have the scope available but you could always just have a different function for finding the common types of C.

The C behaviour (in a sense) was almost what D used to do before I fixed it a while back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

void* + T* merges to void* in general, both in C and D, only in C there is special treatment when the void* is the NULL constant

Copy link
Member

Choose a reason for hiding this comment

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

Previously it went to T* until I fixed it (I think ldc 1.30 still has this bug)

Could potentially use TypeNull

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am rewriting (void*)0 to use TypeNull

@RazvanN7 RazvanN7 merged commit fc3517e into dlang:stable Mar 13, 2023
@dkorpel dkorpel deleted the importc-ternary-null branch March 13, 2023 12:01
@ibuclaw ibuclaw mentioned this pull request Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:ImportC Pertaining to ImportC support Severity:Bug Fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants