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

[David Blaikie via llvm-commits]

On Tue, Jan 9, 2018 at 9:47 AM Robinson, Paul <paul.robinson at sony.com>
wrote:

> ================
> 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*.
>

Oh, I mostly just meant a wrapper around MD5::MD5Result* - either a typedef
(MD5Handle) or class to wrape that. But that's probably not all that
necessary/useful -  just tossing some ideas around in case they sound
good/inspire other, better ideas.

- Dave


> --paulr
>
>
>
> https://reviews.llvm.org/D41846
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180109/13c42609/attachment.html>