Skip to content
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

Cast a macro argument to int to fix a C++20 error. #958

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

Cons-Cat
Copy link
Contributor

@Cons-Cat Cons-Cat commented Nov 13, 2024

C++ 20 errors for me on several expansions of the KHR_DFDSETVAL macro because it attempts to bitwise-and two different types of enums. For example:

/home/conscat/vulkan-experiments/KTX-Software/lib/basis_encode.cpp:256:5: error: invalid bitwise operation between different enumeration types ('_khr_df_vendorid_e' and '_khr_df_mask_e')
  256 |     KHR_DFDSETVAL(nbdb, VENDORID, KHR_DF_VENDORID_KHRONOS);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/conscat/vulkan-experiments/KTX-Software/include/KHR/khr_df.h:109:13: note: expanded from macro 'KHR_DFDSETVAL'
  109 |     (((val) & (KHR_DF_MASK_ ## X)) << (KHR_DF_SHIFT_ ## X)))
      |       ~~~~~ ^ ~~~~~~~~~~~~~~~~~~~

This PR simply casts the value to an int inside of the macro.

@CLAassistant
Copy link

CLAassistant commented Nov 13, 2024

CLA assistant check
All committers have signed the CLA.

@MarkCallow
Copy link
Collaborator

CI passed a few days ago using clang 20.0.0git (https:/github.com/llvm/llvm-project d6344c1cd0d099f8d99ee320f33fc9254dbe8288. Are you using a newer version than this, @Cons-Cat.

@Cons-Cat
Copy link
Contributor Author

My version was 2 weeks old.

@Cons-Cat Cons-Cat closed this Nov 15, 2024
@Cons-Cat
Copy link
Contributor Author

Cons-Cat commented Nov 21, 2024

The issue was not due to my version of Clang, it was due to my CMAKE_CXX_FLAGS containing -std=gnu++26. Apparently C++20 deprecated implicit enum conversions in this context. Arguably I shouldn't have had that flag there, but this is the only error I have that prevents building the library this way.

@Cons-Cat Cons-Cat reopened this Nov 21, 2024
@Cons-Cat Cons-Cat changed the title Cast a macro argument to int to fix a Clang error. Cast a macro argument to int to fix a C++20 error. Nov 21, 2024
@MarkCallow
Copy link
Collaborator

@Cons-Cat Thank you for clearing up the confusion of c++20 vs clang v20.

I am looking into the reuse failure. It clearly has nothing to do with the change you have made. I just tried the latest reuse locally and it passes so I don't have any idea of the cause at present. Please confirm your branch contains the file REUSE.toml.

@MarkCallow
Copy link
Collaborator

I re-ran the reuse job and it passed. I have no idea what caused an apparently temporary issue before.

@MarkCallow MarkCallow merged commit 60c3689 into KhronosGroup:main Nov 28, 2024
17 checks passed
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.

3 participants