labath added inline comments.

================
Comment at: lldb/bindings/python.swig:132-136
+if 'LLDB_REPRODUCER_CAPTURE_PATH' in os.environ:
+   SBReproducer.Capture(os.environ['LLDB_REPRODUCER_CAPTURE_PATH'])
+   SBReproducer.SetAutoGenerate(True)
+elif 'LLDB_REPRODUCER_REPLAY_PATH' in os.environ:
+   SBReproducer.APIReplay(os.environ['LLDB_REPRODUCER_REPLAY_PATH'])
----------------
I am doubly unhappy about this. Not only does it leak testing logic into 
production code, it actually does that using environment variables... :/

I take it the problem here is that reproducers need to be set up before the 
Initialize call, but the lldb module does not let us do that as it calls 
Initialize automatically? I am not sure that was such a wise idea, but that 
ship has sailed a long time ago.

So, can we at least do something like [[ 
https://stackoverflow.com/questions/3720740/pass-variable-on-import/39360070#39360070
 | this ]] instead? I.e. have a separate configuration module which would be 
imported here and would drive this logic.

I believe it should be possible to arrange it so that the configuration module 
does not even have to get shipped -- this code could just attempt the import 
and ignore any errors.

And the configuration module itself would not need to do anything 
reproducer-related either. It could just contain a single flag to disable the 
automatic initialization. That sounds like a thing someone might conceivably 
want anyway, and it could be later extended to handle other kinds of global 
lldb configuration (ConstString pool size maybe ?).

Once the Initialize call is disabled, the normal test harness can handle the 
rest...


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77588/new/

https://reviews.llvm.org/D77588



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to