[lld] r265477 - [lld][MachO] Check Expected<T> for error prior to destruction.

[Lang Hames via llvm-commits]

>
> +
>
> +  // Transfer ownership to "out" File parameter.
>
> +  std::unique_ptr<lld::File> f = std::move(*fileOrError);
>
> +  file = f.release();
>
>
> Aside, this ^ seems like an odd sequence. Are those two lines equivalent
> to:
>


  file = fileOrError->release();


Why yes. Yes they are. Fixed in r265566. I'll look at changing that
out-parameter to a unique_ptr in a subsequent patch too.

- Lang.

On Wed, Apr 6, 2016 at 8:29 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Apr 5, 2016 at 3:06 PM, Lang Hames via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Tue Apr  5 17:06:48 2016
>> New Revision: 265477
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=265477&view=rev
>> Log:
>> [lld][MachO] Check Expected<T> for error prior to destruction.
>>
>> This should fix the failures on the LLD bots caused by r265446.
>>
>>
>> Modified:
>>     lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
>>
>> Modified: lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp?rev=265477&r1=265476&r2=265477&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
>> (original)
>> +++ lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp Tue Apr
>> 5 17:06:48 2016
>> @@ -767,22 +767,9 @@ bool MachOYamlIOTaggedDocumentHandler::h
>>    MappingTraits<NormalizedFile>::mapping(io, nf);
>>    // Step 2: parse normalized mach-o struct into atoms.
>>    auto fileOrError = normalizedToAtoms(nf, info->_path, true);
>> -  if (nf.arch != _arch) {
>> -    io.setError(Twine("file is wrong architecture. Expected ("
>> -                      + MachOLinkingContext::nameFromArch(_arch)
>> -                      + ") found ("
>> -                      + MachOLinkingContext::nameFromArch(nf.arch)
>> -                      + ")"));
>> -    return false;
>> -  }
>> -  info->_normalizeMachOFile = nullptr;
>>
>> -  if (fileOrError) {
>> -    // Transfer ownership to "out" File parameter.
>> -    std::unique_ptr<lld::File> f = std::move(fileOrError.get());
>> -    file = f.release();
>> -    return true;
>> -  } else {
>> +  // Check that we parsed successfully.
>> +  if (!fileOrError) {
>>      std::string buffer;
>>      llvm::raw_string_ostream stream(buffer);
>>      handleAllErrors(fileOrError.takeError(),
>> @@ -793,6 +780,21 @@ bool MachOYamlIOTaggedDocumentHandler::h
>>      io.setError(stream.str());
>>      return false;
>>    }
>> +
>> +  if (nf.arch != _arch) {
>> +    io.setError(Twine("file is wrong architecture. Expected ("
>> +                      + MachOLinkingContext::nameFromArch(_arch)
>> +                      + ") found ("
>> +                      + MachOLinkingContext::nameFromArch(nf.arch)
>> +                      + ")"));
>> +    return false;
>> +  }
>> +  info->_normalizeMachOFile = nullptr;
>> +
>> +  // Transfer ownership to "out" File parameter.
>> +  std::unique_ptr<lld::File> f = std::move(*fileOrError);
>> +  file = f.release();
>>
>
> Aside, this ^ seems like an odd sequence. Are those two lines equivalent
> to:
>
>   file = fileOrError->release();
>
>
>> +  return true;
>>  }
>>
>>
>>
>>
>> _______________________________________________
>> 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/20160406/9be19676/attachment.html>