[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

clayborg added inline comments. Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93 + void GetCompilerContext( + llvm::SmallVectorImpl &context) const; + clayborg wrote: > clayborg wrote: > > aprantl wrote: > > > aprantl wrote: > > > > Why can't this be

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

clayborg added inline comments. Comment at: lldb/include/lldb/Core/Module.h:435 + /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a". + lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match); aprantl wrote: > clayborg wrote: > > aprantl wrot

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. In D138939#3964677 , @erichkeane wrote: > In D138939#3964439 , @erichkeane > wrote: > >> In D138939#3964404 , @cjdb wrote: >> >>> In D138939#396349

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb updated this revision to Diff 479492. cjdb marked 4 inline comments as done. cjdb added a comment. responds to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939 Files: clang-tools-extra/clan

[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work

jingham added a comment. Note that the two most common uses of single step in lldb are 1) stepping over the instruction under a breakpoint (which may be a branch) and 2) stepping to the next instruction from a branch instruction. So stepping won't work correctly till you get single step past a

[Lldb-commits] [PATCH] D139082: [lldb][Module] Document ModuleList::ForEach and assert nullness

Michael137 added a comment. In D139082#3965316 , @aprantl wrote: > If we assert non-null, why still pass a shared pointer? Could we have the > lambda take a `Module &`? I took a stab at changing it to pass a reference but turns out there's several plac

[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work

lh03061238 created this revision. lh03061238 added reviewers: SixWeining, wangleiat, xen0n, xry111, MaskRay, DavidSpickett. Herald added subscribers: StephenFan, s.egerton, simoncook. Herald added a project: All. lh03061238 requested review of this revision. Herald added subscribers: lldb-commits,

[Lldb-commits] [PATCH] D139082: [lldb][Module] Document ModuleList::ForEach and assert nullness

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. If we assert non-null, why still pass a shared pointer? Could we have the lambda take a `Module &`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [lldb] 33cbda4 - Improve error logging when xcrun fails to execute successfully

Author: Adrian Prantl Date: 2022-12-01T16:22:14-08:00 New Revision: 33cbda4cacb87103bbb5f4f9f2368bbb442132f4 URL: https://github.com/llvm/llvm-project/commit/33cbda4cacb87103bbb5f4f9f2368bbb442132f4 DIFF: https://github.com/llvm/llvm-project/commit/33cbda4cacb87103bbb5f4f9f2368bbb442132f4.diff

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

This revision was automatically updated to reflect the committed changes. Closed by commit rG33cbda4cacb8: Improve error logging when xcrun fails to execute successfully (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Core/ModuleList.cpp:1079 +bool ModuleList::AnyOf( +std::function const &callback) +const { module Repository: rG LL

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. I think this is good - welcome to wait for a second opinion if you prefer, or folks can provide feedback post-commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 updated this revision to Diff 479446. Michael137 added a comment. - Add dylib tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138724/new/ https://reviews.llvm.org/D138724 Files: lldb/source/Target/Target.cpp lldb/test/API/functi

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

Michael137 updated this revision to Diff 479444. Michael137 added a comment. - Reference insstead of `shared_ptr` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139083/new/ https://reviews.llvm.org/D139083 Files: lldb/include/lldb/Core/ModuleList

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

clayborg added a comment. We have modified the DIERef class over the years and made it work for both DWARF in .o files (OSO) for mac and then for fission (.dwo). The two made different changes that never affected either one but with these changes they need to coexist and actually work with DIER

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

clayborg added a comment. In D138618#3963767 , @labath wrote: > The explanation makes sense, and I *think* the patch is ok, but it's hard to > review it with all the noise. I still believe the DIERef change would be > better off as a separate patch, so

[Lldb-commits] [PATCH] D138960: [lldb] Enable use of dummy target from dwim-print

This revision was automatically updated to reflect the committed changes. Closed by commit rGcebb87e7dce8: [lldb] Enable use of dummy target from dwim-print (authored by kastiglione). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138960/new/ https:/

[Lldb-commits] [lldb] cebb87e - [lldb] Enable use of dummy target from dwim-print

Author: Dave Lee Date: 2022-12-01T13:21:24-08:00 New Revision: cebb87e7dce891b9a70ccb28e7da8c1fc71f2439 URL: https://github.com/llvm/llvm-project/commit/cebb87e7dce891b9a70ccb28e7da8c1fc71f2439 DIFF: https://github.com/llvm/llvm-project/commit/cebb87e7dce891b9a70ccb28e7da8c1fc71f2439.diff LOG:

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

Michael137 added a comment. In D139083#3964612 , @aprantl wrote: > This is definitely useful — I just have one question: Isn't ForEach a special > case of AnyOf? Could we implement on in terms of the other? Yup it is. But it looked a bit too clever and

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

Michael137 added inline comments. Comment at: lldb/include/lldb/Core/ModuleList.h:480 + /// This function is thread-safe. + bool AnyOf(std::function const + &callback) const; aprantl wrote: > Why not `std::function` ? Unfortunately a lot of APIs

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964439 , @erichkeane wrote: > In D138939#3964404 , @cjdb wrote: > >> In D138939#3963496 , @erichkeane >> wrote: >> >>> In D138939

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

aprantl added a comment. This is definitely useful — I just have one question: Isn't ForEach a special case of AnyOf? Could we implement on in terms of the other? Comment at: lldb/include/lldb/Core/ModuleList.h:480 + /// This function is thread-safe. + bool AnyOf(std::functi

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM with a slightly more relaxed test Comment at: lldb/source/Target/Target.cpp:1707 + +if (should_flush_type_systems) { + m_scratch_type_system_map.Clear();

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964404 , @cjdb wrote: > In D138939#3963496 , @erichkeane > wrote: > >> In D138939#3963473 , @tschuett >> wrote: >> >>> Maybe `voi

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

akyrtzi added a comment. In D139066#3964353 , @jasonmolenda wrote: > FWIW I think the only change needed to the original patch is to keep using > `#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextEverything + 1))` > but switch to the new eSymbo

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb marked 3 inline comments as done. cjdb added a comment. In D138939#3963496 , @erichkeane wrote: > In D138939#3963473 , @tschuett > wrote: > >> Maybe `void FormatDiagnostic(SmallVectorImpl &OutStr, Diagnosti

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

jasonmolenda added a comment. FWIW I think the only change needed to the original patch is to keep using `#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextEverything + 1))` but switch to the new eSymbolContextLastItem that is defined. Possibly the comment about what this code is doing

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

aprantl requested changes to this revision. aprantl added a comment. This revision now requires changes to proceed. I'll defer to @jasonmolenda Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139066/new/ https://reviews.llvm.org/D139066

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

jasonmolenda added a comment. a simple fix would be to stick the 5 extra flags the top of the 32-bit m_flags and hope that eSymbolContext* doesn't go that high. I wonder if there's some compile time error that could check that eSymbolContextEverything is less than a certain value or something

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

jasonmolenda added a comment. I'm not as sure about this. I think the idea was that eSymbolContextEverything would have a value like 0b and so we know only 4 bits (in my example here) are used for the eSymbolContext* enums. StackFrame's m_flags wants to use the bits above that range for R

[Lldb-commits] [lldb] e2a10d8 - [lldb] Remove timer from Module::GetNumCompileUnits

Author: Dave Lee Date: 2022-12-01T09:41:51-08:00 New Revision: e2a10d8ca34a3554d8d19d2bbdd3133970e4d09b URL: https://github.com/llvm/llvm-project/commit/e2a10d8ca34a3554d8d19d2bbdd3133970e4d09b DIFF: https://github.com/llvm/llvm-project/commit/e2a10d8ca34a3554d8d19d2bbdd3133970e4d09b.diff LOG:

[Lldb-commits] [PATCH] D138878: [lldb] Remove timer from Module::GetNumCompileUnits

This revision was automatically updated to reflect the committed changes. Closed by commit rGe2a10d8ca34a: [lldb] Remove timer from Module::GetNumCompileUnits (authored by kastiglione). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138878/new/ https

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

ayermolo added a comment. In D138618#3963767 , @labath wrote: > The explanation makes sense, and I *think* the patch is ok, but it's hard to > review it with all the noise. I still believe the DIERef change would be > better off as a separate patch, so

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added inline comments. Comment at: lldb/include/lldb/Core/Module.h:435 + /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a". + lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match); clayborg wrote: > aprantl wrote: > > Why is this n

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

labath added a comment. The explanation makes sense, and I *think* the patch is ok, but it's hard to review it with all the noise. I still believe the DIERef change would be better off as a separate patch, so that the change is not obscured by the (hopefully mechanical) aspects of increasing th

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

labath added a comment. You may want to check that this kind of automatic dereferencing does not send lldb into a tailspin if the printed data structure is recursive. I know we had problems like that with smart pointer pretty printers. I'd try some code like: #include #include struc

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3963473 , @tschuett wrote: > Maybe `void FormatDiagnostic(SmallVectorImpl &OutStr, DiagnosticMode > mode)`instead of `void FormatDiagnostic(SmallVectorImpl &OutStr)`? > To make the transition easer and future proof.

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe `void FormatDiagnostic(SmallVectorImpl &OutStr, DiagnosticMode mode)`instead of `void FormatDiagnostic(SmallVectorImpl &OutStr)`? To make the transition easer and future proof. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3958185 , @cjdb wrote: >> The clang-side interface to this seems a touch clunky, and I fear it'll make >> continuing its use/generalizing its use less likely. > > Is this w.r.t. the `FormatDiagnostic` being split up

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

Michael137 added a comment. I suppose I could just make ForEach return a boolean instead of breaking. Could then re-use that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139083/new/ https://reviews.llvm.org/D139083 __

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 updated this revision to Diff 479196. Michael137 added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138724/new/ https://reviews.llvm.org/D138724 Files: lldb/source/Target/Target.cpp lldb/test/API/functionalities

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

Michael137 created this revision. Michael137 added a reviewer: aprantl. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D139083 Files:

[Lldb-commits] [PATCH] D139082: [lldb][Module] Document ModuleList::ForEach and assert nullness

Michael137 created this revision. Michael137 added a reviewer: aprantl. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Currently all callsites already assume the pointer is non-null. This patch just

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 updated this revision to Diff 479190. Michael137 added a comment. - Reflow comment - Remove redundant null-check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138724/new/ https://reviews.llvm.org/D138724 Files: lldb/include/lldb/Core/

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 added inline comments. Comment at: lldb/source/Target/Target.cpp:1686 +const bool should_flush_type_systems = +module_list.AllOf([](const lldb::ModuleSP &module_sp) { + if (!module_sp) Michael137 wrote: > kastiglione wrote: > > How

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 added inline comments. Comment at: lldb/source/Target/Target.cpp:1686 +const bool should_flush_type_systems = +module_list.AllOf([](const lldb::ModuleSP &module_sp) { + if (!module_sp) kastiglione wrote: > How come this is `AllOf`

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

Michael137 added inline comments. Comment at: lldb/source/Core/ModuleList.cpp:1080 + bool ret = true; + ForEach([&](const ModuleSP &module_sp) { +ret &= callback(module_sp); aprantl wrote: > kastiglione wrote: > > I wonder why ForEach doesn't deal out a `Mo