[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

shafik added inline comments. Comment at: source/Interpreter/CommandInterpreter.cpp:2733 + + const char *k_space_characters = "\t\n\v\f\r "; + size_t first_non_space = line.find_first_not_of(k_space_characters); This looks like duplicate code from from `HandleC

[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

shafik added inline comments. Comment at: source/Interpreter/CommandInterpreter.cpp:96 +{"echo-comment-commands", OptionValue::eTypeBoolean, true, true, nullptr, + {}, "If true, LLDB will print a command even if it is a pure comment " + "line."}}; ---

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik created this revision. shafik added reviewers: jingham, davide, aprantl. Herald added a reviewer: EricWF. - Adding support to step into the callable wrapped by libc++ std::function - Adding test to validate that functionality https://reviews.llvm.org/D52851 Files: include/lldb/Target/C

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik updated this revision to Diff 168389. shafik marked 6 inline comments as done. shafik added a comment. Addressing comments and fixing a bug in ThreadPlanStepThrough.cpp where I would overwrite the result of objc_runtime. https://reviews.llvm.org/D52851 Files: include/lldb/Target/CPPL

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik updated this revision to Diff 169105. shafik marked 5 inline comments as done. shafik added a comment. - Addressing comments - Adding test to make sure we step through a std::function wrapping a member variable https://reviews.llvm.org/D52851 Files: include/lldb/Target/CPPLanguageRunt

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik added a comment. @jingham I believe I addressed your comments, please review again. https://reviews.llvm.org/D52851 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik updated this revision to Diff 169259. shafik marked an inline comment as done. shafik added a comment. Removing uneeded code and making test NO_DEBUG_INFO_TESTCASE https://reviews.llvm.org/D52851 Files: include/lldb/Target/CPPLanguageRuntime.h packages/Python/lldbsuite/test/lang/cpp

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

shafik added a comment. @jingham removed code and made it NO_DEBUG_INFO_TESTCASE https://reviews.llvm.org/D52851 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

This revision was automatically updated to reflect the committed changes. Closed by commit rL344371: Adding support to step into the callable wrapped by libc++ std::function (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik marked 2 inline comments as done. shafik added a comment. @stella.stamenova Thank you for catching this. I fixed the test names, I am looking into the best way to fix the skipif now. https://reviews.llvm.org/D49271 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/decorators.py:194 py_version[0], py_version[1], sys.version_info) -skip_for_macos_version = (macos_version is None) or ( +skip_for_macos_version = (macos_version is None) or (pl

[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y

shafik added a comment. +1 for modernizing code! Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2262 + // assertion in the call to clang_type.TransferBaseClasses() + for (auto &base_class : bases) { clang::TypeSourceInfo

[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string

shafik created this revision. shafik added reviewers: jingham, davide. Herald added a reviewer: EricWF. Herald added a subscriber: christof. We currently support formatting for std::string and std::wstring but not std::u16string and std::u32string which was added with C++11. This adds support fo

[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule

shafik added inline comments. Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1405 -llvm::StringRef version_part; - -if (sdk_name.startswith(sdk_strings[(int)desired_type])) { - version_part = - sdk_name.drop_front(strlen(sdk_strings[(int)des

[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.

shafik added a comment. LGTM https://reviews.llvm.org/D53709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string

This revision was automatically updated to reflect the committed changes. Closed by commit rL345402: [DataFormatters] Adding formatters for libc++ std::u16string and std::u32string (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec

shafik added a comment. Are we refactoring in the right place? Why not just refactor `FileSpec::GetByteSize()` why is `FileSpec` the wrong place? Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:718 + (FileSystem::Instance().GetByteSize(file) - file

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik created this revision. shafik added reviewers: jingham, davide, teemperor. Herald added a subscriber: JDevlieghere. Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter. Both calling sites of the sites

[Lldb-commits] [PATCH] D54074: CPlusPlusLanguage: Use new demangler API to implement type substitution

shafik added inline comments. Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:336 + llvm::itanium_demangle::Node *parseType() { +if (llvm::StringRef(First, Last - First).startswith(Search)) { + Result += llvm::StringRef(Written, First - Written); ---

[Lldb-commits] [PATCH] D54074: CPlusPlusLanguage: Use new demangler API to implement type substitution

shafik added a comment. Much better, thank you! Repository: rL LLVM https://reviews.llvm.org/D54074 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik added a comment. @zturner I don't see `AddEnumerationValueToEnumerationType` being called in `SymbolFile/NativePDB.cpp` https://github.com/llvm-mirror/lldb/search?q=AddEnumerationValueToEnumerationType&unscoped_q=AddEnumerationValueToEnumerationType https://reviews.llvm.org/D54003 __

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik added a comment. @zturner I was out of date when I posted this diff but I have that file updated locally and it will show up when I update the diff. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik updated this revision to Diff 173083. shafik marked 8 inline comments as done. shafik added a comment. Made changes based on comments. https://reviews.llvm.org/D54003 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefil

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:909 clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - lldb::opaque_compiler_type_t type, - const CompilerType &enumerator_qual_type, const Declaration &decl, - const c

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik added a comment. @jingham @zturner @teemperor @clayborg I believe I have addressed all the comments. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/ll

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rL346428: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove… (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

shafik added a comment. @zturner I would not be against discussing using pass by value for small objects going forward. I don't currently have a good feeling for at what sizes/data types the right trade-off is at though. Repository: rL LLVM https://reviews.llvm.org/D54003 ___

[Lldb-commits] [PATCH] D54452: [NativePDB] Add support for handling S_CONSTANT records

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:329 +std::pair GetIntegralTypeInfo(TypeIndex ti, TpiStream &tpi) { + if (ti.isSimple()) { lemo wrote: > Just a suggestion: I'm not a big fan of returning

[Lldb-commits] [PATCH] D54528: [lldb] NFC: Remove the extra ';'

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, thank you for fixing this. Repository: rLLDB LLDB https://reviews.llvm.org/D54528 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()

shafik added subscribers: stella.stamenova, shafik. shafik added a comment. Hello, This PR broke the green dragon bots, see the following log file: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/13655/consoleFull#-15796076f80f5c9c-2aaa-47fb-b15d-be39b7128d72 I was about to revert it bu

[Lldb-commits] [PATCH] D55584: Simplify boolean expressions

shafik added a comment. I find the `static_cast` to be a bit too clever, I don't think it helps readability. This is also too large to digest in a way I would feel satisfied that I did not miss anything. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55584/new

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

shafik accepted this revision as: shafik. shafik added a comment. LGTM after comment are addressed. Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5894 +if (!success) { + offset = MachHeaderSizeFromMagic(m_header.magic); + for (uint32_t i = 0; !s

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik created this revision. shafik added reviewers: jingham, friss, jasonmolenda. Herald added subscribers: christof, mgorny. Herald added a reviewer: EricWF. Add formattors for libc++ std::optional and tests to verify the new formattors. https://reviews.llvm.org/D49271 Files: lldb.xcodepro

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 155508. shafik marked 7 inline comments as done. shafik edited the summary of this revision. shafik added a comment. Address comments - Applying clang-formt - Refactoring OptionalFrontEnd to produce out that makes the underlying data look like an array - Remo

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik added a comment. @davide @jingham Thank you for the review, those were good catches and comments. I have addressed them except for refactoring the test to use lldbInline which I will be working on now. Comment at: packages/Python/lldbsuite/test/functionalities/data-fo

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 155618. shafik added a comment. Removing unrelated changes due to applying clang-format to patch file with context. https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 155758. shafik added a comment. Refactoring test to use lldbinline https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile package

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik marked 5 inline comments as done and 3 inline comments as done. shafik added a comment. @jingham @davide I believe I have addressed all your comments https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 156886. shafik marked 4 inline comments as done. shafik added a comment. Addressing comments - -O0 is not needed in Makefile - engaged is not friendly terminology so switching to "Has Value" - Switching test away from lldbinline style due to bug w/ .categories

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik marked 6 inline comments as done. shafik added a comment. @davide One more pass Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8 + +lldbinline.MakeInlineTest(__file__, globa

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 157337. shafik added a comment. Adding additional comments https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile packages/Python

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 158121. shafik added a comment. Adjust test to filter on clang version and libc++ version to precent build bots from failing due to lack of C++17 or lack of optional support https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik added a comment. @davide @labath I believe I have addressed both the C++17 filter and the libc++ filter. Please let me know if you have further comments. https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.o

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 158342. shafik marked 2 inline comments as done. shafik added a comment. - Adding comments - Changing from exit to self.skipTest - Adding skip for gcc https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/f

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21 +@add_test_categories(["libc++"]) +@skipIf(oslist=no_match(["macosx"]), compiler="clang", com

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 158362. shafik added a comment. Fixing gcc skipIf to check for version as well https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefil

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 158662. shafik added a comment. Simplifying initialization of has_optional in test. https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Ma

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik marked 4 inline comments as done. shafik added a comment. @labath Addressed comment, thank you for helping out. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21 +@add_t

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

shafik created this revision. shafik added reviewers: jingham, davide. Herald added a reviewer: EricWF. Herald added a subscriber: christof. Adding formatter summary for std::function. - Added LibcxxFunctionSummaryProvider - Removed LibcxxFunctionFrontEnd - Modified data formatter tests

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

shafik updated this revision to Diff 161837. shafik marked 12 inline comments as done. shafik added a comment. Fixes based on comments - Switched to early exits - Added a lot more comments to explain all the cases being dealt with and noting which cases different sections are dealing with http

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

shafik added a comment. @jingham @vsk I believe I have addressed most of your comments Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58 + + if (process != nullptr) { +Status status; jingham wrote: > vsk wrote: > > Please use an early exit here,

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

shafik updated this revision to Diff 161997. shafik added a comment. Updating comment https://reviews.llvm.org/D50864 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py packages/Python/lldbsuite/test/functionalitie

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } Is this saying that an empty parameter pack is invalid?

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

shafik created this revision. shafik added reviewers: jingham, aprantl, davide. lldb::StateType is an enum without an explicit underlying type. According to [dcl.enum]p8 http://eel.is/c++draft/dcl.enum#8 and [expr.static.cast]p10 http://eel.is/c++draft/expr.static.cast#10 we are limited to set

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

shafik added a comment. Very nice! Do we have a way of verifying the performance gain? Comment at: source/Breakpoint/BreakpointResolver.cpp:183 llvm::StringRef log_ident) { - Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_L

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik created this revision. shafik added reviewers: jingham, davide. Herald added subscribers: christof, mgorny. Herald added a reviewer: EricWF. Adding data formatter for libc++ std::variant and appropriate tests https://reviews.llvm.org/D51520 Files: lldb.xcodeproj/project.pbxproj pack

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

shafik marked 3 inline comments as done. shafik added a comment. @jingham I am switching to the @aprantl suggestions which feels cleaner and removes this issue. Comment at: include/lldb/lldb-enumerations.h:60 ///< or threads get the chance to run. + kNumSt

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

shafik updated this revision to Diff 163450. shafik marked an inline comment as done. shafik added a comment. Switching enum guard to kLastStateType which references the last valid enum which lead to cleaner code instead of inventing a new value which does not have a good meaning in many cases.

[Lldb-commits] [PATCH] D51557: Replace uses of LazyBool with LazyBool template

shafik added inline comments. Comment at: include/lldb/DataFormatters/ValueObjectPrinter.h:146 + +#define LLDB_VOP ValueObjectPrinter + LazyBoolMember m_should_print; davide wrote: > can you typedef? I feel like using ... = is cleaner Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

shafik created this revision. shafik added a reviewer: jingham. Moving the core functionality of std::function formatter into CPPLanguageRuntime in preparation for future changes to allow us to step into the wrapped callable of std::function. This will prevent code duplication since both functi

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

This revision was automatically updated to reflect the committed changes. Closed by commit rL341957: Remove undefined behavior around the use of StateType (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51445?vs=16

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB341957: Remove undefined behavior around the use of StateType (authored by shafik, committed by ). Herald added a subscriber: abidh. Repository: rLLDB LLDB https://reviews.llvm.org/D51445 Files:

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

shafik updated this revision to Diff 164962. shafik added a comment. Removing change that was meant for the next PR https://reviews.llvm.org/D51896 Files: include/lldb/Target/CPPLanguageRuntime.h source/Plugins/Language/CPlusPlus/LibCxx.cpp source/Target/CPPLanguageRuntime.cpp Index: sou

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

This revision was automatically updated to reflect the committed changes. Closed by commit rL341991: Refactoring std::function formatter to move core functionality into… (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.o

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik updated this revision to Diff 165201. shafik marked 9 inline comments as done. shafik added a comment. Address comments on std::variant formatter - Adding tests for reference to variant and variant of variant - Refactoring test to be more efficient - Refactoring unsafe code that assumed si

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik added a comment. @vsk @jingham I believe I have addressed your comments, please review again. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp:29 +std::variant v3; +std::variant v_no_value; + --

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik added a comment. @vsk Interesting I ran git clang-format before generating the diff and it made changes, so I am not sure what happened https://reviews.llvm.org/D51520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.ll

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

shafik added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:153 +static OptionDefinition g_dependents_options[1] = { +{LLDB_OPT_SET_1, false, "no-dependents", 'd', + OptionParser::eOptionalArgument, nullptr, g_dependents_enumaration, 0, -

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik updated this revision to Diff 165536. shafik added a comment. Applying clang-format https://reviews.llvm.org/D51520 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile packages/Python/lldbs

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik added a comment. @teemperor Thanks for the catch! https://reviews.llvm.org/D51520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik updated this revision to Diff 165540. shafik added a comment. Applying clang-format to files I missed earlier https://reviews.llvm.org/D51520 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefil

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

This revision was automatically updated to reflect the committed changes. Closed by commit rL342421: [DataFormatters] Add formatter for C++17 std::variant (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51520?vs=16

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

shafik created this revision. shafik added a reviewer: jingham. SBFrame should be a thin wrapper around the core functionality in StackFrame. Currently FindVariable() core functionality is implemented in SBFrame and this will move that into StackFrame. This is step two in enabling stepping into

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

shafik updated this revision to Diff 166144. shafik added a comment. Updating LibcxxVariantGetIndexValidity() to no longer do type check of __index. It was left over from the old method of checking for an empty variant and was also breaking clang 5. https://reviews.llvm.org/D51520 Files: ll

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

shafik updated this revision to Diff 166194. shafik marked 6 inline comments as done. shafik added a comment. Addressing comments: - Adding documentation to FindVariable() - Using ConstString instead of const char * https://reviews.llvm.org/D52247 Files: include/lldb/Target/StackFrame

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

shafik added a comment. @jingham @clayborg @davide addressed comments w/ the exception of the lambda one which I politely disagree with. Comment at: source/Target/StackFrame.cpp:1733-1738 +if (sc.block->AppendVariables( +can_create, get_parent_variables, stop_

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

shafik updated this revision to Diff 166308. shafik added a comment. Adjusting documentation based on feedback https://reviews.llvm.org/D52247 Files: include/lldb/Target/StackFrame.h source/API/SBFrame.cpp source/Target/StackFrame.cpp Index: source/Target/StackFrame.cpp =

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

This revision was automatically updated to reflect the committed changes. Closed by commit rL342663: Refactor FindVariable() core functionality into StackFrame out of SBFrame (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

shafik added a comment. Sorry for the late review Comment at: lldb/trunk/include/lldb/Target/ProcessStructReader.h:70 } -size_t total_size = struct_type.GetByteSize(nullptr); -lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0)); +auto total_size = s

[Lldb-commits] [PATCH] D57222: Simplify LangOpts initalization in ClangExpressionParser [NFC]

shafik added a comment. Nice improvement in readability! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57222/new/ https://reviews.llvm.org/D57222 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://li

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

shafik created this revision. shafik added reviewers: aprantl, jingham, teemperor, martong. Herald added a subscriber: rnkovacs. Herald added a reviewer: serge-sans-paille. When we are creating a ClassTemplateSpecializationDecl in ParseTypeFromDWARF(...) we are not handling the case where variadi

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

shafik updated this revision to Diff 184173. shafik marked 19 inline comments as done. shafik added a comment. Addressed comments, including - Renamed test - Reduced test size - Stream-lined code in CreateTemplateParameterList(...) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57363/new

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

shafik added a comment. @aprantl @teemperor @davide thank you for the review, some great catches. I believe I have addressed the comments. Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile:1 +LEVEL = ../../make + aprantl wr

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:789-792 + assert(encodedBits.repr.unused == 0); + decodedBits.repr.sign = encodedBits.repr.sign; + decodedBits.repr.fraction = encodedBits.repr.fraction; + decodedBits.repr.exponent = decode

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

shafik added a comment. In D57413#1376280 , @davide wrote: > To clarify, I think we ought to fix the UB regardless, but Zachary's change > can go in anyway as it doesn't make the situation worse than it was before. It does not, I was more comment on the

[Lldb-commits] [PATCH] D57467: Fix use of non-existing variable in crashlog.py

shafik created this revision. shafik added reviewers: aprantl, jingham. Herald added a reviewer: serge-sans-paille. The method find_matching_slice(self) uses `uuid_str` on one of the paths but the variable does not exist and so this results in a NameError exception if we take that path. https:

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

This revision was automatically updated to reflect the committed changes. Closed by commit rL352677: Fix handling of CreateTemplateParameterList when there is an empty pack (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llv

[Lldb-commits] [PATCH] D57467: Fix use of non-existing variable in crashlog.py

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB352772: Fix use of non-existing variable in crashlog.py (authored by shafik, committed by ). Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57467/new/ https://reviews.

[Lldb-commits] [PATCH] D58090: [WIP] Deserialize Clang module search path from DWARF

shafik added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:266 - for (size_t ci = 1; ci < path.size(); ++ci) { -llvm::StringRef component = path[ci].GetStringRef(); + for (size_t ci = 1; ci < module.path.size(); ++ci) { +

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

shafik added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/basic/main.cpp:1 +#include + Looks like you are relying on `iostream` to bring in `cstdlib` which is not guaranteed. Comment

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

shafik added a comment. Looks like a great fix! Comment at: MIDataTypes.h:67 +template +class MIDereferenceable { +public: Can we use `llvm::Optional` instead? Then we can use `getValueOr()`. Comment at: MIDataTypes.h:71 + + T *Or(T &alt_va

[Lldb-commits] [PATCH] D58273: Fix TestDataFormatterLibcxxListLoop.py test

shafik added a comment. Thanks for fixing this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58273/new/ https://reviews.llvm.org/D58273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

shafik created this revision. shafik added reviewers: teemperor, aprantl, jingham, martong. Herald added subscribers: jdoerfert, rnkovacs. Herald added a reviewer: serge-sans-paille. This tests a fix in the ASTImpoter.cpp to ensure that we import built-in correctly, see differential: https://rev

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

shafik updated this revision to Diff 188780. shafik marked 5 inline comments as done. shafik added a comment. Addressing comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58790/new/ https://reviews.llvm.org/D58790 Files: packages/Python/lldbsuite/test/expression_command/import_

[Lldb-commits] [PATCH] D58792: Add "operator bool" to SB APIs

shafik added a comment. It stood out to me that some of the conversions were not `const` and I can see that `IsValid` is not consistently `const` across the API but after talking to @jingham it is unfortunately something we can't change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58

[Lldb-commits] [PATCH] D58792: Add "operator bool" to SB APIs

shafik added a comment. In D58792#1415390 , @clayborg wrote: > In D58792#1414964 , @labath wrote: > > > In D58792#1414191 , @shafik wrote: > > > > > It stood out to me that s

[Lldb-commits] [PATCH] D59004: [lldb] Fix DW_OP_addrx uses.

shafik added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:2910 //-- // OPCODE: DW_OP_GNU_addr_index // OPERANDS: 1 Should this comment be updated? Repo

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355525: Adding test to cover the correct import of SourceLocation pertaining to a built… (authored by shafik, committed by ). Herald added a project: LLDB. Repository: rLLDB LLDB CHANGES SINCE LAST

[Lldb-commits] [PATCH] D59040: Move ExpressionSourceCode.cpp -> ClangExpressionSourceCode.cpp

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM although there are some good comments by @aprantl Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59040/new/ https://reviews.llvm.org/D59040 __

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

shafik added a comment. Herald added a subscriber: ormris. It looks like this commit introduced undefined behavior via a misaligned load see this log from lldb sanitized bot on green dragon http://lab.llvm.org:8080/green/view/LLDB/job/lldb-sanitized/2050/testReport/junit/lldb-Suite/functionaliti

  1   2   3   4   5   6   7   8   >