This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Disconnect instrumentation from the reproducers
ClosedPublic

Authored by JDevlieghere on Jan 19 2022, 12:22 PM.

Details

Summary

Remove the last remaining references to the reproducers from the instrumentation.

  • ReproducerInstrumentation.cpp -> Instrumentation.cpp
  • LLDB_RECORD_ -> LLDB_INSTRUMENT

I was going to with a more SBAPI/LLDB API focussed naming scheme, but D117632 made me decide to keep things more generic.

Diff Detail

Event Timeline

JDevlieghere created this revision.Jan 19 2022, 12:22 PM
JDevlieghere requested review of this revision.Jan 19 2022, 12:22 PM

It seems the set of macros could be reduced. If you don't need to generate "unique identifiers" for each method, then you can stop worrying about const methods, constructors, etc. I think this could boil down to one or two macros...

It seems the set of macros could be reduced. If you don't need to generate "unique identifiers" for each method, then you can stop worrying about const methods, constructors, etc. I think this could boil down to one or two macros...

Yeah, I think we can reduce everything to:

#define LLDB_INSTRUMENT(...)                                                   \
  lldb_private::Instrumenter _instrumenter(LLVM_PRETTY_FUNCTION, __VA_ARGS__);

plus a variant that takes no args. The reason it's not part of this patch is because it means modifying every single line and I'm lazy^Z^Z^Z^Z not sure when I'll have the time for that. Maybe I'll just revive lldb-instr locally and have it generate the new macros for me.

Simplify macros

labath accepted this revision.Jan 20 2022, 11:04 AM
labath added inline comments.
lldb/include/lldb/Utility/Instrumentation.h
23

I didn't notice this before, but I think this should really be in some namespace, ideally even separated from the rest of "normal" code. lldb_private::instrumentation perhaps?

84

I guess this is not used?

This revision is now accepted and ready to land.Jan 20 2022, 11:04 AM

Thanks, I'll address those things before landing the patch.

Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2022, 6:06 PM