[PATCH] D23766: DebugInfo: introduce di_flags_t type for debug info flags
Victor via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 25 12:02:58 PDT 2016
vleschuk added inline comments.
================
Comment at: include/llvm/IR/DebugInfoMetadata.h:49
@@ +48,3 @@
+/// Type capable of holding all DI flags values
+typedef uint32_t di_flags_t;
+
----------------
echristo wrote:
> majnemer wrote:
> > Could we just declare `enum DIFlags : uint32_t` in DIBuilder.h and use that in the DIBuilder interface instead of introducing a new type name?
> +1
If we do that we still need some typedef, otherwise there still can be problems. Imagine code like this:
enum DIFlags: uint32_t { flag1 = (1 << 1), ... , (1 << 17) };
void foo(??? Flags /* some integral constant containing flags combination */);
What should we place instead of "???"? If we use uint32_t explicitly, we lose the abstraction and ability to change the underlying type of enum just in one place in future. We can use std::underlying_type<> but this is too ugly or we need to typedef it. That's why I think we still need to add a new typedef.
https://reviews.llvm.org/D23766
More information about the llvm-commits
mailing list