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

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 w

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

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. https://reviews.llv

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

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 top-level

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

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 re

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

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 pr

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

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 breaking,

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

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&revision=357210 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59847/new/ https://reviews.ll

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

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] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik updated this revision to Diff 194377. shafik added a comment. Addressing comments: - Now applies to all languages not just C++ - When adding locals be more selective on filtering i.e. only filter self and _cmd for Objective C etc... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

shafik marked 3 inline comments as done. shafik added a comment. @friss I believe I have addressed your comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 ___ lldb-commits mailing list lldb-com

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

shafik marked an inline comment as done. shafik added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:172 + +// We can check for .block_descriptor w/o checking for langauge since this +// is not a valid identifier in either C or C++. --

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

shafik added inline comments. Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:247 +struct StdModuleScope { + StdModuleHandler m_handler; aprantl wrote: > Can you add a `///` one-liner for this class? This definitely needs a description, it took

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

shafik updated this revision to Diff 194600. shafik added a comment. -Adjusting tests to ensure coverage of Objecive-C static and non-static methods and C and C++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 Files: packages/Python/lldbsui

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

shafik added a comment. @friss I had to rework the tests a little but they now cover Objective-C static and non-static methods as well as C and C++. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 _

[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

shafik created this revision. shafik added a reviewer: jingham. Herald added a reviewer: EricWF. Herald added a subscriber: christof. This previous fix https://github.com/llvm-mirror/lldb/commit/5469bda296c183d1b6bf74597c88c9ed667b3145 did not have a test since we did not have a reproducer. Thi

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

shafik marked 2 inline comments as done. shafik added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/namespace_local_var_same_name_cpp_and_c/TestNamespaceLocalVarSameNameCppAndC.py:11 +@skipUnlessDarwin +@add_test_categories(["gmodules"])

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

shafik updated this revision to Diff 195254. shafik marked 3 inline comments as done. shafik added a comment. Small updated to test, remove use of `printf` and associated include. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59667/new/ https://reviews.llvm.org/D59667 Files: package

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

shafik added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/cast_int_to_anonymous_enum/main.cpp:8 +int main() { + flow_e f; + aprantl wrote: > It looks like this variable is not actually used by the test? I need the type in the

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL358462: [ASTImporter] Regression test to ensure that we handling importing of anonymous… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.

shafik added a comment. Herald added a project: All. Is this PR still relevant or can it be closed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54053/new/ https://reviews.llvm.org/D54053 ___ lldb-commits mailing list lldb-commits@lists.llvm

[Lldb-commits] [PATCH] D115062: [LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType

shafik added a comment. I would like to see more extensive test cases, for example alternating `public` and `private` specifiers for both `class` and `struct` case. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1356 +clang::AccessSpecifier previous_a

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

shafik added a comment. I am happy as long as Pavel is good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114746/new/ https://reviews.llvm.org/D114746 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D115201: [lldb] Move UpdateSymbolContextScopeForType

shafik added a comment. Can you add a motivation for this to your summary, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115201/new/ https://reviews.llvm.org/D115201 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D115308: [LLDB] Uniquify Type in type list.

shafik added a comment. I would also like to know where this duplicate insertion is happening. Can you walk us through the steps that lead to the duplicate entries? thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115308/new/ https://revie

[Lldb-commits] [PATCH] D115662: [lldb][DWARF] Remove duplicate DIE type assignment

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

[Lldb-commits] [PATCH] D115062: [LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType

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

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

shafik added a comment. Pavel, I apologize no one let you know earlier but it looks like this change broke `TestFormattersBoolRefPtr.py` test. see: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/39449/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

shafik added a comment. Because `BOOL` is actually a `signed char` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112709/new/ https://reviews.llvm.org/D112709 ___ lldb-commits mailing list lldb-commits@li

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

shafik added a subscriber: aprantl. shafik added a comment. In D112709#3223098 , @shafik wrote: > Because `BOOL` is actually a `signed char` @aprantl corrected me, it can vary on some platforms it is `signed char` but on others it is `bool`. Repositor

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

shafik created this revision. shafik added reviewers: labath, jingham, JDevlieghere. shafik requested review of this revision. Both `close` and `closesocket` should return `0` on success so using `!!` looks incorrect. I replaced this will a more readable `== 0` check. https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

shafik added a comment. AFAICT this fix is correct but I am not sure how to verify of test it. I ran the test suite and it passed but that does not mean this is being covered. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116768/new/ https://reviews.llvm.org/D116768 __

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

This revision was automatically updated to reflect the committed changes. Closed by commit rG4f6d3a376c9f: [LLDB] Fix setting of success in Socket::Close() (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D116982: Fix clang-tidy bugprone-argument-comment that was mixed up

shafik created this revision. shafik added reviewers: wallace, JDevlieghere. shafik requested review of this revision. Several of the comments were annotating the wrong argument. I caught this while reviewing this clean-up: https://github.com/llvm/llvm-project/commit/8afcfbfb8fc1e53023ffac9d9bdc

[Lldb-commits] [PATCH] D116982: Fix clang-tidy bugprone-argument-comment that was mixed up

This revision was automatically updated to reflect the committed changes. Closed by commit rGc0e415471136: Fix clang-tidy bugprone-argument-comment that was mixed up (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

shafik added a comment. This is a nice refactor, I am curious was there a motivating bug or issue for this change or just refactoring? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117490/new/ https://reviews.llvm.org/D117490

[Lldb-commits] [PATCH] D118482: [lldb/Plugins] Add ScriptedProcessInterface::GetSelectedThreadIndex method

shafik added a comment. Just nitpicks Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:362 // previous thread state (if any). + uint32_t selected_thread_index = GetInterface().GetSelectedThreadIndex(); + `const` Commen

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

shafik accepted this revision. shafik added a comment. LGTM after addressing Jonas's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114627/new/ https://reviews.llvm.org/D114627 ___ lldb-commits

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

shafik created this revision. shafik added reviewers: aprantl, dblaikie. shafik requested review of this revision. Currently we are not emitting debug-info for all cases of structured bindings a C++17 feature which allows us to bind names to subobjects in an initializer. A structured binding is

[Lldb-commits] [PATCH] D119168: [lldb/crashlog] Fix arm64 register parsing on crashlog.py

shafik added inline comments. Comment at: lldb/examples/python/crashlog.py:529 +if key == "x": +registers.update(self.parse_thread_registers({ str(idx) : reg + for idx,reg in

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

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. I believe this a program like this int main() { int arr[2]{0}; return arr[1]; } and an expression like this `expr arr[0]` will give us the constant expression `getelementptr`

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

shafik added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4667 + + SmallVector Expr; + AppendAddressSpaceXDeref(AddressSpace, Expr); aprantl wrote: > 13 seems to be unnecessarily high. Shouldn't 1 be enough for the expected > single DW_OP_der

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

shafik added a comment. I was hoping you would add a positive test as well like the one I came up with that covered the new code as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113498/new/ https://reviews.llvm.org/D113498 _

[Lldb-commits] [PATCH] D119388: [lldb/Plugin] Add artificial stackframe loading in ScriptedThread

shafik added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:159 + + size_t arr_size = arr_sp->GetSize(); + if (arr_size > std::numeric_limits::max()) `const` Comment at: lldb/source/Plugins/Process/script

[Lldb-commits] [PATCH] D119616: [lldb] Replace asserts on .Success() with assertSuccess()

shafik added a comment. Can you please provide a motivation in the summary, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119616/new/ https://reviews.llvm.org/D119616 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D119734: [lldb] Add a positive test for `getelementptr` constant args

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

[Lldb-commits] [PATCH] D119857: [lldb] Don't rely on unsigned integer wrapping in PutRawBytes and PutBytesAsRawHex8

shafik added a comment. Do we have a test that covers this edge case? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119857/new/ https://reviews.llvm.org/D119857 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

shafik updated this revision to Diff 409056. shafik marked 5 inline comments as done. shafik added a comment. Addressed comments on SmallVector size and fixed test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119178/new/ https://reviews.llvm.org/D119178 Files: clang/lib/CodeGen/CGD

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

shafik added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); + assert(!LexicalBlockStack.empty() && "Region stack mismatch, s

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

shafik created this revision. shafik added reviewers: labath, aprantl. shafik requested review of this revision. Identifiers with `__` anywhere are reserved. I picked this up via the `bugprone-reserved-identifier` clang-tidy check but `-Wreserved-identifier` will also flag these uses as well.

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

shafik updated this revision to Diff 409344. shafik added a comment. Used shorter options for renaming based on feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119915/new/ https://reviews.llvm.org/D119915 Files: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/source/

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

This revision was automatically updated to reflect the committed changes. Closed by commit rG80a11e080358: [LLDB] Replace use of double underscore in identifiers (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D119501: [lldb/crashlog] Add CrashLogScriptedProcess & remove interactive mode

shafik added a comment. Looks like the `scripted_crashlog_json.test` is broken on green dragon: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/41454/testReport/junit/lldb-shell/ScriptInterpreter_Python_Crashlog/scripted_crashlog_json_test/ Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf56cb520d855: [DEBUGINFO] [LLDB] Add support for generating debug-info for structured… (authored by shafik). Herald added projects: clang, LLDB. Rep

[Lldb-commits] [PATCH] D120105: Remove recursive include of GDBRemoteCommunicationServerCommon.h

shafik created this revision. shafik added reviewers: jingham, aprantl. shafik requested review of this revision. `GDBRemoteCommunicationServerCommon.h` includes itself, removing this include. https://reviews.llvm.org/D120105 Files: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicatio

[Lldb-commits] [PATCH] D120105: Remove recursive include of GDBRemoteCommunicationServerCommon.h

This revision was automatically updated to reflect the committed changes. Closed by commit rG14101f48d205: [LLDB] Remove recursive include of GDBRemoteCommunicationServerCommon.h (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

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

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

shafik added a comment. In D120718#3352450 , @JDevlieghere wrote: > I think the better solution here is to get rid of the `using namespace llvm;` > in the implementation file instead. That is a good point. I did a quick look at the file `DataFileCache.

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added a comment. I used `using namespace` in a few functions but I am not committed to this approach. So I am happy to hear feedback on whether we want to just use fully qualified names everywhere instead or nail down a criteria as to when it is acceptable. We also have `using namespace

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added a comment. In D120836#3355167 , @labath wrote: > I think it's reasonable to be able to refer to the dwarf constants from > within the dwarf plugin via their base names alone. The implementation -- a > top-level `using namespace llvm::dwarf`

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik updated this revision to Diff 412548. shafik added a comment. Updating diff based on comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120836/new/ https://reviews.llvm.org/D120836 Files: lldb/include/lldb/Core/dwarf.h lldb/include/lldb/Symbol/DWARFCallFrameInfo.h lldb

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:46 using namespace lldb_private; +using namespace llvm::dwarf; JDevlieghere wrote: > Why not `lldb_private::dwarf`? I think I started out with this file and must have written

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik updated this revision to Diff 412751. shafik marked 6 inline comments as done. shafik added a comment. Moved to `using namespace lldb_private::dwarf` everywhere since the consensus is that being verbose is preferred. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120836/new/ http

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

This revision was automatically updated to reflect the committed changes. Closed by commit rGae869d448499: [LLDB] Remove cases of using namespace llvm:: from header file (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D120966: Remove cases of using namespace std

shafik created this revision. shafik added reviewers: labath, aprantl, JDevlieghere. Herald added a project: All. shafik requested review of this revision. We had `using namespace std;` sprinkled around several source files and tests. This also follows the LLVM coding standard

[Lldb-commits] [PATCH] D120966: Remove cases of using namespace std

shafik added a comment. As a side note, I noticed that we don't prefix typedefs from `cstdint` with `std::` e.g. `size_t`. These typedefs are not guaranteed to be in the global namespace. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120966/new/ https://reviews.llvm.org/D120966 _

[Lldb-commits] [PATCH] D120966: Remove cases of using namespace std

This revision was automatically updated to reflect the committed changes. Closed by commit rG9bd72b5c2585: [LLDB] Remove cases of using namespace std (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[Lldb-commits] [PATCH] D121161: [lldb] Avoid global constructor in LLDBLog.cpp

shafik added a comment. In D121161#3368117 , @JDevlieghere wrote: > In D121161#3367806 , @labath wrote: > >> The class has a constexpr constructor. I thought that would be enough to >> avoid runtime initializati

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik created this revision. shafik added reviewers: aprantl, labath, jingham. Herald added a project: All. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Currently `DW_OP_deref_size` just drops the `ValueType::FileAddress` case

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik added a comment. There can probably be some more refactoring wrt to the `Value` class which also does a lot of the same work but there are enough differences that I think any attempt at that should be left for a separate PR. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121408/n

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik updated this revision to Diff 414663. shafik marked 12 inline comments as done. shafik added a comment. Addressing comment - Adding documentation - Moving some more error handling into helper - Cleaning up the calling side CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121408/new/

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:948 +llvm::Optional +ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp, + Status *error_ptr, const char *dw_op_type, aprantl

[Lldb-commits] [PATCH] D121481: Applying clang-tidy modernize-use-default-member-init over LLDB

shafik created this revision. shafik added reviewers: aprantl, JDevlieghere, labath. Herald added a subscriber: arphaman. Herald added a project: All. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Applied `modernize-use-default-m

[Lldb-commits] [PATCH] D121481: Applying clang-tidy modernize-use-default-member-init over LLDB

shafik updated this revision to Diff 414715. shafik added a comment. Applying clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121481/new/ https://reviews.llvm.org/D121481 Files: lldb/source/API/SBBroadcaster.cpp lldb/source/API/SBListener.cpp lldb/source/API/SBPlatform

[Lldb-commits] [PATCH] D121481: Applying clang-tidy modernize-use-default-member-init over LLDB

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

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik updated this revision to Diff 415440. shafik marked an inline comment as done. shafik added a comment. Removing dead code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121408/new/ https://reviews.llvm.org/D121408 Files: lldb/source/Expression/DWARFExpression.cpp lldb/test/S

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

shafik added a comment. I think this PR broke green dragon incremental build: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/42185/ It looks like the failures are all related to platforms in some ways. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

shafik added a comment. I believe this broke the lldb build bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45608/console Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130528/new/ https://reviews.llvm.org/D130528 __

[Lldb-commits] [PATCH] D130795: [LLDB][NFC][Reliability] Fixes for int overflow and uninitialized state

shafik added inline comments. Comment at: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp:67 + return static_cast(size == 8 ? 0x2 : size - 1) + << (18 + 4 * wp_index); } So we know that `wp_index` is never greater than 11?

[Lldb-commits] [PATCH] D131460: [LLDB] Remove undefined behavior in TestConstStaticIntegralMember.py

shafik created this revision. shafik added reviewers: JDevlieghere, werat, aaron.ballman. Herald added a project: All. shafik requested review of this revision. Setting an enum without a fixed underlying type to a value which is outside the value range is undefined behavior. The initializer need

[Lldb-commits] [PATCH] D131460: [LLDB] Remove undefined behavior in TestConstStaticIntegralMember.py

shafik added a comment. I will probably land this soon since this is being reported by several built bots and I this should bring the builds back to green. I just wanted the interested parties to see this in case there is some functionality this is testing that I am missing and some additions a

[Lldb-commits] [PATCH] D131460: [LLDB] Remove undefined behavior in TestConstStaticIntegralMember.py

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 rG1438639a2f7e: [LLDB] Remove undefined behavior in TestConstStaticIntegralMember.py (authored by shafik). Herald added a pr

[Lldb-commits] [PATCH] D131472: [LLDB] Add multi value test for const static enum

shafik added a comment. In D131472#3709684 , @labath wrote: > Good catch. Lldb should try to be useful even if the debugged program invokes > undefined behavior. Totally agree, for the purposes here there should be no difference for these purposes betw

[Lldb-commits] [PATCH] D131472: [LLDB] Add multi value test for const static enum

shafik accepted this revision. shafik added a comment. LGTM. Thank you for the quick follow-up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131472/new/ https://reviews.llvm.org/D131472 ___ lldb-commits

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

shafik added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:39 + CompilerType coro_func_type = ast_ctx.CreateFunctionType( + /*result_type*/ void_type, /*args*/ &void_type, /*num_args*/ 1, + /*is_variadic*/ false, /*qualifiers*/ 0);

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

shafik updated this revision to Diff 415473. shafik added a comment. Fixing up diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121408/new/ https://reviews.llvm.org/D121408 Files: lldb/source/Expression/DWARFExpression.cpp lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_sta

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

This revision was automatically updated to reflect the committed changes. Closed by commit rG6583f0170721: [LLDB] Fixing DWARFExpression handling of ValueType::FileAddress case for… (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[Lldb-commits] [PATCH] D121831: Modifying expression code in MakeLoadImageUtilityFunction to be more consistent

shafik created this revision. shafik added reviewers: aprantl, JDevlieghere. Herald added a subscriber: emaste. Herald added a project: All. shafik requested review of this revision. `MakeLoadImageUtilityFunction()` is not using `extern "C"` for external C functions and it is not using `eLanguage

[Lldb-commits] [PATCH] D121831: Modifying expression code in MakeLoadImageUtilityFunction to be more consistent

shafik added a comment. Note, we have a few tests that cover `LoadImage`: TestCompletion.py TestLoadUsingLazyBind.py TestLoadUsingPaths.py So they should be covering this code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121831/new/ https://reviews.llvm.org/D121831 __

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

shafik created this revision. shafik added reviewers: labath, aprantl, JDevlieghere. Herald added a project: All. shafik requested review of this revision. Applied `modernize-use-equals-default ` clang-tidy check over LLDB. This check is already present in the lldb/.clang-tidy config. https://r

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

shafik added a comment. We sure do have a lot of `CommandOptions` classes out there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121844/new/ https://reviews.llvm.org/D121844 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://

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

shafik added inline comments. Comment at: lldb/source/Core/Value.cpp:667 -const ValueList &ValueList::operator=(const ValueList &rhs) { +const ValueList &ValueList::operator=(const ValueList &rhs) { // NOLINT(modernize-use-equals-default) m_values = rhs.m_values; -

[Lldb-commits] [PATCH] D121831: Modifying expression code in MakeLoadImageUtilityFunction to be more consistent

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG33d74170a36e: [LLDB] Modifying expression code in MakeLoadImageUtilityFunction to be more… (authored by shafik). Herald added a project: LLDB. Repos

[Lldb-commits] [PATCH] D121831: Modifying expression code in MakeLoadImageUtilityFunction to be more consistent

shafik added a comment. In D121831#3390152 , @aprantl wrote: > Did you check if we have other helper expressions with the same problem? Yes, I could not find any other instances like this. They are not straight forward to find but I think I found all th

[Lldb-commits] [PATCH] D122041: [llvm][utils] Fix llvm::Optional summary provider

shafik added a comment. Do we have any tests for these? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122041/new/ https://reviews.llvm.org/D122041 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

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

shafik marked 2 inline comments as done. shafik added inline comments. Comment at: lldb/source/Core/Value.cpp:667 -const ValueList &ValueList::operator=(const ValueList &rhs) { +const ValueList &ValueList::operator=(const ValueList &rhs) { // NOLINT(modernize-use-equals-defau

[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

shafik created this revision. shafik added reviewers: labath, aprantl, JDevlieghere. Herald added a subscriber: arphaman. Herald added a project: All. shafik requested review of this revision. `NSIndexPathSyntheticFrontEnd::Impl::Clear()` currently calls `Clear()` on both unions members regardles

[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

shafik added a comment. I believe that `NSIndexPathSyntheticFrontEnd::Update()` also needs a fix in order to properly set the active member but I will do that as a separate fix since. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122753/new/ https://reviews.llvm.org/D122753 _

[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

This revision was automatically updated to reflect the committed changes. Closed by commit rG14cad95d3823: [LLDB] Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active… (authored by shafik). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

shafik marked 3 inline comments as done. shafik added inline comments. Comment at: lldb/source/Core/Value.cpp:667 -const ValueList &ValueList::operator=(const ValueList &rhs) { +const ValueList &ValueList::operator=(const ValueList &rhs) { // NOLINT(modernize-use-equals-defau

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

shafik updated this revision to Diff 419550. shafik marked 2 inline comments as done. shafik added a comment. - Rebased - Applied clang-format - Address Pavel's comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121844/new/ https://reviews.llvm.org/D121844 Files: lldb/source/API/S

<    1   2   3   4   5   6   7   8   >