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

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/docs/LangRef.rst:1589
+       argument is invalidated, even if the function returns the same address.
+     * "free": the function frees the block of memory specified by ``allocptr``
+     * "uninitialized": Any newly-allocated memory (either a new block from 
----------------
Missing period at end of sentence.


================
Comment at: llvm/include/llvm/Analysis/MemoryBuiltins.h:111
+/// Gets the AllocKind information for a function.
+AllocFnKind getAllocKind(const CallBase *CB, const TargetLibraryInfo *TLI);
+
----------------
You don't define this function -- or at least not in this patch.


================
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 |
----------------
durin42 wrote:
> 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.
Oh yeah, BitmaskEnum is doing some serious magic here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123088