<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jul 21, 2017 at 9:54 AM Derek Schuff via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dschuff added a comment.<br>
<br>
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:<br>
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?<br></blockquote><div><br>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.<br><br>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?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D35522" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35522</a><br>
<br>
<br>
<br>
</blockquote></div></div>