[PATCH] D35522: Move Runtime libcall definitions to a .def file

[David Blaikie via llvm-commits]

On Fri, Jul 21, 2017 at 9:54 AM Derek Schuff via Phabricator <
reviews at reviews.llvm.org> wrote:

> dschuff added a comment.
>
> So it looks like switching the API away from char*-based would be, maybe
> not super-trivial, but probably not too bad. It's interlinked essentially
> with other APIs relating to external symbols:
> mostly `SelectionDAG::getExternalSymbol()`, and
> also`MachineInstrBuilder::addExternalSymbol()` but those could be converted
> too, and I don't think I'd mind doing it as general cleanup. I wonder if
> Expected<StringRef> is a little heavyweight though. It seems to have a
> really strong convention that the result should be checked first, and most
> use cases just don't care (arguably they could assert, but requiring that
> would add a lot of boilerplate asserts for probably not a lot of benefit).
> StringRef by itself (with empty string taking the place of the current use
> of nullptr) wouldn't be too bad, although having the empty string as a sort
> of sentinel value only used by a small fraction of the uses doesn't seem
> ideal either. I might still be inclined to prefer that over Expected
> though. Any thoughts?
>

Oh, yeah, if there's no real "Error" that needs to be returned here just
"string or <nothing>" then Optional<StringRef> Might be a better option.

Presumably most/all callers should be testing that they get the "not
nothing" answer, though? Or do many callers call it on specific intrinsics
that aren't the 'interesting' ones that might not be present/named?


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D35522
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170721/322793ca/attachment.html>