[PATCH] D48309: [sanitizer] Unify and generalize Apple platforms in CMake and lit test configs
Dan Liew via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 20 02:08:42 PDT 2018
delcypher accepted this revision.
delcypher added inline comments.
This revision is now accepted and ready to land.
================
Comment at: test/lit.common.cfg:132
+ prepare_output = json.loads(prepare_output_json)
+ config.environment.update(prepare_output["env"])
elif config.android:
----------------
kubamracek wrote:
> delcypher wrote:
> > We should probably check that the JSON data is what we expect before trying to use it.
> >
> > ```
> > assert 'env' in prepare_output
> > assert isinstance(prepare_output['env'], dict)
> > ```
> >
> > Or something similar.
> Hm, this seems very non-Python-y. Just using `prepare_output["env"]` implicitly checks for these things as well.
Not quite. Python is duck typed so as long as `prepare_output['env']` acts enough like a `dict` python is happy. You're right though that it probably isn't python-y, so I'll drop this point.
https://reviews.llvm.org/D48309
More information about the llvm-commits
mailing list