[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

This revision was automatically updated to reflect the committed changes. Closed by commit rG24f9a2f53db7: [LLDB] Applying clang-tidy modernize-use-equals-default over LLDB (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

shafik added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp:236 if (count) -m_name = ConstString((char *)buffer_sp->GetBytes()); +m_name = ConstString((const char *)buffer_sp->GetBytes()); else

[Lldb-commits] [PATCH] D123098: [lldb] Fix undefined behavior: left shift of negative value

shafik added a comment. Note, in C++20 this was made well-defined see godbolt the paper that did this was P0907R4 Signed Integers are Two’s Complement CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik created this revision. shafik added reviewers: labath, JDevlieghere, aprantl. Herald added a project: All. shafik requested review of this revision. Herald added a subscriber: aheejin. Applied clang-tidy `modernize-use-override` over LLDB and added it to the LLDB `.clang-tidy` config. ht

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik added a comment. In D123340#3438391 , @labath wrote: > The reason for the funny `/*override*/` thingy in the mock classes is that it > is impossible (in the gmock version that we use) to add the override keyword > to the methods overridden by the

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik updated this revision to Diff 423725. shafik added a comment. -Removed override `RemoteAwarePlatformTest.cpp` and added NOLINT CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123340/new/ https://reviews.llvm.org/D123340 Files: lldb/.clang-tidy lldb/source/Plugins/ExpressionPar

[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

shafik added a comment. Thank you, the names make a lot more sense to me but I am not sure why this was originally used either. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:648 + ValueObjectSP location_sp = + l->GetChildMemberWithName(ConstString("__data

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd1464604367: [LLDB] Applying clang-tidy modernize-use-override over LLDB (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

shafik added a comment. This mostly makes sense, the purpose of the `|| alternate_defn` was not clear to me either. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124370/new/ https://reviews.llvm.org/D124370 ___

[Lldb-commits] [PATCH] D123954: [lldb] Add setting for max depth of value object printing (NFC)

shafik added inline comments. Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:718 + /// been told. + ChildrenOmissionWarningStatus m_truncation_warning; + /// Whether we reached the maximum child nesting depth and whether the user Why not use in

[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

shafik added a comment. You might want to try fun cases like `operator<` and `operator()()` from a lambda. They should work but might be worth throwing them in. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:330 + +// size_t from = 0; +// llvm::Stri

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

shafik updated this revision to Diff 427081. shafik removed reviewers: teemperor, jingham, jasonmolenda. shafik added a comment. Herald added a project: All. - Expanded test - applied clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp:204 +#if __cplusplus == 201703L + // cxx17-error@-3 {{non-type template argument refers to subobject '(int *)1'}} +#endif Shouldn't this be an error b/c it is a tempo

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/test/CodeGenCXX/template-arguments.cpp:4 + +template CONSTEXPR T id(T v) { return v; } +template auto value = id(V); I don't see any tests covering unions or enums. Comment at: clang/test/SemaTemp

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/include/clang/AST/TemplateBase.h:88 +/// so cannot be dependent. +UncommonValue, + erichkeane wrote: > I definitely hate the name here... Just `Value` makes a bit more sense, but > isn't perfectly accurate.

[Lldb-commits] [PATCH] D109231: [lldb] Improve error handling around GetRngListData()

shafik added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:522 +entry->getContribution(llvm::DW_SECT_RNGLISTS)) { + Offset = contribution->Offset; return DWARFDataExtractor(data, co

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

shafik added inline comments. Comment at: lldb/include/lldb/Target/AppleArm64ExceptionClass.h:14 + +enum class AppleArm64ExceptionClass : unsigned { +#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code, We should use fixed sized integer types whenever pos

[Lldb-commits] [PATCH] D110011: [lldb] Add --stack option to `target symbols add` command

shafik added a comment. Does it make sense to add a test for this? It looks like we have two basic tests for `add-dsym` already. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110011/new/ https://reviews.llvm.org/D110011 ___ lldb-commits mail

[Lldb-commits] [PATCH] D109928: [lldb] Remove IRExecutionUnit::CollectFallbackNames

shafik added a comment. I think this looks fine, just verify that this in a C++ case an expression with an `extern "C"` function call works fine e.g. extern "C" { int g() { return 10;} } int main() { return g(); // break here and run expr g() } I discovered the other day th

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

shafik added a comment. Just a nitpick. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3564 + // If we start a new block, compute a new block context and recurse. + Block *block = sc.function->GetBlock(true).FindBlockByID(die.GetID()); + if

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

shafik added inline comments. Comment at: lldb/include/lldb/Host/Terminal.h:20 + struct termios; I don't think we need this forward declaration anymore. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110721/new/ https://reviews.llvm.org/D110721

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

shafik added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:838 AttributeList AL = Attrs[i]; -for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) { +for (auto i : AL.indexes()) { AttributeSet AS = AL.getAttributes(i); -

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:674 + static RegularExpression g_swiftlang_version_regex( + llvm::StringRef(R"(swiftlang-([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?))")); `const`? CHANGES SINCE LAST AC

[Lldb-commits] [PATCH] D112058: [lldb/DWARF] Ignore debug info pointing to the low addresses

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:472 + InitializeFirstCodeAddress(*m_objfile_sp->GetModule()->GetSectionList()); + if (m_first_code_address == LLDB_INVALID_ADDRESS) +m_first_code_address = 0;

[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

shafik added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:186 void ScriptedThread::RefreshStateAfterStop() { - // TODO: Implement - if (m_reg_context_sp) -m_reg_context_sp->InvalidateAllRegisters(); + GetRegisterContext()->Invalidat

[Lldb-commits] [PATCH] D112167: [lldb/Plugins] Refactor ScriptedThread register context creation

shafik added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:99 ScriptedThread::CreateRegisterContextForFrame(StackFrame *frame) { uint32_t concrete_frame_idx = frame ? frame->GetConcreteFrameIndex() : 0; `const` Repos

[Lldb-commits] [PATCH] D112222: [LLDB] libcxx summary formatters for std::string_view

shafik added a comment. It looks good to me but can we verify it behaves nicely with undefined behavior cases e.g.: std::string s = "Hellooo "; std::string_view sv = s + "World\n"; and a case like the one that was fixed in D108228 I am not ex

[Lldb-commits] [PATCH] D112310: [lldb/DWARF] Don't create lldb_private::Functions for gc'ed DW_TAG_subprograms

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:866 + // discontiguous) + AddressRange func_range; + lldb::addr_t lowest_func_addr = ranges.GetMinRangeBase(0); You declare `func_range` here but don't use it ti

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

shafik added a comment. Nice catch! Comment at: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:21 +MAKE_VARS(); +MAKE_VARS(const); + It feels like writing out the decls by hand would have been quicker but perhaps less satisfying... Repo

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

shafik added a comment. I agree, we should be trying to get ride of inline tests. The last time I had to debug one it was not a fun experience. Maybe some of the recent improvements have helped there though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112708/new/ https://reviews.llvm.org/D112708 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D113174: [lldb] Summary provider for char flexible array members

shafik added a comment. In D113174#3109664 , @jingham wrote: > Regex Type summary matching happens after the ConstString matching. Enrico > did it that way because the ConstString matching is so much quicker, so if we > can find a match there we'll get

[Lldb-commits] [PATCH] D113330: [LLDB][Breakpad] Make lldb understand INLINE and INLINE_ORIGIN records in breakpad.

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:316 +Declaration callsite(callsite_file, record->CallSiteLineNum); +block_sp->SetInlinedFunctionInfo(name.str().c_str(), nullptr, nullptr, +

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

shafik added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:762 +static void fill_clamp(T &dest, U src, typename T::value_type fallback) { + dest = src <= std::numeric_limits::max() ? src +

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

shafik added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:762 +static void fill_clamp(T &dest, U src, typename T::value_type fallback) { + dest = src <= std::numeric_limits::max() ? src +

[Lldb-commits] [PATCH] D112147: [lldb] Fix lookup for global constants in namespaces

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3281-3282 if ((parent_tag == DW_TAG_compile_unit || - parent_tag == DW_TAG_partial_unit) && + parent_tag == DW_TAG_partial_unit || + parent_tag == DW_TA

[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

shafik added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:95 - // that Clang calls its internal Decl::AccessDeclContextSanity check. - decl->getAccess(); -#endif No, we don't care about the return value but we care about

[Lldb-commits] [PATCH] D113673: [lldb] Unwrap the type when dereferencing the value

shafik added a comment. I was trying to fix this a while back in D108717 but got distracted and did not get back to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113673/new/ https://reviews.llvm.org/D113673

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1026 +/*mangled_name=*/nullptr, func_ct, lldb::AccessType::eAccessPublic, +/*is_virtual=*/false, /*is_static=*/false, +/*is_inline=*/false, /*is_explicit=*

[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

shafik added a comment. So `(int*)100` can't be a constant expression b/c it is basically a `reinterpret_cast` and that is forbidden in a constant expression e.g.: constexpr const int* ip2 = (int*)100; is ill-formed. On the other hand this is a well-formed constant expression: static cons

[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

shafik added a comment. I am OOO for a bit but this makes sense and LGTM, let me think about it a bit more. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113498/new/ https://reviews.llvm.org/D113498 ___

[Lldb-commits] [PATCH] D112222: [LLDB] libcxx summary formatters for std::string_view

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Apologies, this dropped off my radar it LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11/new/ https://reviews.llvm.org/D112

[Lldb-commits] [PATCH] D114719: [lldb][NFC] Move non-clang specific method to the generic DWARF Parser

shafik accepted this revision. shafik added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114719/new/ https://reviews.llvm.org/D114719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D114668: [lldb][NFC] Move generic DWARFASTParser code out of Clang-specific code

shafik added a subscriber: clayborg. shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77 + default: + case DW_AT_abstract_origin: + case DW_AT_accessibility: ljmf00 wrote: > bulbazord wro

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

shafik added a comment. Is there a specific use case that motivated this feature? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114627/new/ https://reviews.llvm.org/D114627 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

shafik added a comment. I also echo Pavel's question, why not in `DWARFASTParser`? Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1140 : containing_decl_ctx, -GetOwningClangModule(die), name, clang_typ

[Lldb-commits] [PATCH] D114819: [lldb] Split TestCxxChar8_t

shafik added a comment. I recently ran into this, thanks for fixing the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114819/new/ https://reviews.llvm.org/D114819 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

shafik added a comment. Note, in both C and C++ converting a `-1` to unsigned will always result in the max unsigned value e.g.: #include #include int main() { int8_t i8 = -1; int32_t i32 = -1; unsigned x = i8; std::cout << x << "\n"; x = i32; std::cout <

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

shafik added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:733 + static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty, +SourceLocation Loc) { I think this makes sense but if we are go

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

shafik accepted this revision. shafik added a comment. LGTM Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39 const static auto char_min = std::numeric_limits::min(); const static auto uchar_min = std::numeric_limits::min(); We

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

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1534 + std::string qualified_name; + DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE(); + // TODO: change this to get the correct decl context parent -

[Lldb-commits] [PATCH] D135547: [lldb/Utility] Add GetDescription(Stream&) to StructureData::*

shafik added inline comments. Comment at: lldb/unittests/Utility/StructuredDataTest.cpp:41 + object_sp->GetDescription(S); + EXPECT_EQ(0, S.GetSize()); +} When doing a build I am seeing: ``` warning: comparison of integers of different signs: 'const int' and '

[Lldb-commits] [PATCH] D137003: [lldb-vscode] Send Statistics Dump in terminated event

shafik added a comment. Herald added a subscriber: JDevlieghere. It looks like this change broke `TestVSCode_terminatedEvent.py` see Green Dragon build bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/48111/ Please fix or revert. Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

shafik accepted this revision. shafik added a comment. was there a bug that inspired this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103107/new/ https://reviews.llvm.org/D103107 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

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

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h:117 +// (CU, (DIE)nullptr) == (nullptr, nullptr) -> true +if (!m_die.IsValid() && !it.m_die.IsValid()) + return true; I think: ``` bool operator==(const DWARFB

[Lldb-commits] [PATCH] D103158: [lldb][NFC] Use Language plugins in Mangled::GuessLanguage

shafik added a comment. I am curious how we would use this, for example `extern "C"` names won't fall under these categories, which may be fine depending on how we are using it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103158/new/ https://rev

[Lldb-commits] [PATCH] D103454: [lldb][docs] Document SBType

shafik added inline comments. Comment at: lldb/bindings/interface/SBType.i:338 +* C: Returns true for anonymous struct and unions. +* C++: Same as in C. +* Objective-C: Same as in C. Worth noting that anonymous structs are a GNU extension in C++. Re

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

shafik added inline comments. Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:169 + /// as a temporary. + bool m_hermetic; }; JDevlieghere wrote: > aprantl wrote: > > bool m_hermetic = false; > This is the second time someone has suggested to u

[Lldb-commits] [PATCH] D103504: Improve performance when parsing symbol tables in mach-o files.

shafik added inline comments. Comment at: lldb/source/Symbol/ObjectFile.cpp:623 ConstString file_name = GetModule()->GetFileSpec().GetFilename(); - ss.Printf("___lldb_unnamed_symbol%u$$%s", ++m_synthetic_symbol_idx, -file_name.GetCString()); - return ConstString(

[Lldb-commits] [PATCH] D103483: [lldb] Convert the default constructor’s member initializers into default member initializers

shafik added a comment. Thank you for doing this! This will be a big improvement. I am not done going through this change but I think it will require a bit more careful look though to make sure we are getting the maximum benefit from this refactor. Comment at: lldb/include/l

[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:104 + // We also return the FunctionNameType of each possible name. + std::vector> GetMethodNameVariants(ConstString method_name) const override; teemperor wrote: > Co

[Lldb-commits] [PATCH] D104041: [lldb] Replace default bodies of special member functions with = default;

shafik added a comment. I am sure I could find a bunch of other ends and odds but perhaps you have other checks that will cover these coming up. Comment at: lldb/include/lldb/Core/ThreadSafeValue.h:21 // Constructors and Destructors ThreadSafeValue() : m_value(), m_mute

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

shafik added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:392 + + if (result.Fail()) { +return result; What case is this catching? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104413/new/ http

[Lldb-commits] [PATCH] D104724: Modernize Module::RemapFile to return an Optional

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104724/new/ https://reviews.llvm.org/D104724 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Core/SourceManager.cpp:444 if (!FileSystem::Instance().Exists(m_file_spec)) { -FileSpec new_file_spec; // Check target sp

[Lldb-commits] [PATCH] D104406: Express PathMappingList::FindFile() in terms of PathMappingList.h::RemapPath() (NFC)

shafik added a comment. Looks good but would rather a second eye on this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104406/new/ https://reviews.llvm.org/D104406 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.

[Lldb-commits] [PATCH] D105038: [lldb/Interpreter] Fix session-save-on-quit when using ^D

shafik added inline comments. Comment at: lldb/source/Core/Debugger.cpp:611 + if (cmd_interpreter.GetSaveSessionOnQuit()) { +CommandReturnObject result(/*colors*/ true); +cmd_interpreter.SaveTranscript(result); `/*colors=*/true` Repository: rG LLVM G

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

shafik added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1082 + io_redirect_or_error = ScriptInterpreterIORedirect::Create( + options.GetEnableIO(), m_debugger, nullptr); + `/*result=*/nullp

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

shafik created this revision. shafik added reviewers: aprantl, teemperor, labath. Herald added a subscriber: arphaman. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Currently when we have a member function that has an auto return

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

shafik updated this revision to Diff 357578. shafik added reviewers: jingham, jasonmolenda. shafik added a comment. Changing approach based on Adrian's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564 Files: lldb/source/Plugins/Expr

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

shafik added a comment. @aprantl after your comments and discussion offline I changed my approach to do this lookup using the symbol table and it worked out. The main issue with the first approach was that gcc would also have to be updated in order for them to change their approach to generatin

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

shafik added a comment. In D105564#2867717 , @probinson wrote: >> Currently when we have a member function that has an auto return type and >> the definition is out of line we generate two DWARF DIE. >> One for the declaration and a second one for the de

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

shafik updated this revision to Diff 358373. shafik added a comment. - Modified `FindTypeForAutoReturnForDIE` to take into account if we have multiple symbols with the same name. - Modified `ParseSubroutine` to take into account that case we get the definition first, this can happen when we set

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

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:406 + virtual lldb::TypeSP + FindTypeForAutoReturnForDIE(const DWARFDIE &die, teemperor wrote: > If this is virtual then I guess `SymbolFileDWARFDwo` should overl

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

shafik updated this revision to Diff 358773. shafik marked 8 inline comments as done. shafik added a comment. - Removed virtual from `FindTypeForAutoReturnForDIE` - Added missing nullptr checks - Modernized the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://revie

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

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; break; JDevlieghere wrote: > What's the p

[Lldb-commits] [PATCH] D106348: Write the # of addressable bits into Mach-O corefiles, read it back out again

shafik added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5575-5613 +addr_t ObjectFileMachO::GetAddressMask() { + addr_t mask = 0; + ModuleSP module_sp(GetModule()); + if (module_sp) { +std::lock_guard guard(module_sp->GetMutex());

[Lldb-commits] [PATCH] D106712: Remember to check whether the current thread is stopped for a no-stop signal

shafik added inline comments. Comment at: lldb/source/Target/Process.cpp:780 StopReason curr_thread_stop_reason = eStopReasonInvalid; -if (curr_thread) { +bool prefer_curr_thread = false; +if (curr_thread && curr_thread->IsValid()) { -

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3024 -m_index->GetGlobalVariables( -dwarf_cu->GetNonSkeletonUnit(), [&](DWARFDIE die) { - VariableSP var_sp( So is the problem that

[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)

shafik added a comment. Thank you for refactoring this, besides my comment on the lambda the rest makes sense. Comment at: lldb/source/Expression/IRExecutionUnit.cpp:785 + auto get_external_load_address = + [target, &symbol_was_missing_weak]( + lldb::addr_t &loa

[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction

shafik added inline comments. Comment at: lldb/source/API/SBModule.cpp:401-403 +ModuleFunctionOptions function_options; +function_options.include_symbols = true; +function_options.include_inlines = true; bulbazord wrote: > nit: IMO this looks cleaner,

[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)

shafik accepted this revision. shafik added a comment. LGTM Comment at: lldb/source/Expression/IRExecutionUnit.cpp:842 + Target *m_target; + bool &m_symbol_was_missing_weak; + lldb::addr_t m_best_internal_load_address = LLDB_INVALID_ADDRESS; Normally I would

[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction

shafik accepted this revision. shafik added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107295/new/ https://reviews.llvm.org/D107295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

shafik added inline comments. Comment at: lldb/source/Core/IOHandler.cpp:374 + if (got_line) { +goto gotLine; + } A `goto` is not warranted here, using `if (!got_line && !in` is perfectly fine and less lines. CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

shafik created this revision. shafik added reviewers: teemperor, aprantl. shafik requested review of this revision. I was debugging a problem and noticed that it would have been helpful to have the type of each `FieldDecl` when looking at the output from `ClangASTSource::layoutRecordType` http

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

shafik 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; This is undefined behavior

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

shafik added a comment. In D108257#2950994 , @aprantl wrote: > If getType() doesn't trigger type completion then this LGTM! AFAICT the call to `getType` end up in `ValueDecl`: QualType getType() const { return DeclType; } and this is just returning a

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

shafik created this revision. shafik added reviewers: aprantl, teemperor, werat. Herald added a subscriber: arphaman. shafik requested review of this revision. D103532 modified this case to preserve type sugar but we can end up with cases where the cast is not v

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

shafik added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6505 if (idx_is_valid) { - const clang::ReferenceType *reference_type = - llvm::cast(GetQualType(type).getTypePtr()); - CompilerType pointee_clang_type = -

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

shafik added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6508 + if (parent_type_class == clang::Type::LValueReference) +pointee_clang_type = GetLValueReferenceType(type).GetPointeeType(); + else teempero

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

shafik added a comment. In D108717#2965815 , @teemperor wrote: > Thanks for the patch! > > So IIUC correctly this fixes a crash when calling `Dereference` on an SBValue > that is of type `SomeTypedef` with `typedef int& SomeTypedef`? If yes, then I > th

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

shafik updated this revision to Diff 368935. shafik added a comment. Applying formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108257/new/ https://reviews.llvm.org/D108257 Files: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp Index: lldb/source/Plugins/Expres

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2a4a498a884a: [LLDB] Add type to the output for FieldDecl when logging in ClangASTSource… (authored by shafik). Herald added a project: LLDB. Reposi

[Lldb-commits] [PATCH] D74187: [lldb] Add method Language::IsMangledName

shafik added subscribers: mib, shafik. shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:96-98 + bool IsMangledName(llvm::StringRef name) const override { +return false; + } xiaobai wrote: > friss wrote: > > The orig

[Lldb-commits] [PATCH] D73946: [lldb] Fix another instance where we pass a nullptr as TypeSourceInfo to NonTypeTemplateParmDecl::Create

shafik accepted this revision. shafik added a comment. Thank you fixing this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73946/new/ https://reviews.llvm.org/D73946 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

shafik added a comment. I will try to look into `dwarfdump --verify` separately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73921/new/ https://reviews.llvm.org/D73921 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://list

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

shafik updated this revision to Diff 243678. shafik marked 5 inline comments as done. shafik added a comment. Herald added a reviewer: jdoerfert. Updated approach based on comments and added test for the new approach. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73921/new/ https://revi

[Lldb-commits] [PATCH] D74310: [lldb] Don't model std::atomic as a transparent data structure in the data formatter

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM otherwise. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py:70 +# This should

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

shafik added a comment. I can see how stripping `__1` would be nice but I seeing `(anonymous namespace)` may be useful to know especially b/c it effects visibility and linkage. It would be nicer if could make this optional and default it off but be able to turn it back on. Repository: rLLDB

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. If we still see the info using `-R` then I am happy but Jim's concerns are valid, they won't match the bracktrace. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.o

<    1   2   3   4   5   6   7   8   >