[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. I was testing out how `NSStringSummaryProvider` deals w/ `NULL` using `NSString *foo = nullptr` and we filter out `NULL` values in `ValueObjectPrinter::GetValueSummaryError(...)` : if (IsNil()) summary.assign("nil"); That should mean that the `text->GetValueAs

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik updated this revision to Diff 279965. shafik added a comment. Removing `text->GetValueAsUnsigned(0) == 0` check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84272/new/ https://reviews.llvm.org/D84272 Files: lldb/source/Plugins/Language/ObjC/Cocoa.cpp Index: lldb/source/Plu

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. In D84272#2168172 , @davide wrote: > why? Do you have a testcase? There are already tests but we have a crash report w/o a reproducer, these are obviously correct checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. I updated to include the radar number. So we have a crash in `NSStringSummaryProvider(...)` when calling `valobj.GetProcessSP()` and we have what looks like a `nullptr` e.g. `0x0068` In this specific case we are coming from `NSBundleSummaryProvider(...)` and

[Lldb-commits] [PATCH] D84285: Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Expression/Materializer.cpp:283 (uint64_t)mem, - (unsigned long long)m_persistent_variable_sp->GetByteSize())

[Lldb-commits] [PATCH] D84440: [llb] Remove the user-defined copy-ctor in ConstString

shafik added inline comments. Comment at: lldb/include/lldb/Utility/ConstString.h:45 /// Initializes the string to an empty string. ConstString() : m_string(nullptr) {} Since your touching this type, why not modernize it and use an in class member initia

[Lldb-commits] [PATCH] D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition

shafik updated this revision to Diff 280291. shafik added a comment. Updating diff since the parent landed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83972/new/ https://reviews.llvm.org/D83972 Files: clang/lib/AST/ASTImporter.cpp lldb/test/API/lang/objc/bitfield_ivars/TestBitfi

[Lldb-commits] [PATCH] D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition

This revision was automatically updated to reflect the committed changes. Closed by commit rG0db2934b0fa9: [ASTImporter] Modify ImportDefiniton for ObjCInterfaceDecl so that we always… (authored by shafik). Herald added projects: clang, LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rG6700f4b9fe63: [LLDB] Add checks for ValueObjectSP in Cocoa summary providers (authored by shafik). Herald added a projec

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik created this revision. shafik added reviewers: aprantl, jingham, vsk. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. When bit-field data was stored in a `Scalar` in `ValueObjectChild` during `UpdateValue()` it was extracti

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added a comment. Note: for the test I did not want to rely on clang choosing to pass the union by register, so I used assembly which ensures I will obtain the behavior I am looking to capture for the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added a subscriber: JDevlieghere. shafik added inline comments. Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205 -if (m_bitfield_bit_size) - scalar.ExtractBitfield(m_bitfield_bit_size, - m_bitfield_bit_o

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 283647. shafik added a comment. Removing use of `-O1` from Makefile. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectChild.cpp lldb/test/API/functionalities/data-formatt

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 283658. shafik added a comment. - Refactored the Makefile and test based on offline comments from Adrian. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectChild.cpp lldb/

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added inline comments. Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205 -if (m_bitfield_bit_size) - scalar.ExtractBitfield(m_bitfield_bit_size, - m_bitfield_bit_offset); -else ---

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added inline comments. Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205 -if (m_bitfield_bit_size) - scalar.ExtractBitfield(m_bitfield_bit_size, - m_bitfield_bit_offset); -else ---

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 283703. shafik added a comment. - Add more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectChild.cpp lldb/test/API/functionalities/data-formatter/valueobj-pass-by-

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 284507. shafik added a comment. Updated to use llvm.org clang, remove header files from the main.c and add the commit hash for the clang so that it is easier to replicate the main.s in the future. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added a comment. In D85376#2209638 , @labath wrote: > This manifested itself for variables in registers because those end up being > described as `eValueTypeScalar`, is that so? > > If that's the case, then I think this would also manifest for vari

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik added a comment. We found a way to hand modify the assembly and it looks good, I just need to convert it to a test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 285176. shafik added a comment. Replacing python test with Shell test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectChild.cpp lldb/test/Shell/SymbolFile/DWARF/valueobj

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 285233. shafik marked 4 inline comments as done. shafik added a comment. Updated test using more compact code from Fred. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectCh

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

shafik updated this revision to Diff 285423. shafik marked an inline comment as done. shafik added a comment. Update test name CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 Files: lldb/source/Core/ValueObjectChild.cpp lldb/test/Shell/Symbo

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG25bbceb047a3: [LLDB] Fix how ValueObjectChild handles bit-fields stored in a Scalar in… (authored by shafik). Herald added

[Lldb-commits] [PATCH] D85904: [lldb] Check Decl kind when completing -flimit-debug-info types

shafik accepted this revision. shafik added a comment. Minor comment but LGTM, nice fix! Comment at: lldb/test/API/functionalities/limit-debug-info/main.cpp:34 + int member = 47; +} shadowed_one; + Just to clarify we `func_shadow::One` is being shadowed here.

[Lldb-commits] [PATCH] D86140: [lldb] Add typedefs to the DeclContext they are created in

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. This is nice catch! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86140/new/ https://reviews.llvm.org/D86140 _

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

shafik created this revision. shafik added reviewers: aprantl, friss, labath, jingham, vsk. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. In some cases when we have a `DW_AT_const_value` and the data can be found in the `DWARFEx

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

shafik added a comment. Thank you finding these additional fixed, it looks good but I don't know some of the details as well as Adrian so I would prefer he gives the LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86348/new/ https://reviews.ll

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

shafik marked an inline comment as done. shafik added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:2 +# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s +# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s + -

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

shafik added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix shows this having this additional s

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

shafik added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } labath wrote: > davide wrote: > > shafik wrote: > > > Wouldn't it be better to turn `m_byte_size` into an `Optio

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

shafik updated this revision to Diff 287482. shafik marked 5 inline comments as done. shafik added a comment. -Add REQUIRES x86 - Fix-up the comments in the test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86311/new/ https://reviews.llvm.org/D86311 Files: lldb/source/Core/ValueObj

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

shafik added inline comments. Comment at: lldb/source/Core/ValueObjectVariable.cpp:136 +if (expr.GetExpressionData(m_data)) { + if (m_data.GetDataStart() && m_data.GetByteSize()) +m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize()); la

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

This revision was automatically updated to reflect the committed changes. Closed by commit rG93b255142bb7: [LLDB] Fix how ValueObjectVariable handles DW_AT_const_value when the… (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik created this revision. shafik added reviewers: martong, teemperor. Herald added a subscriber: rnkovacs. shafik requested review of this revision. When we fixed `ImportDeclContext(...)` in D71378 to make sure we complete each `FieldDecl` of a `RecordDecl`

[Lldb-commits] [PATCH] D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes

shafik accepted this revision. shafik added a comment. LGTM with minor comments. Thank you for these fixes, they are awesome! Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3213 + bool use_type_size_for_value = false; + if (location_form.IsValid(

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik marked 2 inline comments as done. shafik added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRe

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik marked an inline comment as done. shafik added a subscriber: rsmith. shafik added a comment. @martong I have been experimenting w/ how we pass around `ImportDefinitionKind` and pushing it through to more places does not help. I also tried defaulting most locations to `IDK_Everything` to e

[Lldb-commits] [PATCH] D87441: Speedup collecting DWARF attribute values

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:83 + +AttributeValue(const DWARFUnit *_cu, dw_offset_t _die_offset, + dw_attr_t _attr, dw_form_t _form) aprantl wrote: > We usually don't prefix

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik added a comment. Herald added a reviewer: JDevlieghere. We are going to move forward with this approach (after dealing with the multi-dimensional array case) temporarily. We are seeing crash bugs from this from users and we want to fix it while we explore the solution space more. This PR

[Lldb-commits] [PATCH] D87640: Add '<' meta command to read in code from external file

shafik added inline comments. Comment at: lldb/source/Expression/REPL.cpp:199 +error_sp->Printf("file at path '%s' too large: " + "file_size = %llu, max_size = %llu\n", + path.c_str(), file_size, max_size); `%llu` is no

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik updated this revision to Diff 292607. shafik added a comment. Fix handling of multi-dimensional arrays. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86660/new/ https://reviews.llvm.org/D86660 Files: clang/lib/AST/ASTImporter.cpp lldb/test/API/commands/expression/codegen-cr

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rG6807f244fa67: [ASTImporter] Modifying ImportDeclContext(...) to ensure that we also handle… (authored by shafik). Herald added projects: clang, LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[Lldb-commits] [PATCH] D88792: [lldb/test] Catch invalid calls to expect()

shafik added a comment. I am happy this was not hiding any regressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88792/new/ https://reviews.llvm.org/D88792 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D88992: [lldb] Fix "frame var" for large bitfields

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM I notice that we are using `m_byte_offset` directly a little above the line you fixed instead of `GetByteOffset()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[Lldb-commits] [PATCH] D89351: Move initialization of Variable::m_loc_is_const_data into constructor (NFC)

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:823 scope, comp_unit.get(), ranges, &decl, location, is_external, false, - false); - var_sp->SetLocationIsConstantValueData(false); + false, false);

[Lldb-commits] [PATCH] D89589: [lldb] Implement ObjCExceptionThrowFrameRecognizer::GetName()

shafik added a comment. test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89589/new/ https://reviews.llvm.org/D89589 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi

[Lldb-commits] [PATCH] D67589: Fix crash on SBCommandReturnObject & assignment

shafik added a comment. @jankratochvil it looks like this PR broke the green dragon build: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/1926/ The specific test that is failing is `TestFunctionStarts.py` see the more detailed log here: http://lab.llvm.org:8080/green/view/LLDB/job/lld

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

shafik added a comment. So if I look at `ClangASTContext::AddMethodToCXXRecordType(...)` it has the following: if (is_artificial) return nullptr; // skip everything artificial but why? if I look at the godbolt w/ an AST dump it is not creating as a global f

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

shafik added a comment. In D68130#1686549 , @jingham wrote: > > IIRC, it is because we can't count on which artificial functions get > generated in the debug information in any given CU. It depends on what gets > used. If we write artificial func

[Lldb-commits] [PATCH] D68140: [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

shafik added inline comments. Comment at: clang/include/clang/AST/ExternalASTMerger.h:92 ImporterTarget Target; + std::shared_ptr SharedState; Can you add a comment explaining what this is and why we need it and how it relates to the `ASTImpoter`. It is n

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

shafik added inline comments. Comment at: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:387 std::string expected_packet1 = - R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)"; + R"(jTraceStart:{"buffersize":8192,"metab

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

shafik added a comment. The fix makes sense to me, I wonder if we have a similar issue w/ namespaces or perhaps they are handled differently. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68278/new/ https://reviews.llvm.org/D68278 __

[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

shafik added inline comments. Comment at: clang/include/clang/AST/ExternalASTMerger.h:95 public: -ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM) -: AST(_AST), FM(_FM), OM(_OM) {} +ImporterSource(ASTContext &_AST, FileManager &_FM, cons

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik updated this revision to Diff 222861. shafik retitled this revision from "[WIP] Modify lldb-test to print out ASTs from symbol file" to "Modify lldb-test to print out ASTs from symbol file". shafik added a comment. - Updated approach based on comment - Pushed clang ast manipulation into `C

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik added a comment. In D67994#1683934 , @labath wrote: > In D67994#1683440 , @shafik wrote: > > > I believe this is due to us being lazy as to when we import. > > > Yes, but doesn't calling `Module::ParseAllDebu

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik marked 7 inline comments as done. shafik added a comment. I ended up splitting out the functionality into a new method `dumpClangAST` it looks PDB is not as lazy as DWARF and there are several PDB tests already using the current `dumpAST` as it is. Comment at: tools/ll

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik updated this revision to Diff 222935. shafik marked 14 inline comments as done. shafik added a comment. Updates based on comments: - Remove use of AsCString() - Fixed use of old API - other minor issues CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67994/new/ https://reviews.llv

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

shafik added a comment. Thank you for doing this, this looks like an excellent start from a quick review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68422/new/ https://reviews.llvm.org/D68422 ___ lldb-commits mailing list lldb-commits@l

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik added a comment. In D67994#1692645 , @labath wrote: > Maybe this is my fault since I'm the one who introduced the first bunch of > arguments here IIRC, but anyway, I have a feeling all of these dumping > options are getting out of hand. Looking at

[Lldb-commits] [PATCH] D68354: [platform process list] add a flag for showing the processes of all users

shafik added a comment. @wallace it looks like `TestPlatformClient.py` is failing on osx see green dragon log here: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/2317/testReport/junit/lldb-Suite/functionalities_gdb_remote_client/TestPlatformClient_py/ ===

[Lldb-commits] [PATCH] D68354: [platform process list] add a flag for showing the processes of all users

shafik added a comment. I reverted this change b/c the green dragon bots have been red for too long. When you fix the problem please make sure you add Differential Revision: https://reviews.llvm.org/D68354 to your commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D67520: Add pretty printing of Clang "bitfield" enums

shafik added a comment. See the same break on green dragon as well: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/2360/testReport/LLDB/SymbolFile_NativePDB/s_constant_cpp/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67520/new/ https://

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

shafik accepted this revision. shafik added a comment. LGTM Comment at: source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp:34 std::vector m_elements; - ValueObjectSP m_first; + ValueObject* m_first = nullptr; CompilerType m_bool_type; I feel like each o

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik added a comment. In D67994#1697981 , @labath wrote: > In D67994#1695172 , @shafik wrote: > > > In D67994#1692645 , @labath wrote: > > > > > Maybe this is my fault sinc

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

shafik added inline comments. Comment at: lldb/include/lldb/API/SBBreakpointLocation.h:59 + void SetScriptCallbackFunction(const char *callback_function_name, + SBStructuredData &extra_args); + Could this be a `const &`

[Lldb-commits] [PATCH] D68533: Explicitly set entry point arch when it's thumb [Second Try]

shafik added a comment. `SymbolFile/Breakpad.symtab.test` is failing on green dragon cmake build see: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/2378/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68533/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

shafik added a comment. This change broek the`TestDataFormatterInvalidStdUniquePtr.py` test, see: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/2411/testReport/ I verified that reverting this commit fixes the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D68010: [lldb] Fix string summary of an empty NSPathStore2

shafik added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:544 bool is_truncated = false; const auto max_size = process_sp->GetTarget().GetMaximumSizeOfStringSummary(); Why not just make this `uint32_t` making this `auto` gain

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik updated this revision to Diff 224479. shafik marked 2 inline comments as done. shafik added a comment. - Simplified DumpFromSymbolFile() based on comments - Reused -name option in lldb-test and to replace -symbol-name CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67994/new/ https

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

shafik added inline comments. Comment at: source/Symbol/ClangASTContext.cpp:9195 + GetAsEnumDecl(type->GetFullCompilerType())) + enum_decl->dump(s.AsRawOstream()); +else { teemperor wrote: > shafik wrote: > > aprantl wrote: > > > is an En

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

This revision was automatically updated to reflect the committed changes. Closed by commit rG5f46982b452b: [lldb-test] Modify lldb-test to print out ASTs from symbol file (authored by shafik). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D67994?vs=224479&id=22

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik created this revision. shafik added reviewers: aprantl, teemperor. We add support for `DW_AT_export_symbols` to detect anonymous struct on top of the heuristics implemented in D66175 This should allow us to differentiate anonymous structs and unnamed stru

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik added a comment. In D68961#1709708 , @clayborg wrote: > Have many compilers supported DW_AT_export_symbols for a while now? If not, > are there any serious issues introduced here that would change debugger > behavior if this attribute is not emitt

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik updated this revision to Diff 225508. shafik added a comment. Minor fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68961/new/ https://reviews.llvm.org/D68961 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsuite/test/python_api/type/TestTypeList.py p

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik added a comment. In D68961#1711537 , @labath wrote: > In D68961#1711407 , @shafik wrote: > > > In D68961#1709708 , @clayborg > > wrote: > > > > > Have many compilers

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68961/new/ https://reviews.llvm.org/D68961 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik added a comment. In D68961#1714241 , @labath wrote: > In D68961#1713566 , @shafik wrote: > > > We also have the lldb-cmake-matrix > > bot whi

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik updated this revision to Diff 226349. shafik added a comment. -Add test to clarify what we depending wrt to debug info CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68961/new/ https://reviews.llvm.org/D68961 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsu

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik updated this revision to Diff 226366. shafik added a comment. - Updated test to add dwarfdump test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68961/new/ https://reviews.llvm.org/D68961 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsuite/test/python_api/

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

shafik updated this revision to Diff 226450. shafik marked an inline comment as done. shafik added a comment. Using llvm-dwarfdump instead of dwarfdump CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68961/new/ https://reviews.llvm.org/D68961 Files: include/lldb/Symbol/ClangASTContext.

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

This revision was automatically updated to reflect the committed changes. Closed by commit rGde2c7cab715e: Add support for DW_AT_export_symbols for anonymous structs (authored by shafik). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D68961?vs=226450&id=226764#

[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

shafik created this revision. shafik added reviewers: aprantl, jingham. clang has a mangling extension for block invocations and currently lldb does not handle it correctly. https://reviews.llvm.org

[Lldb-commits] [PATCH] D69820: [Symbol] Add TypeSystem::GetClassName

shafik added a comment. As far as I can tell we don't have coverage for `GetBaseClassPath` it would be good to add it while we are refactoring this code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69820/new/ https://reviews.llvm.org/D69820 _

[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

shafik updated this revision to Diff 227786. shafik marked 12 inline comments as done. shafik added a comment. Updating based on comments - Adding documentation - using startswith(...) - Updating initialization to use = instead of {} CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69738/n

[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

shafik added inline comments. Comment at: lldb/source/Core/Mangled.cpp:99 + + if (s.size() >= 2 && (s[0] == '_' && s[1] == 'Z')) +return Mangled::eManglingSchemeItanium; aprantl wrote: > jingham wrote: > > StringRef has a startswith. That might be easier to

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik created this revision. shafik added reviewers: jasonmolenda, aam, jingham. Herald added a subscriber: christof. This PR depends on D67111 Performance issues lead to the libc++ std::function formatter to be disabled. We addressed some of those performance

[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

This revision was automatically updated to reflect the committed changes. shafik marked 2 inline comments as done. Closed by commit rG83393d27af66: [LLDB] Fix handling for the clang name mangling extension for block invocations (authored by shafik). Herald added a project: LLDB. Repository: rG

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rGe18f4db208ba: [LLDB] Adding caching to libc++ std::function formatter for lookups that… (authored by shafik). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D67111?vs=21

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik updated this revision to Diff 228533. shafik marked 6 inline comments as done. shafik added a comment. After updating branch the stepping test started failing. It was failing because the __invoke case was subtly broken in this patch. Fixing that case opened up a lot of refactoring opportu

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik added a comment. @aprantl you will want to take a second look at this, I did some major refactoring. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69913/new/ https://reviews.llvm.org/D69913 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik updated this revision to Diff 228774. shafik added a comment. - Adjust comment - matching_lambda argument is now `FunctionSP` to match `ForeachFunction` signature. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69913/new/ https://reviews.llvm.org/D69913 Files: lldb/include/lld

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik marked an inline comment as done. shafik added a comment. In D69913#1739579 , @jingham wrote: > Why not have the FindFunctions lambda take a FunctionSP? It would be easy to > get the Function name out of the function in the lambda, and that would

[Lldb-commits] [PATCH] D69704: [lldb] Add IsTypeSystemCompatible method to SBModule to allow checking compatibility between language versions

shafik added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/python_api/module_compability/TestModuleDebugInfoCompatible.py:61 +self.assert_compatible(exe_module, lldb.eLanguageTypeC_plus_plus_11) +self.assert_compatible(exe_module, lldb.eLanguag

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

shafik updated this revision to Diff 228923. shafik added a comment. Applying clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69913/new/ https://reviews.llvm.org/D69913 Files: lldb/include/lldb/Symbol/CompileUnit.h lldb/packages/Python/lldbsuite/test/functionalities/dat

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

This revision was automatically updated to reflect the committed changes. Closed by commit rG91e94a7015f1: [LLDB][Formatters] Re-enable std::function formatter with fixes to improve non… (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D70449: Add a "Using LLDB" section to the welcome page of the website

shafik added inline comments. Comment at: lldb/docs/index.rst:25 +:ref:`LLDB Tutorial `. For users already familiar with +other debuggers there is also a cheat sheet listing common tasks and +their LLDB equivalent in the :ref:`GDB to LLDB command map `. `other de

[Lldb-commits] [PATCH] D70449: Add a "Using LLDB" section to the welcome page of the website

shafik added inline comments. Comment at: lldb/docs/status/goals.rst:38 + +In order to achieve our goals we decided to start with a fresh architecture +that would support modern multi-threaded programs, handle debugging symbols in Is it worth adding anything abou

[Lldb-commits] [PATCH] D70448: [LLDB] Fix wrong argument in CommandObjectThreadStepWithTypeAndScope

shafik added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:529 m_step_type(step_type), m_step_scope(step_scope), m_options(), -m_class_options("scripted step", 'C') { +m_class_options("scripted step", true, 'C') { CommandA

[Lldb-commits] [PATCH] D64917: Add offsetof support to expression evaluator.

shafik added a comment. Thanks for fixing this one! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64917/new/ https://reviews.llvm.org/D64917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

<    1   2   3   4   5   6   7   8   >