[llvm] r338975 - [ORC] Remove an incorrect use of 'cantFail'.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 16 14:19:34 PDT 2018


On Thu, Aug 16, 2018 at 11:37 AM Lang Hames <lhames at gmail.com> wrote:

> I'm not sure this particular case deserves a test case. Prior to the
> change, passing a valid object was a matter of contract.
>

Seems to me a test case demonstrating that the program behaves as expected
when an invalid object is passed, now that that's a valid use-case is
merited. Usual sort of LLVM test approach.

Whether it's a unit test or an end-to-end test I don't have strong feelings
about.


> After the change it is a trivial error-propagation, and we don't usually
> test them at every level of the stack.
>
> I would like to see more "bad object" tests for libObject itself, but
> that's outside the scope of this patch.
>
> -- Lang.
>
> On Mon, Aug 6, 2018 at 8:47 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> Test case?
>>
>> On Sun, Aug 5, 2018 at 4:55 PM Lang Hames via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: lhames
>>> Date: Sun Aug  5 16:55:35 2018
>>> New Revision: 338975
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=338975&view=rev
>>> Log:
>>> [ORC] Remove an incorrect use of 'cantFail'.
>>>
>>> This code was moved out from BasicObjectLayerMaterializationUnit, which
>>> required
>>> the supplied object to be well formed. The getObjectSymbolFlags function
>>> does
>>> not require a well-formed object, so we have to propagate the error here.
>>>
>>> Modified:
>>>     llvm/trunk/lib/ExecutionEngine/Orc/Layer.cpp
>>>
>>> Modified: llvm/trunk/lib/ExecutionEngine/Orc/Layer.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Layer.cpp?rev=338975&r1=338974&r2=338975&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/ExecutionEngine/Orc/Layer.cpp (original)
>>> +++ llvm/trunk/lib/ExecutionEngine/Orc/Layer.cpp Sun Aug  5 16:55:35 2018
>>> @@ -116,8 +116,10 @@ Expected<SymbolFlagsMap> getObjectSymbol
>>>    for (auto &Sym : (*Obj)->symbols()) {
>>>      if (!(Sym.getFlags() & object::BasicSymbolRef::SF_Undefined) &&
>>>          (Sym.getFlags() & object::BasicSymbolRef::SF_Exported)) {
>>> -      auto InternedName =
>>> -          ES.getSymbolStringPool().intern(cantFail(Sym.getName()));
>>> +      auto Name = Sym.getName();
>>> +      if (!Name)
>>> +        return Name.takeError();
>>> +      auto InternedName = ES.getSymbolStringPool().intern(*Name);
>>>        auto SymFlags = JITSymbolFlags::fromObjectSymbol(Sym);
>>>        if (!SymFlags)
>>>          return SymFlags.takeError();
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180816/bb3738a7/attachment.html>


More information about the llvm-commits mailing list