[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