[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 9 05:11:54 PDT 2022


aaron.ballman added a comment.

In D131307#3709494 <https://reviews.llvm.org/D131307#3709494>, @JonChesterfield wrote:

> Did some digging here. The function hsa_agent_get_info takes an argument of type hsa_agent_info_t, which has declared values in the range [0 24]. The implementation of that (in amd_gpu_agent fwiw) casts that argument to a size_t and then switches on it, checking those declared values and a bunch of extensions. This is used to provide vendor extensions through a vendor-agnostic interface.
>
> This seems to be a case where C and C++ have diverged.

Yes, they've always been divergent in this area, it's only thanks to the magic of constexpr that anyone really notices now.

> As far as I can tell, C thinks an enum is an int, and anything that fits in an int can be stored in one and retrieved later.

It's a bit more complicated than that, unfortunately. But this is kind of the gist of it.

> C23 lets one specify the underlying type. C++ evidently thinks the value stored must be within [min max] of the declaration, which is at least more flexible than requiring it be one in the declaration.

Correct, C++ uses the minimum-sized bit-field that can hold all of the values of the enumeration.

> So I think the fix here is to change hsa_agent_info_t to include `HSA_AGENT_INFO_UNUSED_INCREASE_RANGE_OF_TYPE = INT32_MAX` so the vendor extensions remain accessible. It's a header that is usable from C and C++ so it needs to do something conforming to both. Does that sound right?

Yes, that's how I'd solve the problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131307/new/

https://reviews.llvm.org/D131307



More information about the cfe-commits mailing list