[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik created this revision. shafik added reviewers: jingham, aprantl, clayborg, teemperor, davide. The change D55575 modified `ClangASTContext::CreateParameterDeclaration` to call `decl_ctx->addDecl(decl);` this caused a regression since the existing code in

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik updated this revision to Diff 212231. shafik added a comment. Addressing comments: - Simplifying Makefile - Adding comments to the test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/new/ https://reviews.llvm.org/D65414 Files: packages/Python/lldbsuite/test/lang/cpp/bre

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik added a comment. @aprantl @clayborg Thank you for the comments, I think I have addressed your concerns. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/new/ https://reviews.llvm.org/D65414 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik updated this revision to Diff 212243. shafik added a comment. Simplifying Makefile even more CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/new/ https://reviews.llvm.org/D65414 Files: packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_param

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik added a comment. @stella.stamenova this could potentially break the windows build, could you please verify before I land this change. Thank you in advance! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/new/ https://reviews.llvm.org/D65414 ___

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik added a comment. @stella.stamenova that is unfortunate but not surprising. I don't have a way to test a fix locally. Is there anyone who might be able to help me iterate over a fix or maybe a new maintainer of the PDB parsing? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik updated this revision to Diff 212884. shafik added a comment. Fix that should fix the failing PDB test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65414/new/ https://reviews.llvm.org/D65414 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsuite/test/lang/

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

shafik added a comment. @labath @stella.stamenova I updated the diff with a fix that I believe should address the test failure. This is an unfortunate difference in how DWARF and PDB works. After spending some time digging into this I don't see the key difference that allows DWARF to work w/o

[Lldb-commits] [PATCH] D65666: Temporarily disable libc++ std::function formatter due to performance issue

shafik created this revision. shafik added reviewers: aprantl, jasonmolenda, JDevlieghere. Herald added a reviewer: EricWF. Herald added a subscriber: christof. We have been seeing increased reports of performance issue around large project and formatting std::function variables especially in fun

[Lldb-commits] [PATCH] D65666: Temporarily disable libc++ std::function formatter due to performance issue

shafik updated this revision to Diff 213090. shafik added a comment. Adding as suggested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65666/new/ https://reviews.llvm.org/D65666 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/

[Lldb-commits] [PATCH] D65666: Temporarily disable libc++ std::function formatter due to performance issue

shafik added a comment. @aprantl I did not know either until I looked at decorators.py and it says: # @skipIf, skip for all platform/compiler/arch, CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65666/new/ https://reviews.llvm.org/D65666

[Lldb-commits] [PATCH] D65666: Temporarily disable libc++ std::function formatter due to performance issue

This revision was automatically updated to reflect the committed changes. Closed by commit rL367701: [Formatters] Temporarily disable libc++ std::function formatter due to… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL367726: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[Lldb-commits] [PATCH] D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType

shafik created this revision. shafik added reviewers: aprantl, teemperor. Currently the heuristic used in `ClangASTContext::CreateRecordType` to identify an anonymous class is that there is that `name` is a `nullptr` or simply a null terminator. This heuristic is not accurate since it will also

[Lldb-commits] [PATCH] D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType

shafik added a comment. @aprantl @teemperor Good catches, I have updated the review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66175/new/ https://reviews.llvm.org/D66175 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://

[Lldb-commits] [PATCH] D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType

shafik updated this revision to Diff 215228. shafik marked 7 inline comments as done. shafik added a comment. Addressing comments: - Rewording comments - Moving test location - Adding test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66175/new/ https://reviews.llvm.org/D66175 Fi

[Lldb-commits] [PATCH] D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType

shafik updated this revision to Diff 215243. shafik marked 6 inline comments as done. shafik added a comment. Fixing `has_name` to better reflect the condition. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66175/new/ https://reviews.llvm.org/D66175 Files: packages/Python/lldbsuite/

[Lldb-commits] [PATCH] D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType

This revision was automatically updated to reflect the committed changes. Closed by commit rL368937: Improve anonymous class heuristic in ClangASTContext::CreateRecordType (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[Lldb-commits] [PATCH] D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints

shafik added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointOptions.h:120 + /// BreakpointOptions(const char *condition, bool enabled = true, int32_t ignore = 0, bool one_shot = false, You have a lot of `bool` paramet

[Lldb-commits] [PATCH] D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints

shafik added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointOptions.h:120 + /// BreakpointOptions(const char *condition, bool enabled = true, int32_t ignore = 0, bool one_shot = false, mib wrote: > shafik wrote: > > Y

[Lldb-commits] [PATCH] D66250: [JIT][Unwinder] Add Trampoline ObjectFile and UnwindPlan support for FCB

shafik added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:3561 + if (UnwindPlanSP plan_sp = func_unwinders_sp->GetTrampolineUnwindPlan()) { +result.GetOutputStream().Printf("Trampoline UnwindPlan:\n"); Curious I notice

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

shafik added a comment. In D66447#1638783 , @labath wrote: > In D66447#1638047 , @JDevlieghere > wrote: > > > In D66447#1637640 , @labath wrote: > > > > > This looks good to

[Lldb-commits] [PATCH] D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

shafik added inline comments. Comment at: lldb/source/Expression/UserExpression.cpp:276 keep_expression_in_memory, generate_debug_info); + if (parse_success) { Is this just white space? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION htt

[Lldb-commits] [PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

shafik accepted this revision. shafik added a comment. Other than my two comment this LGTM Comment at: clang/lib/AST/ASTImporter.cpp:3452 << FoundField->getType(); - - return make_error(ImportError::NameConflict); + ConflictingDecls.push_back(FoundField);

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

shafik added inline comments. Comment at: llvm/include/llvm/Support/MathExtras.h:271 unsigned char out[sizeof(Val)]; - std::memcpy(in, &Val, sizeof(Val)); + memcpy(in, &Val, sizeof(Val)); for (unsigned i = 0; i < sizeof(Val); ++i) What are you building on

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

shafik created this revision. shafik added reviewers: jingham, jasonmolenda, aprantl. Herald added a reviewer: EricWF. Herald added subscribers: ldionne, christof. Performance issues lead to the libc++ `std::function` formatter to be disabled, see D65666 This ch

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

shafik updated this revision to Diff 218797. shafik marked 2 inline comments as done. shafik added a comment. Addressing comments - Using log timers to verify whether we are using the cache or not - Switched to llvm::StringMap CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67111/new/ ht

[Lldb-commits] [PATCH] D67227: [lldb] Extend and document TestIRInterpreter.py

shafik added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/commands/expression/ir-interpreter/TestIRInterpreter.py:54 +# Shifting longer than size of a type also doesn't work. +if rhs.value <= 0 or rhs.value >= 7: +

[Lldb-commits] [PATCH] D67227: [lldb] Extend and document TestIRInterpreter.py

shafik added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/commands/expression/ir-interpreter/TestIRInterpreter.py:54 +# Shifting longer than size of a type also doesn't work. +if rhs.value <= 0 or rhs.value >= 7: +

[Lldb-commits] [PATCH] D67227: [lldb] Extend and document TestIRInterpreter.py

shafik added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/commands/expression/ir-interpreter/TestIRInterpreter.py:48 +self.type = type +self.decl_expr = type + " " + self.name + " = " + str(self.value) +self.unsigned_type =

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

shafik updated this revision to Diff 219604. shafik marked an inline comment as done. shafik added a comment. - Refactored test to use a function to do repetitive task CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67111/new/ https://reviews.llvm.org/D67111 Files: packages/Python/lld

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

shafik updated this revision to Diff 219621. shafik marked 2 inline comments as done. shafik added a comment. - Fixing test based on comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67111/new/ https://reviews.llvm.org/D67111 Files: packages/Python/lldbsuite/test/functionalitie

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

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py:31 +self.expect("log timers dump", + patterns=["(?!lldb_private::Module::FindSymbolsMatchin

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

shafik updated this revision to Diff 219633. shafik marked 3 inline comments as done. shafik added a comment. Changes based on comments: - Removed extra lookup - Refactored redundant code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67111/new/ https://reviews.llvm.org/D67111 Files:

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

shafik created this revision. shafik added reviewers: labath, JDevlieghere, xiaobai, aam, amccarth. Herald added a subscriber: aprantl. shafik marked an inline comment as done. shafik added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3060 + +

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

shafik marked an inline comment as done. shafik added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3060 + +switch (tag) { + case DW_TAG_array_type: I added this change because currently when end up trying to parse non-

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

shafik updated this revision to Diff 221622. shafik added a comment. - Formatting code - Removing FunctionDecl case since it was not correct CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67994/new/ https://reviews.llvm.org/D67994 Files: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF

[Lldb-commits] [PATCH] D68005: Make dw_tag_t a llvm::dwarf::Tag

shafik added a comment. Thanks for doing this! I spent a bunch of time trying to understand that relationship yesterday. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68005/new/ https://reviews.llvm.org/D68005 ___ ll

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

shafik added a comment. In D67994#1682051 , @labath wrote: > For dumping a specific type something like this could be right, but for > "indiscriminately dumping" everything, this seems to be a bit fragile. > > Would it be possible to make this use the `Sy

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

shafik marked an inline comment as done. shafik added inline comments. Comment at: tools/lldb-test/lldb-test.cpp:552-579 + +lldb_private::TypeList type_list; +size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list); +printf( "Type list size: %zu\n", n

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

shafik marked an inline comment as done. shafik added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3077 + ParseType(sc, die, &type_is_new).get(); + printf( "pubname: %s is_type = %d\n", die.GetPubname(), true); + brea

[Lldb-commits] [PATCH] D66451: [ClangExpressionParser] Add ClangDeclVendor

shafik added inline comments. Herald added a subscriber: usaxena95. Comment at: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:110 do { - DeclVendor *modules_decl_vendor = - m_target->GetClangModulesDeclVendor(); + auto *modules_decl_

[Lldb-commits] [PATCH] D60588: Adjusting libc++ std::list formatter to act better with pointers and references and adding a test to cover a previous related fix

This revision was automatically updated to reflect the committed changes. Closed by commit rL359118: [DataFormatters] Adjusting libc++ std::list formatter to act better with… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik created this revision. shafik added reviewers: jingham, aprantl, teemperor, rsmith. This will fix a bug where during expression parsing we are not setting a `CXXRecordDecl` to not be passed in registers and the resulting code generation is wrong. The DWARF attribute `DW_CC_pass_by_refere

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added a comment. @rsmith I tagged you in this change in case we are missing any implications in using `DW_CC_pass_by_reference` to do `setArgPassingRestrictions(clang::RecordDecl::APK_CannotPassInRegs);` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.l

[Lldb-commits] [PATCH] D61266: Skip TestClassTemplateParameterPack.py on all platforms

shafik created this revision. shafik added reviewers: jingham, aprantl, teemperor. The `TestClassTemplateParameterPack.py` test does not work for the right reasons. The expressions such as: expression -- C().isSixteenThirtyTwo() work only because we are currently pulling all the local variabl

[Lldb-commits] [PATCH] D61266: Skip TestClassTemplateParameterPack.py on all platforms

shafik added a comment. @friss updated the change to only effect those specifically broken. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61266/new/ https://reviews.llvm.org/D61266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D61266: Skip TestClassTemplateParameterPack.py on all platforms

shafik updated this revision to Diff 197178. shafik added a comment. Fred is correct, I mistakenly thought the parts of the test that were working were being covered elsewhere but that is not the case. So I have reworked this change to instead of skipping the whole test to comment out the inline

[Lldb-commits] [PATCH] D61266: Skip TestClassTemplateParameterPack.py on all platforms

shafik updated this revision to Diff 197360. shafik added a comment. Updated comment to be more precise. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61266/new/ https://reviews.llvm.org/D61266 Files: packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTem

[Lldb-commits] [PATCH] D61266: Skip TestClassTemplateParameterPack.py on all platforms

shafik added a comment. @friss makes sense, updated comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61266/new/ https://reviews.llvm.org/D61266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D61305: Add std::stack and std::queue support to CxxModuleHandler

shafik accepted this revision. shafik added a comment. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61305/new/ https://reviews.llvm.org/D61305 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https:/

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/argument_passing_restrictions/main.cpp:22 + bounds.y = 2; + return; +} aprantl wrote: > what's the point of the return? This is vestigial return left over f

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik updated this revision to Diff 197426. shafik marked 9 inline comments as done. shafik added a comment. Changed to reflect comments. - Added comments to test to explain what it is doing. - Formatting and other minor fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/

[Lldb-commits] [PATCH] D61299: Rename Minion to ASTImporterDelegate

shafik accepted this revision. shafik added a comment. This is a good change! Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:259 public: - CxxModuleScope(Minion &minion, clang::ASTContext *dst_ctx) + CxxModuleScope(ASTImporterDelegate &minion, clang::AST

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added a comment. @aprantl @teemperor I believe I addressed your comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://list

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added a comment. @teemperor good call, that is indeed simpler and yes I did not intend that delete. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 ___ lldb-commits mailing list lldb-commits@

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik updated this revision to Diff 197618. shafik added a comment. - Simplifying test - Fixing unintended deleted test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 Files: packages/Python/lldbsuite/test/expression_command/argument_passing

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik updated this revision to Diff 197628. shafik added a comment. Testing both passing as and argument and returning CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 Files: packages/Python/lldbsuite/test/expression_command/argument_passing_

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added a comment. @friss added second test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listi

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/argument_passing_restrictions/TestArgumentPassingRestrictions.py:32 +self.expect("expr takePassByRef(p)", +substrs=['(int)', '= 11223344']) friss wrote: >

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

shafik updated this revision to Diff 197636. shafik marked an inline comment as done. shafik added a comment. Modifying copy contructor to be act more intuitively. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61146/new/ https://reviews.llvm.org/D61146 Files: packages/Python/lldbsui

[Lldb-commits] [PATCH] D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF

This revision was automatically updated to reflect the committed changes. Closed by commit rL359732: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pr

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

shafik added a comment. Thank you! LGTM, in general we avoid "large" refactoring changes to avoid polluting the blame list but the changes are relatively local and they are good changes that can catch real bugs in the future. I would like a second set of eyes though. Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik added a comment. @teemperor @jingham @clayborg I believe now that https://reviews.llvm.org/D46551 is landed the performance concerns should be addressed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik updated this revision to Diff 198061. shafik added a comment. Updating after https://reviews.llvm.org/D46551 landed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 Files: packages/Python/lldbsuite/test/expression_command/namespace_loca

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

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 rL359921: Fix for ambiguous lookup in expressions between local variable and namespace (authored by shafik, committed by ).

[Lldb-commits] [PATCH] D61759: Switch to FindSymbolsMatchingRegExAndType() from FindFunctions() in FindLibCppStdFunctionCallableInfo()

shafik created this revision. shafik added reviewers: friss, jingham, jasonmolenda. FindLibCppStdFunctionCallableInfo() currently uses FindFunctions() in order to find a lambdas `operator()()` but using `FindSymbolsMatchingRegExAndType()` is cheaper and if we also anchor the regex using `^` this

[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

shafik created this revision. shafik added reviewers: friss, jingham, aam. We track down a crash in FindLibCppStdFunctionCallableInfo() to a missing `nullptr` check for the `symbol` variable. https://reviews.llvm.org/D61805 Files: source/Target/CPPLanguageRuntime.cpp Index: source/Target/C

[Lldb-commits] [PATCH] D61759: Switch to FindSymbolsMatchingRegExAndType() from FindFunctions() in FindLibCppStdFunctionCallableInfo()

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB360599: [DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses… (authored by shafik, committed by ). Herald added a project: LLDB. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

shafik updated this revision to Diff 199649. shafik marked 4 inline comments as done. shafik added a comment. Simplified the checking of symbol being a `nullptr` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61805/new/ https://reviews.llvm.org/D61805 Files: source/Target/CPPLanguageR

[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

shafik added a comment. @friss we have several bugs, once of which I can reproduce but I have not been able to reduce it to a minimal case yet and the nullptr check is obviously the right to do. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61805/new/ https://reviews.llvm.org/D61805

[Lldb-commits] [PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

shafik added a comment. I ran `check-lldb` and I hit one regression in `TestFormatters.py`, part of what I am seeing is as follows: AssertionError: False is not True : FileCheck'ing result of `expression --show-types -- *(new foo(47))` Error output: error: no matching constructor for init

[Lldb-commits] [PATCH] D62061: Add AST logging

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM outside of Jonas's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62061/new/ https://reviews.llvm.org/D62061 __

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

shafik created this revision. shafik added reviewers: martong, teemperor, jasonmolenda, friss. Herald added a subscriber: rnkovacs. https://reviews.llvm.org/D51633 added error handling to the `ASTNodeImporter::VisitRecordDecl` for the conflicting names case. This could lead to erroneous return o

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

shafik added a comment. @davide We have a reproducer but so far it has proven difficult to narrow it down to a test case. This is serious regression b/c this leads to fields being dropped in records during expression evaluation leading to results that looks like a bug but are actually expressio

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

This revision was automatically updated to reflect the committed changes. Closed by commit rL361650: [ASTImporter] Call to HandleNameConflict in VisitRecordDecl mistakeningly using… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[Lldb-commits] [PATCH] D57880: Add assert for 'bad' code path in GetUniqueNamespaceDeclaration

shafik added inline comments. Comment at: lldb/trunk/source/Symbol/ClangASTContext.cpp:1957 } else { -// BAD!!! +assert(false && "GetUniqueNamespaceDeclaration called with no name and " +"no namespace as decl_ctx");

[Lldb-commits] [PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

shafik added a comment. This LGTM now but I will wait for @teemperor to take a look at it. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:620 if (predicate(decl->getKind())) { if (log) { I think a comment on the `predica

[Lldb-commits] [PATCH] D64777: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory

shafik created this revision. shafik added a reviewer: teemperor. In `ClangASTContext::CreateFunctionTemplateSpecializationInfo` a `TemplateArgumentList` is allocated on the stack but is treated as if it is persistent in subsequent calls. When we exit the function `func_decl` will still point t

[Lldb-commits] [PATCH] D64777: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory

shafik marked 5 inline comments as done. shafik added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp:1 +template +struct M {}; labath wrote: > JDevlieghere wrote: > > labath wro

[Lldb-commits] [PATCH] D64777: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory

shafik updated this revision to Diff 210127. shafik added a comment. Applying clang-format to test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64777/new/ https://reviews.llvm.org/D64777 Files: packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_

[Lldb-commits] [PATCH] D64858: [lldb] Make log for ClangModulesDeclVendor's compiler flag less verbose

shafik added a comment. Is it worth it to write a test that verifies the output? Otherwise LGTM. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64858/new/ https://reviews.llvm.org/D64858 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D64777: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory

This revision was automatically updated to reflect the committed changes. Closed by commit rGa0858e2f20c8: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack… (authored by shafik). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D64777?v

[Lldb-commits] [PATCH] D64995: [lldb] Fix crash when tab-completing in multi-line expr

shafik added a comment. Also once we get one test going then it should be easy to add coverage for all sorts of scenarios. Who knows maybe we will find more bugs. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64995/new/ https://reviews.llvm.org/D64995 _

[Lldb-commits] [PATCH] D91118: Fix handling of bit-fields in a union

shafik created this revision. shafik added reviewers: teemperor, aprantl. shafik requested review of this revision. When laying out bit-fields we don't properly take into account when they are in a union, they will all have a zero offset. https://reviews.llvm.org/D91118 Files: lldb/source/Pl

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

shafik added a comment. So this work for primitive types like `int*` etc? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77153/new/ https://reviews.llvm.org/D77153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.or

[Lldb-commits] [PATCH] D91118: Fix handling of bit-fields in a union

shafik updated this revision to Diff 304354. shafik added a comment. Add a check to make sure the bit offset is zero for fields in a union. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91118/new/ https://reviews.llvm.org/D91118 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTPar

[Lldb-commits] [PATCH] D91118: Fix handling of bit-fields in a union

This revision was automatically updated to reflect the committed changes. Closed by commit rGbae9aedb341c: [LLDB] Fix handling of bit-fields in a union (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

shafik added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/test/Shell/Register/x86-fp-write.test:2 # XFAIL: system-windows -# REQUIRES: native && target-x86 +# REQUIRES: native && (target-x86 || target-x86_64) # RUN: %clangxx_host %p/Inputs/x86-fp-w

[Lldb-commits] [PATCH] D92223: [lldb] Add support for looking up static const members

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3181 + if (tag == DW_TAG_member && !const_value_form.IsValid()) { +// Allows only members with DW_AT_const_value attribute, i.e. static const This is a nitpi

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

shafik added inline comments. Comment at: lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py:25 -deque_type = "std::deque >" size_type = deque_type + "::size_type" Why do the default arguments not show up

[Lldb-commits] [PATCH] D92510: [lldb] set created function decl to public access in TypeSystemClang

shafik added a comment. +1 to what @teemperor said Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92510/new/ https://reviews.llvm.org/D92510 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik added a comment. I am not sure if I need to worry about `ValueObject::IsLogicalTrue(...)` as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93421/new/ https://reviews.llvm.org/D93421 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik added a comment. After chatting with Raphael offline, we realize that `BOOL` will indeed always be signed. I was under the impression for some reason that it was actually unsigned. So the fix is simpler is that we just need to make the formatter use `GetValueAsSigned(...)` CHANGES SINC

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik updated this revision to Diff 312636. shafik added a comment. Updating diff to reflect comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93421/new/ https://reviews.llvm.org/D93421 Files: lldb/source/Plugins/Language/ObjC/Cocoa.cpp lldb/test/API/functionalities/data-fo

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik marked 4 inline comments as done. shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:1038 } - uint8_t value = (real_guy_sp->GetValueAsUnsigned(0) & 0xFF); + uint8_t value = (real_guy_sp->GetValueAsSigned(0) & 0xFF); switch (value)

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik updated this revision to Diff 312904. shafik marked 2 inline comments as done. shafik added a comment. - Fixed formatters::ObjCBOOLSummaryProvider to use int8_t and fixed the Printf - Updated tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93421/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D93479: [lldb] Simplify the is_finalized logic in process and make it thread safe.

shafik added inline comments. Comment at: lldb/include/lldb/Target/Process.h:2837 + /// from looking up or creating things during or after a finalize call. + std::atomic m_finalizing; + ``` std::atomic m_finalizing{false}; ``` honestly the whole thing should b

[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

shafik added a comment. We can have unscoped enums in namespace and class scope and the enumerators won't leak out from those scopes. Thus we can have shadowing going on e.g.: #include enum GEnum {eOne=2,}; namespace A { enum AEnum {eOne=0,}; void g() {std::cout << eOne;

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

shafik added a comment. landed this fix: https://reviews.llvm.org/rGdc057e87f6c18c24d17c7cae97ebe95f78b6d934 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93421/new/ https://reviews.llvm.org/D93421 ___ lldb-commits mailing list lldb-commits@

<    1   2   3   4   5   6   7   8   >