Skip to content

Fix call convention warnings for AArch64 #10919

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

Merged
merged 1 commit into from
Apr 13, 2025
Merged

Conversation

GulinSS
Copy link
Contributor

@GulinSS GulinSS commented Apr 10, 2025

These warnings stop CI process at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13856 due of -Werror:

libraries/Cabal/Cabal/src/Distribution/Compat/Time.hs:103:1: warning: [GHC-01245] [-Wunsupported-calling-conventions]
    • the 'stdcall' calling convention is unsupported on this platform,
      treating as ccall
    • When checking declaration:
        foreign import stdcall safe "windows.h GetFileAttributesExW" c_getFileAttributesEx
          :: LPCTSTR -> Int32 -> LPVOID -> Prelude.IO BOOL
    |
103 | foreign import CALLCONV "windows.h GetFileAttributesExW"
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

TBH does that even belong there any more? I thought Win32 used ccall exclusively and stdcall was deprecated, and since ghc doesn't support pre-Win32 stdcall should simply be removed?

@GulinSS
Copy link
Contributor Author

GulinSS commented Apr 11, 2025

stdcall should simply be removed?

My opinion is the deprecated stuff should be removed every major version. If something called deprecated at current major version, it should be removed over one major version. So, if we have quorum to remove stdcall from Cabal I could do that during this PR (but I would prefer to do this in a separate PR).

@mpickering
Copy link
Collaborator

@GulinSS

commit d2a83302a267ccd8a3eae1af7f2225d91477dc0a
Author: Cheng Shao <[email protected]>
Date:   Fri May 31 17:05:26 2024 +0000

    testsuite: adapt the testsuite for stdcall removal
    
    This patch adjusts test cases to handle the stdcall removal:
    
    - Some stdcall usages are replaced with ccall since stdcall doesn't
      make sense anymore.
    - We also preserve some stdcall usages, and check in the expected
      warning messages to ensure GHC always warn about stdcall usages
      (-Wunsupported-calling-conventions) as expected.
    - Error code testsuite coverage is slightly improved,
      -Wunsupported-calling-conventions is now tested.
    - Obsolete code paths related to i386 windows are also removed.

commit 65fe75a42c0bb6e145916e4dc4693a94d212850e
Author: Cheng Shao <[email protected]>
Date:   Fri May 31 17:05:07 2024 +0000

    libraries/utils: remove stdcall related legacy logic
    
    This commit removes stdcall related legacy logic in libraries and
    utils. ccall should be used uniformly for all supported windows hosts
    from now on.

These commits removed things to do with stdcall from GHC before. However, this patch does seem fine to merge as-is to me.

@mpickering mpickering self-requested a review April 11, 2025 11:18
@mpickering mpickering added merge me Tell Mergify Bot to merge and removed attention: needs-review labels Apr 11, 2025
@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Apr 11, 2025
Copy link
Contributor

mergify bot commented Apr 13, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify mergify bot merged commit f2f9250 into haskell:master Apr 13, 2025
53 checks passed
@GulinSS GulinSS deleted the wip/T24603 branch April 13, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants