[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added inline comments. Comment at: utils/clangdiag.py:62 + +def enable(debugger, args): +# Always disable existing breakpoints Pass exe_ctx or target into this instead of the debugger (see Jim's comment on execution contexts below.

[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Looks good. A little bit of cleanup. Let me know what you think of the comments. Comment at: utils/clangdiag.py:17 +import os +from subprocess import check_output as qx; + I would rather just do: ``` import subprocess ``` Then later

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. Please do convert to python. Just know that you can use "lldb -P" to get the python path that is needed in order to do "import lldb" in the python script. So you can try doing a "import lldb", and if that fails, catch the exception, run "lldb -P", add that path to the

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. If you want to run the script from the command line, then it is necessary. If it is run from within LLDB it will just work. I like to have my LLDB python scripts work both ways. This might be better implemented as a new command that gets installed and can be used

[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg accepted this revision. clayborg added inline comments. Comment at: utils/clangdiag.py:117 +disable(exe_ctx) +else: +getDiagtool(exe_ctx.GetTarget(), args.path[0]) hintonda wrote: > clayborg wrote: > > should probably verify that

[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Looks good. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Each lldb.SBValue has accessors for the stuff in an execution context: `` lldb::SBTarget GetTarget(); lldb::SBProcess GetProcess(); lldb::SBThread GetThread(); lldb::SBFrame GetFrame(); You could keep a global map of process ID to diagtool if you want?

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

clayborg added a comment. Actually, we might need to still emit some Apple tables as the objective C and namespace tables might still be needed even with the DWARF5 tables. Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

clayborg added a comment. We want to ensure that both Apple and DWARF5 tables never get generated though. That would waste a lot of space. I would say if DWARF5 tables are enabled, then we need ensure we disable Apple tables. Repository: rC Clang https://reviews.llvm.org/D51576

[PATCH] D69933: [ASTImporter] Limit imports of structs

clayborg added a comment. Clang AST contexts know how to complete types and is done via the external AST source code that will ask a type to complete itself. Each object file has an AST that knows how to lazily complete a type when and only when it is needed. Each object file also only knows

[PATCH] D78874: [clang] Add vendor identity for Hygon Dhyana processor

clayborg added a comment. I am not sure I am the right person to review? I work on LLDB and LLVM mostly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78874/new/ https://reviews.llvm.org/D78874 ___

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inline comments. Changes needed are: - test for eBroadcastBitSymbolsLoaded - add "version" to module info - test all data we expect in module info ('name', 'symbolFilePath',

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just test for paths and this will be good to go! Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:35 +

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82477/new/ https://reviews.llvm.org/D82477

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So looks like you didn't use "git commit --amend -a" again here. These changes are incremental changes on your patch which hasn't been submitted yet? Comment

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg added a comment. In D82477#2145565 , @MaskRay wrote: > Hi, your git commit contains extra Phabricator tags. You can drop > `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git > commit with the following script: > >

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg added a comment. Here is a makefile that does stripping: llvm-project/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile Then when creating the target, use a.out.stripped: exe = self.getBuildArtifact("a.out.stripped") symbols = exe = self.getBuildArtifact("a.out") target =

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. We need to add a test for the symbols added target notification. See my previous comment on stripping a.out to a.out.stripped and then using "a.out.stripped" as the main

[PATCH] D82505: [lldb-vscode] Add Support for Module Event

clayborg added a comment. This seems like a continuation of https://reviews.llvm.org/D82477. I would close this one down by selecting abandon revision, and then click the "update diff" button in D82477 and upload your new patch. I am guessing you did this

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Looks like there is an indentation issue in the test. See inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61 +

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a space before the '(' of the debug info size. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:357 + std::ostringstream oss; + oss << "("; + oss <<

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61 +active_modules = self.vscode.get_active_modules() +program_module = active_modules[program_basename] +

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So you should revert any clang format changes that are not in modified lines of source, mostly revert a lot of lines in lldb-vscode.cpp. Comment at:

[PATCH] D27683: Prepare PrettyStackTrace for LLDB adoption

clayborg added inline comments. Comment at: llvm/trunk/lib/Support/PrettyStackTrace.cpp:92-93 +#elif defined(__APPLE__) && HAVE_CRASHREPORTER_INFO +extern "C" const char *__crashreporter_info__ +__attribute__((visibility("hidden"))) = 0; asm(".desc ___crashreporter_info__,

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

clayborg requested changes to this revision. clayborg added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1305 + + // If a function has an auto return type we need to find the defintion since + // that will have the deduced return

[PATCH] D116113: Add LLDB synthetic child and summary scripts for llvm::SmallVector, llvm::Optional, llvm::ErrorOr and llvm::Expected.

clayborg created this revision. clayborg added reviewers: labath, teemperor, aprantl. Herald added subscribers: usaxena95, arphaman. clayborg requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This allows us to see the contents of the

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

clayborg added a comment. Changes look fine to me. I would like someone that specializes in the expression parser to give the final ok though. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp:551 const