This is an archive of the discontinued LLVM Phabricator instance.

[Utility] Fix ArchSpec.MergeFrom to correctly merge environments
ClosedPublic

Authored by xiaobai on Feb 25 2019, 11:03 PM.

Details

Summary

This behavior was originally added in rL252264 (git commit 76a7f365da)
in order to be extra careful with handling platforms like watchos and tvos.
However, as far as triples go, those two (and others) are treated as OSes and
not environments, so that should not really apply here.

Additionally, this behavior is incorrect and can lead to incorrect ArchSpecs.
Because android is specified as an environment and not an OS, not propogating
the environment can lead to modules and targets being misidentified.

Diff Detail

Event Timeline

xiaobai created this revision.Feb 25 2019, 11:03 PM
xiaobai updated this revision to Diff 188312.Feb 25 2019, 11:14 PM
xiaobai set the repository for this revision to rLLDB LLDB.

Accidentally added a commit to this diff that I didn't intend to add.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2019, 11:14 PM
Herald added a subscriber: abidh. · View Herald Transcript

Could you please add more context to the diff? Something like git diff -U9999 HEAD~1 works fine.

clayborg accepted this revision.Feb 26 2019, 9:57 AM
This revision is now accepted and ready to land.Feb 26 2019, 9:57 AM

Could you please add more context to the diff? Something like git diff -U9999 HEAD~1 works fine.

Will do. It looks like the dependent diff might change significantly so I'm going to rebase this on top of master instead since the functionality isn't technically tied to the dependent diff.

xiaobai updated this revision to Diff 188417.Feb 26 2019, 10:50 AM

More context + rebase on top of master

xiaobai updated this revision to Diff 188474.Feb 26 2019, 4:02 PM

Rebase on master

This revision was automatically updated to reflect the committed changes.