[PATCH] D123088: attributes: introduce allockind attr for describing allocator fn behavior

[Augie Fackler via Phabricator via llvm-commits]

durin42 marked 8 inline comments as done.
durin42 added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:2091
+        ((K & (AllocFnKind::Alloc | AllocFnKind::Realloc |
+               AllocFnKind::Free)) == AllocFnKind::Unknown))
+      CheckFailed(
----------------
nikic wrote:
> This enumeration of different combinations seems a bit awkward, maybe we can do something along these lines instead?
> ```
> AllocFnKind Type = K & (AllocFnKind::Alloc | AllocFnKind::Realloc | AllocFnKind::Free);
> if (!is_contained(Type, {AllocFnKind::Alloc, AllocFnKind::Realloc, AllocFnKind::Free))
>   CheckFailed(...);
> ```
Very nice, I like it. Thanks!


================
Comment at: llvm/lib/IR/Verifier.cpp:2094
+          "'allockind()' requires exactly one of alloc, realloc, and free");
+    if (((K & AllocFnKind::Free) != AllocFnKind::Unknown) &&
+        ((K & (AllocFnKind::Uninitialized | AllocFnKind::Zeroed |
----------------
nikic wrote:
> The `!= AllocFnKind::Unknown` checks here aren't really necessary, and imho don't really add to readability either. `(K & AllocFnKind::Free)` should be directly usable as a boolean. (Though I don't care strongly about this.)
It's not: `error: could not convert ‘llvm::BitmaskEnumDetail::operator&<llvm::AllocFnKind>(Kind, llvm::AllocFnKind::Free)’ from ‘llvm::AllocFnKind’ to ‘bool’`

I think the BitmaskEnum stuff is getting in the way, and it's beyond my C++-fu to figure out how to allow them to be convertible to bool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123088