Fix bugzilla 24509: importC cannot handle _stdcall Function Calling …

[aferust]

…Convention with single heading underscore

[dlang-bot]

Thanks for your pull request and interest in making D better, @aferust! 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
✓	24509	normal	importC cannot handle _stdcall Function Calling Convention with single heading underscore

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

[RazvanN7]

How come this worked before adding these guards?

[aferust]

I am surprised that the guard didn't exist in the first place, too.

[RazvanN7]

I think I get it now. It seems that __stdcall is recognized by the compiler irrespective of the platform where you are compiling on, whereas you are adding the alias only for version windows. For more info, check: #13614

[RazvanN7]

cc @WalterBright - is there any reason to hava stdcall being recognized on non-windows platforms?

[aferust]

@RazvanN7 if you request I remove the guard. Without it CI test fail for non-windows systems.

[RazvanN7]

@aferust It seems that @WalterBright has implemented stdcall so that it is recognized by the parser irrespective of the target platform. Now I wonder if this was simply an overlook or if it was intended behavior. Depending on the answer to that question we will know if this PR needs to be implemented in a way that _stdcall is recognized irrespective of the platform or if it should be windows only.

I guess if we want to be consistent, the _stdcall alias should defined for all platforms, but I want confirmation from @WalterBright before doing that.

[ibuclaw]

stdcall is just one of the few ABIs possible with on x86-32.

https://godbolt.org/z/osxa3qhcs

It's not rejected, but rather a warning on x86-64 and other targets. ImportC follows the convention that if it's only warned about, then just accept and ignore.

[thewilsonator]

@aferust the commit message has bugzilla spelled with 3 l instead of two.

[thewilsonator]

the commit message not the PR title (well that too, but the commit message is what the bot uses to auto-close the issue)

[aferust]

the commit message not the PR title (well that too, but the commit message is what the bot uses to auto-close the issue)

Ahh. too bad. I cannot change it without a revert and ... ahh

[aferust]

the commit message not the PR title (well that too, but the commit message is what the bot uses to auto-close the issue)

@thewilsonator what if I change the title of the last commit "test requires a new line at the end of the file"

From a StackOverflow post:
Changing history
If it is the most recent commit, you can simply do this:

git commit --amend
This brings up the editor with the last commit message and lets you edit the message. (You can use -m if you want to wipe out the old message and use a new one.)

Pushing
And then when you push, do this:

git push --force-with-lease

[thewilsonator]

just

git reset HEAD~4 
git add ...
git commit ...
git push --force

[kinke]

FWIW, from https://learn.microsoft.com/en-us/cpp/cpp/stdcall?view=msvc-170:

The __stdcall modifier is Microsoft-specific.
[…]
For compatibility with previous versions, _stdcall is a synonym for __stdcall unless compiler option /Za (Disable language extensions) is specified.

So LGTM, keeping the macro MS-specific, unless DigitalMars C also supports _stdcall. Wrt. tests, I'd only guard the added tests via #ifdef _MSC_VER, keeping the existing tests as they are.

[aferust]

FWIW, from https://learn.microsoft.com/en-us/cpp/cpp/stdcall?view=msvc-170:

The __stdcall modifier is Microsoft-specific.
[…]
For compatibility with previous versions, _stdcall is a synonym for __stdcall unless compiler option /Za (Disable language extensions) is specified.

So LGTM, keeping the macro MS-specific, unless DigitalMars C also supports _stdcall. Wrt. tests, I'd only guard the added tests via #ifdef _MSC_VER, keeping the existing tests as they are.

"unless DigitalMars C also supports _stdcall"
Yes it supports

I tried to compile test22727.c with dmc -c test22727.c without the guard. And it is happy with it. But there is a problem that has nothing to do with my PR. dmc is not happy when __stdcall appears before the return type.: __stdcall int foostdcall(int a) { return a; }

yielding:

__stdcall int foostdcall(int a) { return a; }
            ^
test22727.c(6) : Error: illegal combination of types

As I said, if it is an issue, it existed before my PR. So I will just finalize my PR with:

// https://issues.dlang.org/show_bug.cgi?id=22727


int fooc(int a) { return a; }

__stdcall int foostdcall(int a) { return a; }

int __stdcall foostdcall2(int a) { return a; }

#if _MSC_VER
int _stdcall foostdcall3(int a) { return a; } // test issue 24509
#endif

int __stdcall (*fp1)(int a) = &foostdcall;

int(__stdcall *fp2)(int a) = &foostdcall2;

please note that I don't know if we have any reason to do:

#if defined _MSC_VER || defined __DMC__