Skip to content
Snippets Groups Projects

include: Use GCC-style attributes and builtins on Clang MSVC target.

Merged Jacek Caban requested to merge jacek/vkd3d:clang_attr into master
1 unresolved thread

Merge request reports

Merge request pipeline #18595 skipped

Merge request pipeline skipped for 616bf939

Merged by Alexandre JulliardAlexandre Julliard 1 year ago (Nov 20, 2023 9:33pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
75 75 return (addr + (alignment - 1)) & ~(alignment - 1);
76 76 }
77 77
78 #ifdef __GNUC__
78 #ifdef __MINGW_PRINTF_FORMAT
  • I'm not sure if it's worth to move out the whole handling of VKD3D_PRINTF_FUNC here, instead of leaving it all where it was, and just change the #ifdef __GNUC__ into #if defined(__GNUC__) || defined(__clang__) here.

    I guess the only case where there's a behavioural difference, is if one would have __MINGW_PRINTF_FORMAT defined but neither __GNUC__ or __clang__ defined, i.e. be compiling with actual MSVC, but with mingw headers. That's not really a real case though - so it doesn't really matter, but I was just curious if there's any other advantage, to just keeping the code as it was and just change the one ifdef?

  • Author Developer

    vkd3d suffers from the same problem as Wine before PE conversion: things like DWORD need to be ints instead of longs when building for Unix targets. It actually assumes ints in debug formats, which leads to various warnings. I didn't want to extend the problem to MSVC targets.

    However, I took a deeper look at Wine build system and came out with: wine!4423 (closed). It's more comprehensive: aligns Clang builds with GCC ones, avoids warnings in other imported libraries as well and fixes both mingw and msvc Clang targets.

    I will change this MR as you suggested, thanks!

  • Jacek Caban changed this line in version 2 of the diff

    changed this line in version 2 of the diff

  • Oh, right - I failed to set that your original version didn’t set any format attributes in clang/msvc builds, while my suggestion does. How could I overlook that? I guess I didn’t consider that you intentionally wanted to omit the argument from a configuration where it could be supported. Anyway, avoiding such unnecessary warnings probably is good in any case.

  • Author Developer

    Yes, fixing it in standalone vkd3d would be nice as well. I'm not sure which solution would be preferred, vkd3d already uses macros like PRIx64, so I their usage could be extended. An easy solution would be disabling those warnings in condigure or skipping format attribute on PE targets in the header and depending on Unix builds to validate formats. Anyway, it's a separated issue, this MR just brings msvc target on pair with mingw.

  • Agreed that it would be nice to fix the -Wformat warnings. In the CI we explicitly disable them for the moment.

  • Please register or sign in to reply
  • Jacek Caban added 1 commit

    added 1 commit

    • a7dc0685 - include: Use GCC-style attributes and builtins on Clang MSVC target.

    Compare with previous version

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • added 9 commits

    • a7dc0685...078cf6a2 - 8 commits from branch wine:master
    • 616bf939 - include: Use GCC-style attributes and builtins on Clang MSVC target.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading