[PATCH] D41846: [DWARFv5] MC support for MD5 checksum

Robinson, Paul via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 9 09:47:29 PST 2018


================
Comment at: llvm/include/llvm/MC/MCDwarf.h:56
+  /// The MD5 checksum, if there is one.
+  MD5::MD5Result *Checksum = nullptr;
 };
----------------
aprantl wrote:
> Optional<MD5::MD5Result>?
probinson:
This is a non-owning pointer to data allocated in MCContext.  Does that change your opinion?  (Or Blaikie's?)
dblaikie:
Roughly sounds OK to me, with some comments about that (and/or maybe a typedef/opaque handle type or something - and document the ownership where that type is defined)

It's a nested class, the only way I know to make it opaque is to un-nest it so it can be forward-declared (affecting not a huge number of other places).  Or make this one a void* and cast it once we actually want to do something with it, but this data gets passed through a bunch of APIs and using void* seems bad for self-documentation purposes.
So, I'm willing to un-nest the class as a pre-req refactor, or do something else you suggest, but I'd rather not do void*.
--paulr

https://reviews.llvm.org/D41846


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180109/a3d0fcb2/attachment.html>


More information about the llvm-commits mailing list