[LLVMdev] object construction patterns and unique_ptr

[Jeffrey Yasskin]

On Jun 18, 2012 1:24 PM, "Nick Kledzik" <kledzik at apple.com> wrote:
>
> On Jun 16, 2012, at 3:51 PM, Chris Lattner wrote:
> > On Jun 15, 2012, at 3:48 PM, Nick Kledzik wrote:
> >
> >> In lld we have a Reader class which is a factory that takes .o file(s)
and produces an in memory lld::File object(s).  But in doing so, there
could be I/O errors (file not readable) or file may be malformed.  We are
also using C++11 in lld, so we use std::unique_ptr for managing object
ownership.
> >>
> >> The Reader class currently has an interface that can be simplified
down to:
> >>
> >>   virtual error_code readFile(StringRef path,
std::unique_ptr<lld::File> &result);
> >>
> >> But this interface has become awkward to use. There are two "return"
values.  This method is a unique_ptr "source" but the use of a by-refernce
parameter means you have to start with an uninitialized unique_ptr and the
readFile() has to std::move() into it.   unique_ptr is much nicer to use as
a return value.
> >
> > Not a c++'11 expert here, but does a returning an std::tuple work?
> I talked to Howard about that.  It does work, but it is cumbersome.
 Either:
>
> std::tuple<std::unique_ptr<lld::File>, error_code>  result =
reader.readFile(path);
>

I suggest an error_or<T> class that can be combined with any result type,
not just File. It could be implemented with a fully generic variant class,
but the interface would likely be better with some specialization to
errors.

> Or you use std::tie() but then you still have to declare the two local
variables and tie them together.  You could define a wrapper class for the
tuple, but then you realize it would be simpler to push the error_code into
the File and get rid of the wrapper class.
>
> >> An other issue is that since llvm::error_code was designed to return a
fixed set of strings, there is no way to return a dynamic error message
(e.g. "relocation 27 is invalid").
> >
> > I'm pretty sure that the linker can specify a custom error_category
with whatever information you want, and then return an error_code put with
that.  Michael would know for sure, but it is definitely in the design of
error_code for it to be extensible.
>
> >From what I can tell error_code is about wrapping out OS specific "int"
error codes, and how to map those to static strings.  I suppose you could
abuse it by having a error_category which has a static vector or recently
dynamically generated strings and the int "error code" is an index into
that vector.
>
> Clang has this great Diagnostics infrastructure for showing where the
error occurred in the source file.  Does developing something like that
make sense for object files whose source is not human readable?
>
> >> In asking around for clean models on how to do this better, I've heard
two interesting ideas:
> >>
> >> 1) Incorporate the error_code and string message into the File class.
 That is the factory method always returns a File object. But there is a
method on File objects to query if they are valid or not.  If they are not,
there is a method to get the error_code and/or error string.
> >>
> >> 2) A variation on this is to have a new subclass of File called
FileError.  If the Reader cannot construct a real FileFoo object because of
some error, it instead constructs a FileError object and returns that.  The
FileError object has methods to get the error_code and/or error string.
> >>
> >> With both these ideas the Reader method is:
> >>
> >>   virtual std::unique_ptr<lld::File> readFile(StringRef path);
> >>
> >> which is much easier to use.
> >>
> >>
> >> Any recommendations on a good way to design this?
> >
> > Would an API like:
> >
> >   virtual std::unique_ptr<lld::File> readFile(StringRef path,
error_code &ErrorInfo);
>
> That is a common pattern, used for instance in the bit code reader.  If
we can unify the OS level error and the syntax/semantic errors from parsing
object files, this pattern might work.
>
> One other thought that leans me towards integrating the error code/msg
into the File object is that it works cleaner when you start adding
multithreading to the linker.  When you have multiple threads going, each
reading an object file and one of them returns an error, you need to
cleaning shut down the other threads, and return the error.  With the error
status integrated into the File object, all you do is have each thread
append it resulting File object onto a list, then after all threads are
joined, run through the list once to see if any say they encountered an
error.  If the error_code is returned separately from the File, you have to
create some data structure to bind together the error_code with which file
path could not be parsed.
>
> -Nick
>
>
>
>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120619/6d263d3f/attachment.html>