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

[Lang Hames via llvm-commits]

I think that once this functionality is exposed via lli/LLJIT it would be
reasonable to have a regression test to verify that a bad object files
trigger an error from the tool code. This will be a New Thing for JIT API
testing though: MCJIT and ORCv1 both notionally deal with bad object files,
but never had tests for them. I assume it was just an aversion to checking
in binary object-files for tests, but in this case we could just test with
an empty object file.

-- Lang.

On Thu, Aug 16, 2018 at 2:19 PM David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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/20180817/54ea8a0d/attachment.html>