[Lldb-commits] [PATCH] D157609: [lldb] Add more ways to find split DWARF files

dblaikie added a comment. In D157609#4645298 , @clayborg wrote: > In D157609#4644194 , @DavidSpickett > wrote: > >>> Any chance we could simplify this situation and have dwo searches use >>> exactly the

[Lldb-commits] [PATCH] D157609: [lldb] Add more ways to find split DWARF files

dblaikie added a comment. Any chance we could simplify this situation and have dwo searches use exactly the same/shared logic as source file searches? It seems like the dwarf spec intent was for that to be the case. I don't mind adding more ways to find things, but think it'd be useful if we

[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

dblaikie added a comment. Filed https://github.com/llvm/llvm-project/issues/64093 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154907/new/ https://reviews.llvm.org/D154907 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

dblaikie added a subscriber: jmorse. dblaikie added a comment. Simple clang example that produces this invalid DWARF: void b(double); void c(); void e(double e) { c(); b(e); } $ clang-tot x.ii -g -c -o - -O3 | llvm-dwarfdump-tot - | grep DW_OP_deref_size

[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

dblaikie added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:1082-1089 void *src = (void *)stack.back().GetScalar().ULongLong(); intptr_t ptr; ::memcpy(, src, sizeof(void *)); // I can't decide whether the size

[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

dblaikie added a comment. In D154907#4487335 , @jasonmolenda wrote: > This looks good to me, thanks for digging in Caroline! Is there a naughty > compiler emitting this, or are we mis-parsing somehow? In D154907#4487523

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

dblaikie added a comment. In D153840#4474213 , @cmtice wrote: > Hi Jason, > > I had been talking more with David, and yes, I had come to the conclusion > that you are both right and that this was not the right fix. I am planning > on reverting this,

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

dblaikie added a comment. I'm not sure if this is the right fix - these reads are for implementing DW_OP_deref_size, by the looks of it - so I think it does make sense that the size read is not the size of the address, but the size specified in the DW_OP_deref_size. There is a requirement that

[Lldb-commits] [PATCH] D152708: [RFC][Draft] Enable primitive support for Two-Level Line Tables in LLVM

dblaikie added a comment. Are there any known (or vague/unknown) limitations on the implementation with respect to the actual output/on-disk representation? (like, would doing some kind of size analysis of the output when using this patch/feature lead to incorrect conclusions about the cost of

[Lldb-commits] [PATCH] D152708: [RFC][Draft] Enable primitive support for Two-Level Line Tables in LLVM

dblaikie added a comment. Really appreciate you providing this prototype. > This solves one of the current weaknesses in DWARF debugging, which is the > inability to set breakpoints or step onto inlined callsites. However, there > are other proposals (such as Location View Numbering) that

[Lldb-commits] [PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

dblaikie added a comment. Got a link to a design discussion motivating this change? I'd have thought it made sense to put modulemaps in subdirectories - since they cover the whole directory, putting them in the root of an include path would be problematic if there are multiple distinct

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

dblaikie added a comment. In D147292#4238834 , @augusto2112 wrote: > In D147292#4238820 , @JDevlieghere > wrote: > >> There seems to be overlap in the code added in this patch and D146679 >>

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

dblaikie added inline comments. Comment at: clang/test/CodeGen/preferred_name.cpp:49 + +Foo> varFooInt; + Michael137 wrote: > dblaikie wrote: > > Michael137 wrote: > > > probinson wrote: > > > > This doesn't become `Foo` ? > > > The name stays as `Foo>` but

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

dblaikie added inline comments. Comment at: clang/test/CodeGen/preferred_name.cpp:49 + +Foo> varFooInt; + Michael137 wrote: > probinson wrote: > > This doesn't become `Foo` ? > The name stays as `Foo>` but the type of the template parameter > becomes

[Lldb-commits] [PATCH] D145609: [lldb] Change dwim-print to default to disabled persistent results

dblaikie added a comment. > The rationale is: dwim-print doesn't always use expression evaluation, it > prefers to use frame variable where possible. In the future it could be > expanded, for example to print register as well. Because dwim-print doesn't > always use expression, there isn't

[Lldb-commits] [PATCH] D143501: [clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

dblaikie added a comment. > I think /maybe/ we had some design principle that DWARF features wouldn't be > solely controlled by debugger tuning, the tuning only indicates defaults but > tehy can be controlled by flags? Not sure if I'm remembering that quite > right, though - maybe @probinson

[Lldb-commits] [PATCH] D143501: [clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

dblaikie added a subscriber: probinson. dblaikie added a comment. In D143501#4116200 , @Michael137 wrote: >> I'd recommend a possible long-term solution would be simplified template >> names (so we don't have to worry about encoding this in the

[Lldb-commits] [PATCH] D143501: [clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

dblaikie added a comment. In D143501#4110302 , @Michael137 wrote: > The alternative would be attaching the preferred name as a new attribute or > an existing attribute like `DW_AT_description`. I'd recommend a possible long-term solution would be

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

dblaikie added inline comments. Comment at: lldb/source/API/SBSection.cpp:187-189 +DataExtractorSP result_data_sp = +std::make_shared(section_data, offset, size); +sb_data.SetOpaque(result_data_sp); Probably either use `std::move` when passing

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D139379#4015624 , @ayermolo wrote: > In D139379#4015569 , @dblaikie > wrote: > >> In

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

dblaikie added a comment. In D139379#3972876 , @ayermolo wrote: > In D139379#3972871 , @dblaikie > wrote: > >> Perhaps the change to use accessors could be removed, now that you've used >> it to find all the

[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

dblaikie added a comment. This has introduced a circular dependency due to the forwarding header (forwarding header depends on new lib, new lib depends on support, where the forwarding header is). Generally this wouldn't be acceptable (& I'd suggest the patch be reverted on that basis) though

[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params

dblaikie added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1251 + if (has_template_params && + ParseTemplateParameterInfos(die, template_param_infos)) { +template_function_decl =

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

dblaikie added a comment. Perhaps the change to use accessors could be removed, now that you've used it to find all the places that needed to be fixed up? (like just using it for cleanup/temporary purposes, without needing to commit that API change?) Comment at:

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

dblaikie added a comment. Sorry if this is derailing, but I wonder/worry about a few things here: 1. Compounding structured output with phraseology - it seems like it might be worthwhile for these to be separate issues (doesn't mean the terminal output has to say exactly the same thing - as

[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

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

dblaikie added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:747-749 + if (ParseTemplateParameterInfos(die, template_param_infos) && + (!template_param_infos.args.empty() || + template_param_infos.packed_args)) {

[Lldb-commits] [PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good to me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138539/new/ https://reviews.llvm.org/D138539

[Lldb-commits] [PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

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

[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

dblaikie added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:2676-2677 if (!m_deref_valobj) { - if (HasSyntheticValue()) { + // FIXME: C++ stdlib formatters break with incomplete types (e.g. + // `std::vector &`). Remove ObjC restriction once

[Lldb-commits] [PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

dblaikie added a comment. In D137583#3917706 , @aaron.ballman wrote: >> ...we expect template params to be fully qualified when comparing them for >> simple template names > > So lldb is not inspecting the AST, they're doing reflection (of a sort) on

[Lldb-commits] [PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

dblaikie added a subscriber: aaron.ballman. dblaikie added a comment. @aaron.ballman does this seem OK? (this was based on my suggestion in the related review linked in the description) It probably needs tests in clang too - not sure if there's an opportunity to use a unit test to simplify

[Lldb-commits] [PATCH] D137098: [lldb] Support simplified template names in the manual index

dblaikie added a comment. In D137098#3897289 , @aeubanks wrote: > updated description with why this doesn't produce false positives with > breakpoints > > this doesn't support regex function name lookup, not sure if we care enough > about the

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

dblaikie added a comment. In D136011#3870677 , @labath wrote: > In D136011#3866854 , @aeubanks > wrote: > >> In D136011#3862150 , @labath wrote: >> >>> >> >> Maybe

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

dblaikie added a comment. In D136011#3862150 , @labath wrote: > In D136011#3860637 , @dblaikie > wrote: > >> I think the place where this will go wrong is in terms of how lldb renders >> `char` values on

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

dblaikie added a comment. I think the place where this will go wrong is in terms of how lldb renders `char` values on non-default-char-signedness programs (it'll render them as the default-char-signedness, which might be confusing to a user - since they'll be looking at literals, etc, that are

[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looked like this was a fairly uncontroversial part of the other patch, so I feel confident approving it - but welcome to wait for a second opinion from more lldb-affiliated developers.

[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

dblaikie added a comment. Been experimenting with this recently and I noticed that loading in the cached indexes seems to do a lot of loading - specifically interning a lot of strings from the index and the symtab. Does this happen when reading a built-in index (apple_names/debug_names) (I

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

dblaikie added a comment. Looking pretty good to me FWIW Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1561 +std::string +DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE ) { + if (!die.IsValid()) Sorry, when I gave

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

dblaikie added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:230-231 -DEBUG_INFO_FLAG ?= -g +# DO NOT SUBMIT +DEBUG_INFO_FLAG ?= -g -gsimple-template-names Oh, nifty place to

[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

dblaikie added a comment. FWIW, clang has support for including template parameter DIEs for template declarations (-Xclang -debug-forward-template-params). We could enable that by default when tuning for lldb (or maybe we're at the tipping point and we should enable it by default in general -

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

dblaikie added a comment. > The overall logic and the include and library paths make sense to me. The > rpath thingy will not work on windows as it (afaik) has no equivalent feature > (it has the equivalent of (DY)LD_LIBRARY_PATH though). Any idea what the libc++ tests do on Windows then? (on

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. >>> Finally, there have been already attempts to change the hash function to >>> use the better non-zero seed (D97396 ), >>> and they were reverted because something depended on the hash function not >>> changing, so I don't see it

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. >>> If the theory is that this should keep working even with the library >>> changing without LLDB rebuild, then as I wrote above that theory needs >>> double-checking first. And additionally a good question to ask would be if >>> it's really a good idea to do

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. In D122974#3536342 , @llunak wrote: > In D122974#3535669 , @dblaikie > wrote: > >>> It doesn't make sense to require a stable hash algorithm for an internal >>> cache file. >> >> It's

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. > It doesn't make sense to require a stable hash algorithm for an internal > cache file. It's at least a stronger stability requirement than may be provided before - like: stable across process boundaries, at least, by the sounds of it? yeah? & then still raises the

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. In D122974#3481269 , @llunak wrote: > In D122974#3480686 , @dblaikie > wrote: > >> In D122974#3424654 , @JDevlieghere >> wrote: >> >>> > >

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

dblaikie added inline comments. Comment at: bolt/include/bolt/Core/DebugData.h:375-377 +if (Optional DWOId = Unit.getDWOId()) + return *DWOId; +return Unit.getOffset(); ayermolo wrote: > ayermolo wrote: > > dblaikie wrote: > > > That seems like a

[Lldb-commits] [PATCH] D124370: [lldb/DWARF] Fix linking direction in CopyUniqueClassMethodTypes

dblaikie added a comment. In D124370#3474824 , @labath wrote: > In D124370#3472394 , @clayborg > wrote: > >> So I believe the reason we need to be able to add methods to a class is for >> templates. Templates

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. In D122974#3424654 , @JDevlieghere wrote: > The ConstString/StringPool is pretty hot so I'm very excited about making it > faster. > > I'm slightly concerned about the two hash values (the "full" hash vs the > single byte

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

dblaikie added a comment. (sorry, this slipping through somehow - it's on my radar now) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122974/new/ https://reviews.llvm.org/D122974 ___ lldb-commits

[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

dblaikie added a comment. I take it no tests fail upon removing this condition? Any hints in version history about its purpose? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124370/new/ https://reviews.llvm.org/D124370

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

dblaikie added inline comments. Comment at: libcxx/utils/gdb/libcxx/printers.py:192 class StdStringPrinter(object): """Print a std::string.""" philnik wrote: > jgorbe wrote: > > Mordante wrote: > > > philnik wrote: > > > > Mordante wrote: > > > > >

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

dblaikie added a subscriber: rsmith. dblaikie added a comment. In D118812#3291303 , @jingham wrote: > In D118812#3291109 , @dblaikie > wrote: > >> Any chance you might want a limit on the size of the demangled

[Lldb-commits] [PATCH] D118812: [lldb] Add a setting to skip long mangled names

dblaikie added a comment. Any chance you might want a limit on the size of the demangled name too? (might be worth considering what the most densely encoded mangled name is (ie: what's the longest name that could be produced by a 10k long mangled name? and see if that's worth having another

[Lldb-commits] [PATCH] D113604: [lldb][NFC] Format lldb/include/lldb/Symbol/Type.h

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks alright CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113604/new/ https://reviews.llvm.org/D113604 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial location at function start

dblaikie added a comment. Side note: The src_file is not to be trusted/used either - once line 0 is specified, nothing else in that line entry is valid. LLVM lets the previous line entries file persist because this reduces encoding size (by not having to switch all the fields in the line table

[Lldb-commits] [PATCH] D113604: [lldb][NFC] Format lldb/include/lldb/Symbol/Type.h

dblaikie requested changes to this revision. dblaikie added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Symbol/Type.h:204-206 + static bool GetTypeScopeAndBasename(const llvm::StringRef name, +

[Lldb-commits] [PATCH] D113604: [lldb][NFC] Format lldb/include/lldb/Symbol/Type.h

dblaikie added inline comments. Comment at: lldb/include/lldb/Symbol/Type.h:205-206 + static bool GetTypeScopeAndBasename(const llvm::StringRef name, + llvm::StringRef scope, + llvm::StringRef basename,

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

dblaikie added a comment. In D110578#3123164 , @ljmf00 wrote: > @dblaikie Maybe you can land this? Nah, might need someone with more lldb context than me - I don't seem to have a clean check-lldb build right now (so don't have a good baseline to

[Lldb-commits] [PATCH] D113314: [lldb] Use std::string instead of llvm::Twine

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me. Alternatively the string concatenation could be inlined, then the Twine usage would be correct - but would still need an explicit "str()", like this:

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

dblaikie added a comment. In D112165#3102055 , @teemperor wrote: > In D112165#3100929 , @dblaikie > wrote: > >> In D112165#3100608 , @teemperor >> wrote: >> >>> Small

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

dblaikie added a comment. In D112165#3100608 , @teemperor wrote: > Small note: gmodules test are never run on Linux, so you actually have to run > them on macOS (or I think FreeBSD) to know whether the tests work. Yeah, I'll admit I didn't test this,

[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

dblaikie added inline comments. Comment at: lldb/source/API/SBTarget.cpp:1589 - const ConstString csFrom(from), csTo(to); - if (!csFrom) + llvm::StringRef srFrom(from), srTo(to); + if (srFrom.empty()) Aside (since the old code did this too): Generally code

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

This revision was automatically updated to reflect the committed changes. Closed by commit rGe6b323379e31: Cleanup a few more PR36048 skips (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112165/new/

[Lldb-commits] [PATCH] D112340: [lldb/Formatters] Remove space from vector type string summaries (NFCI)

dblaikie added a comment. In D112340#3084953 , @teemperor wrote: > Not actually sure when/how Green Dragon is mailing external people, > but usually someone just watches the build bot and emails/comments on > commits that are breaking the buildbot. Green

[Lldb-commits] [PATCH] D112340: [lldb/Formatters] Remove space from vector type string summaries (NFCI)

dblaikie added a comment. In D112340#3081540 , @teemperor wrote: > In D112340#3081532 , @dblaikie > wrote: > >> Sorry I missed this - are these tested anywhere/should I have been able to >> discover if these

[Lldb-commits] [PATCH] D112212: [lldb/test] Print build commands in trace mode

dblaikie added a comment. In D112212#3082352 , @JDevlieghere wrote: > In D112212#3081935 , @dblaikie > wrote: > >> In D112212#3081828 , @JDevlieghere >> wrote: >> >>>

[Lldb-commits] [PATCH] D112212: [lldb/test] Print build commands in trace mode

dblaikie added a comment. In D112212#3081828 , @JDevlieghere wrote: > In D112212#3080491 , @teemperor > wrote: > >> This LGTM, but `shlex.join` is actually Py3 exclusive and I don't think >> there is a good

[Lldb-commits] [PATCH] D112340: [lldb/Formatters] Remove space from vector type string summaries (NFCI)

dblaikie added a comment. Sorry I missed this - are these tested anywhere/should I have been able to discover if these needed to be changed before I made the change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112340/new/

[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

This revision was automatically updated to reflect the committed changes. Closed by commit rGd723ad5bcf71: Enable libc++ in the build for libcxx initializerlist pretty printers (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D112212: [lldb/test] Print build commands in trace mode

dblaikie added a comment. Does this sort of thing itself get tested? (like this one had a test: https://reviews.llvm.org/D111978 but not sure how much that generalizes/whether there are different parts of the infrastructure are more or less testable) Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

dblaikie added a comment. In D112163#3077112 , @teemperor wrote: > LGTM, thanks! > > Tests with libc++ as a category are not run on Windows or when GCC is the > test compiler, so those decorators are redundant. Removing the Clang XFAIL > also seems

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

dblaikie created this revision. dblaikie added a reviewer: aprantl. dblaikie requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Hopefully these were also fixed bi r343545 / 7fd4513920d2fed533ad420976529ef43eb42a35 Repository: rG LLVM

[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

dblaikie added a comment. Herald added a subscriber: JDevlieghere. Looks like this test, when it was introduced/substantially refactored in D9426 - what was missing compared to all the other tests updated to run on linux, was the Makefile change to use libc++,

[Lldb-commits] [PATCH] D112163: Enable libc++ in the build for libcxx initializerlist pretty printers

dblaikie created this revision. dblaikie added reviewers: teemperor, labath. dblaikie 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/D112163 Files:

[Lldb-commits] [PATCH] D111981: [lldb] Fix missing dependency on libc++ from LLDB test suite on non-Darwin platforms

dblaikie added a comment. Yeah, applying that patch gets the expected `cannot open shared object file` issue for `libc++.so.1`: == FAIL: test_with_run_command_dwo (TestDataFormatterLibcxxSet.LibcxxSetDataFormatterTestCase)

[Lldb-commits] [PATCH] D111981: [lldb] Fix missing dependency on libc++ from LLDB test suite on non-Darwin platforms

dblaikie added a comment. In D111981#3071378 , @teemperor wrote: > In D111981#3071349 , @dblaikie > wrote: > >> So this'll add the right test dependency, if the libcxx project is enabled >> in the build? (& if

[Lldb-commits] [PATCH] D111981: [lldb] Fix missing dependency on libc++ from LLDB test suite on non-Darwin platforms

dblaikie added a comment. So this'll add the right test dependency, if the libcxx project is enabled in the build? (& if the build hasn't enabled libcxx, the tests will run, but against the system compiler - and if that system compiler happens to default to libc++ (

[Lldb-commits] [PATCH] D110455: DebugInfo: Use clang's preferred names for integer types

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf6a561c4d675: DebugInfo: Use clangs preferred names for integer types (authored by dblaikie). Herald added subscribers: llvm-commits, lldb-commits,

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

dblaikie added a comment. >> - We could supply a test written in C, but it needs -O1 and is fairly >> sensitive to the meaning of -O1 (e.g., clang started inlining and omitting >> unsued inlined parameters only recently, so changes to -O1 could make a C >> test easily meaningless). Any

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

dblaikie added a comment. Thanks for the suggestions/details, @dexonsmith - I've posted to llvm-dev here: https://groups.google.com/g/llvm-dev/c/m9UVRhzJvh4/m/qdd_SyPuCQAJ and will wait for some follow-up (or dead silence) before starting along probably your latter suggestion. Repository:

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

dblaikie added a comment. In D109345#2990577 , @dexonsmith wrote: > In D109345#2990527 , @dblaikie > wrote: > >> (were there other regressions I mentioned/should think about?) > > I don't have specific

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

dblaikie added a comment. In D109345#2987565 , @dexonsmith wrote: > This seems like the right direction to me! Especially like the > look-through-the-ErrorInfo change for `FileError` -- I hit this before and > found it annoying. Thanks for taking a

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

dblaikie added a comment. In D109345#2986297 , @thopre wrote: > Is there no way to split this patch further? It's going to be hard finding > someone who can review something so big. If there's no way to split it in > incremental changes, you could

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

dblaikie added a comment. In D97786#2976549 , @pfaffe wrote: > In D97786#2974879 , @dblaikie wrote: > >> Not fixable? Not sure I follow. In the case of cwd-relative (old behavior) >> you can fix the behavior by

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

dblaikie added a comment. In D97786#2974202 , @pfaffe wrote: > In D97786#2973868 , @dblaikie wrote: > >> This doesn't solve all use cases/it's not a terribly general purpose >> situation - using

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

dblaikie added a comment. In D97786#2972153 , @pfaffe wrote: > This change breaks all existing uses of relative comp_dirs that don't > accidentally make all of them relative to the executable's directory already. > > It's easy to construct broken use

[Lldb-commits] [PATCH] D108228: Fix error handling in the libcxx std::string formatter

dblaikie added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:102 +std::string *not_a_string = (std::string *) 0x0; +touch_string(*not_a_string); return 0; teemperor wrote: >

[Lldb-commits] [PATCH] D108228: Fix error handling in the libcxx std::string formatter

dblaikie added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:101 S.assign(L"!"); // Set break point at this line. +std::string *not_a_string = (std::string *) 0x0; +

[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D106466#2950268 , @jankratochvil wrote: > In D106466#2947710 , @dblaikie > wrote: > >> I assume

[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

dblaikie added a comment. I assume there's already test coverage for rnglistx in debug_info.dwo/split unit? (because in that case there's no rnglists_base, but rnglistx is usable) - could you explain how this code avoids treating the split unit rnglists_base == 0 case as "there is no

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

dblaikie added a comment. In D105732#2942716 , @jingham wrote: > Do modern Linux's not have posix_spawn? If it exists that's a better > interface, and lets the system handle a lot of the complicated machinations > you have to do by hand if you roll it

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

dblaikie added a comment. In D105732#2942355 , @clayborg wrote: > Sorry for the delay. This seems like it should work just fine. Where is this > actually used? I thought most clients should be using posix_spawn() as this > is the preferred launching

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

dblaikie accepted this revision. dblaikie added a comment. Sounds like this is good for Google and Sony, and Apple doesn't use `-fno-standalone-debug`/`-flimit-debug-info` anyway, so probably about ready to move forward, then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

dblaikie added a comment. In D106084#2897515 , @jmorse wrote: > David wrote: > >> think what I'm missing here: If -fno-standalone-debug is already in use/the >> default and is causing missing types because parts of the program are bulit >> without

[Lldb-commits] [PATCH] D103172: [lldb][NFC] Allow range-based for loops over DWARFDIE's children

dblaikie added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h:97-105 + template + llvm::iterator_range children() const { +return llvm::make_range(T(*this), T()); + } +}; + +class DWARFDIE::child_iterator rather than a

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

dblaikie added a comment. In D106084#2890469 , @probinson wrote: >> (hence the renaming of "limited" a long time ago to "standalone-debug" to >> create a policy/philosophy around what goes in each category). > > Sorry, what? I thought

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

dblaikie added a comment. In D106084#2886659 , @jmorse wrote: > This is going to be excellent for linux targets and similar, > > In D106084#2882970 , @probinson > wrote: > >> + @jmorse who is better placed than

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

dblaikie accepted this revision. dblaikie added subscribers: probinson, aprantl. dblaikie added a comment. This revision is now accepted and ready to land. Please wait for sign-off from @aprantl (or another appropriate Apple representative) & @probinson (or another appropriate Sony

  1   2   3   >