[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] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/include/clang/AST/TemplateBase.h:88 +/// so cannot be dependent. +UncommonValue, + erichkeane wrote: > I definitely hate the name here... Just `Value` makes a bit more sense, but > isn't perfectly accurate.

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/test/CodeGenCXX/template-arguments.cpp:4 + +template CONSTEXPR T id(T v) { return v; } +template auto value = id(V); I don't see any tests covering unions or enums. Comment at: clang/test/SemaTemp

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp:204 +#if __cplusplus == 201703L + // cxx17-error@-3 {{non-type template argument refers to subobject '(int *)1'}} +#endif Shouldn't this be an error b/c it is a tempo

[Lldb-commits] [PATCH] D137003: [lldb-vscode] Send Statistics Dump in terminated event

shafik added a comment. Herald added a subscriber: JDevlieghere. It looks like this change broke `TestVSCode_terminatedEvent.py` see Green Dragon build bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/48111/ Please fix or revert. Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D135547: [lldb/Utility] Add GetDescription(Stream&) to StructureData::*

shafik added inline comments. Comment at: lldb/unittests/Utility/StructuredDataTest.cpp:41 + object_sp->GetDescription(S); + EXPECT_EQ(0, S.GetSize()); +} When doing a build I am seeing: ``` warning: comparison of integers of different signs: 'const int' and '

[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1534 + std::string qualified_name; + DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE(); + // TODO: change this to get the correct decl context parent -

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

shafik accepted this revision. shafik added a comment. LGTM Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39 const static auto char_min = std::numeric_limits::min(); const static auto uchar_min = std::numeric_limits::min(); We

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

shafik added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:733 + static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty, +SourceLocation Loc) { I think this makes sense but if we are go

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

shafik added a comment. Note, in both C and C++ converting a `-1` to unsigned will always result in the max unsigned value e.g.: #include #include int main() { int8_t i8 = -1; int32_t i32 = -1; unsigned x = i8; std::cout << x << "\n"; x = i32; std::cout <

[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] 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] 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] 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] 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

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] 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] 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] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 427081. shafik removed reviewers: teemperor, jingham, jasonmolenda. shafik added a comment. Herald added a project: All. - Expanded test - applied clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564

[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

shafik added a comment. You might want to try fun cases like `operator<` and `operator()()` from a lambda. They should work but might be worth throwing them in. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:330 + +// size_t from = 0; +// llvm::Stri

[Lldb-commits] [PATCH] D123954: [lldb] Add setting for max depth of value object printing (NFC)

shafik added inline comments. Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:718 + /// been told. + ChildrenOmissionWarningStatus m_truncation_warning; + /// Whether we reached the maximum child nesting depth and whether the user Why not use in

[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

shafik added a comment. This mostly makes sense, the purpose of the `|| alternate_defn` was not clear to me either. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124370/new/ https://reviews.llvm.org/D124370 ___

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

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

[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

shafik added a comment. Thank you, the names make a lot more sense to me but I am not sure why this was originally used either. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:648 + ValueObjectSP location_sp = + l->GetChildMemberWithName(ConstString("__data

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik updated this revision to Diff 423725. shafik added a comment. -Removed override `RemoteAwarePlatformTest.cpp` and added NOLINT CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123340/new/ https://reviews.llvm.org/D123340 Files: lldb/.clang-tidy lldb/source/Plugins/ExpressionPar

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik added a comment. In D123340#3438391 , @labath wrote: > The reason for the funny `/*override*/` thingy in the mock classes is that it > is impossible (in the gmock version that we use) to add the override keyword > to the methods overridden by the

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik created this revision. shafik added reviewers: labath, JDevlieghere, aprantl. Herald added a project: All. shafik requested review of this revision. Herald added a subscriber: aheejin. Applied clang-tidy `modernize-use-override` over LLDB and added it to the LLDB `.clang-tidy` config. ht

[Lldb-commits] [PATCH] D123098: [lldb] Fix undefined behavior: left shift of negative value

shafik added a comment. Note, in C++20 this was made well-defined see godbolt the paper that did this was P0907R4 Signed Integers are Two’s Complement CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

shafik added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp:236 if (count) -m_name = ConstString((char *)buffer_sp->GetBytes()); +m_name = ConstString((const char *)buffer_sp->GetBytes()); else

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

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

[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

[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] 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] 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

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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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

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] 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] 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 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 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] 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] 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] 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

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] 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] 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

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 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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

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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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()

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] 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] 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 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] 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] 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] 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] 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

  1   2   3   4   5   6   7   8   >