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

2018-07-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] [PATCH] D49271: Adding libc++ formattors for std::optional

2018-07-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
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",

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

2018-07-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-01 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-01 Thread Shafik Yaghmour via Phabricator via lldb-commits
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 +

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

2018-07-25 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-16 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-08-21 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-08-21 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-22 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-16 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-16 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-15 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-07-23 Thread Shafik Yaghmour via Phabricator via lldb-commits
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/

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

2018-07-23 Thread Shafik Yaghmour via Phabricator via lldb-commits
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__,

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

2018-08-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D51557: Replace uses of LazyBool with LazyBool template

2018-08-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-09-10 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-08-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
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. +

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

2018-08-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D49271: Adding libc++ formattors for std::optional

2018-07-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

2018-07-13 Thread Shafik Yaghmour via Phabricator via lldb-commits
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 -

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

2018-07-13 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-10-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-10-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-10-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-10-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-10-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-10-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-10-23 Thread Shafik Yaghmour via Phabricator via lldb-commits
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 _class : bases) { clang::TypeSourceInfo

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

2018-10-26 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-10-25 Thread Shafik Yaghmour via Phabricator via lldb-commits
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 = -

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

2018-10-25 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

2018-11-01 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D53788: [FileSystem] Remove GetByteSize() from FileSpec

2018-10-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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) -

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

2018-11-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D54074: CPlusPlusLanguage: Use new demangler API to implement type substitution

2018-11-05 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

2018-11-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
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_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

2018-11-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-11-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-11-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:909 clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - lldb::opaque_compiler_type_t type, - const CompilerType _qual_type, const Declaration , - const char *name,

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

2018-11-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-11-08 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-11-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-11-13 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:329 +std::pair GetIntegralTypeInfo(TypeIndex ti, TpiStream ) { + 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 ';'

2018-11-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

2018-10-04 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-10-02 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-10-02 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-10-10 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-10-10 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-10-03 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-19 Thread Shafik Yaghmour via Phabricator via lldb-commits
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,

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

2018-09-19 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-20 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-09-20 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2019-01-15 Thread Shafik Yaghmour via Phabricator via lldb-commits
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 =

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

2018-12-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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;

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

2018-12-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-12-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-09-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-09-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D51445: Remove undefined behavior around the use of StateType

2018-09-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D51520: Add libc++ data formatter for std::variant

2018-09-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-09-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-09-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-09-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-09-17 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-18 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2018-09-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-11 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2018-09-13 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-27 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment. In D59847#1443905 , @friss wrote: > As we're just adding test coverage, could we add a little more? > > - Anonymous enum > - Enum through a typedef > - class enum > - enum declared inside of the function rather than at the

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment. In D59847#1446571 , @stella.stamenova wrote: > This is causing failures on the windows bot. Please fix it or revert it. > > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/3066 I think I know why this is

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment. @stella.stamenova I committed a fix, please let me know if this does not address the regression: http://llvm.org/viewvc/llvm-project?view=revision=357210 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59847/new/

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357188: Regression test to ensure that we handling importing of std::vector of enums… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

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

2019-03-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision. shafik added reviewers: jingham, teemperor. Herald added a subscriber: jdoerfert. In an Objective-C context a local variable and namespace can cause an ambiguous name lookup when used in an expression. The solution involves mimicking the existing C++ solution which

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-26 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision. shafik added reviewers: aprantl, friss, teemperor. Herald added a subscriber: jdoerfert. https://reviews.llvm.org/D59845 added a fix for the `IsStructuralMatch(...)` specialization for `EnumDecl` this test should pass once this fix is committed.

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-27 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment. @friss so far I have not found any other scenarios that relate specifically to this test outside of the one I already have here https://reviews.llvm.org/D59667 I did find some other bugs though. I will do some more tests as a separate PR unless I can find one directly

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

2019-02-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2019-02-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
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:

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

2019-02-28 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2019-03-01 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2019-03-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2019-03-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D59004: [lldb] Fix DW_OP_addrx uses.

2019-03-05 Thread Shafik Yaghmour via Phabricator via lldb-commits
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?

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

2019-03-21 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

[Lldb-commits] [PATCH] D59667: Regression test to ensure that we handling importing of anonymous enums correctly

2019-03-21 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision. shafik added reviewers: aprantl, teemperor, jingham. Herald added a reviewer: serge-sans-paille. Herald added a subscriber: jdoerfert. https://reviews.llvm.org/D51633 added error handling in the ASTImporter.cpp which uncovered an underlying bug in which we used the

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

2019-02-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D58090: [WIP] Deserialize Clang module search path from DWARF

2019-02-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
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] D55653: Check pointer results on nullptr before using them

2019-02-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2019-02-15 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2019-01-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

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

2019-01-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

  1   2   3   4   5   6   7   8   >