This is an archive of the discontinued LLVM Phabricator instance.

Allow empty mappings for optional YAML input
ClosedPublic

Authored by kastiglione on Nov 10 2017, 9:53 AM.

Details

Summary

This change fixes a bug where obj2yaml can in some cases produce YAML that
causes yaml2obj to error.

The ELF YAML document structure has a Sections mapping, which contains three
mappings, all of which are optional: Local, Global, and Weak. Any one of
these can be missing, but if all three are missing, then yaml2obj errors. This
change allows YAML input for cases like this one.

I have tested this with check-llvm and check-lld, and all tests passed.

This change is the result of test failures while working on D39582, which
introduces a DynamicSymbols mapping, which will be empty at times.

Event Timeline

kastiglione created this revision.Nov 10 2017, 9:53 AM
kastiglione added a subscriber: silvas.

Hi @silvas, this is a small change and related to the other ELF/YAML changes I made. Does this look ok to you?

kastiglione edited the summary of this revision. (Show Details)Nov 14 2017, 3:43 PM
compnerd accepted this revision.Nov 15 2017, 9:58 PM
compnerd added inline comments.
lib/Support/YAMLTraits.cpp
164

I would inline and apply Demorgan's:

if (Required || !isa<EmptyHNode>(CurrentNode))
  setError(...)
This revision is now accepted and ready to land.Nov 15 2017, 9:58 PM

inline if condition

This revision was automatically updated to reflect the committed changes.