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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.  Perhaps `NonTypeValue`?  But that is a little 
> redundant.  `Uncommon` here is just strange and not particularly descriptive. 
This catch all `UncommonValue` feels artificial. Maybe we need a separate out 
the cases into more granular cases like `Float` etc



Comment at: clang/lib/AST/TemplateBase.cpp:204-211
+  if (Type->isIntegralOrEnumerationType() && V.isInt())
+*this = TemplateArgument(Ctx, V.getInt(), Type);
+  else if ((V.isLValue() && V.isNullPointer()) ||
+   (V.isMemberPointer() && !V.getMemberPointerDecl()))
+*this = TemplateArgument(Type, /*isNullPtr=*/true);
+  else if (const ValueDecl *VD = getAsSimpleValueDeclRef(Ctx, Type, V))
+// FIXME: The Declaration form should expose a const ValueDecl*.

erichkeane wrote:
> aaron.ballman wrote:
> > Well this is certainly a unique approach...  Personally, I think it'd be 
> > nice to not assign to `this` within a constructor by calling other 
> > constructor; that falls squarely into "wait, what, can you even DO that?" 
> > kind of questions for me.
> I agree, this function needs to be refactored to call some sort of 'init' 
> function or something, even if we have to refactor the other constructors.  
> This assignment to `*this` is just too strange to let stay.
I am going to third this sentiment, this is definitely not the right approach 
and the fact that you have this ad-hoc case below here also speaks to this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140996/new/

https://reviews.llvm.org/D140996

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/SemaTemplate/temp_arg_nontype_cxx20.cpp:12
+using F1 = Float<1.0f>;
+using F1 = Float<2.0f / 2>;
 

I believe this is IFNDR the template-heads are functionally equivelent but not 
equivelent: https://eel.is/c++draft/temp.class#general-3



Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:56
+using CF = ComplexFloat<1.0f + 3.0fi>;
+using CF = ComplexFloat<3.0fi + 1.0f>;
 

Can we add an enum example e.g.:

```
enum E{ E1, E2};
template  struct SE {};
using IPE = SE;
using IPE = SE;

```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140996/new/

https://reviews.llvm.org/D140996

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 temporary? What is `(int*)1` a subobject 
of?



Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:21
+using IPn = IntPtr< + 1>;
+using IPn = IntPtr< + 1>;
 

`using IPn = IntPtr< + 2>` should be an error since that would be out of 
bounds, while `+1` is ok b/c it is one past the end as long as we don't deref.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:21
+using IPn = IntPtr< + 1>;
+using IPn = IntPtr< + 1>;
 

shafik wrote:
> `using IPn = IntPtr< + 2>` should be an error since that would be out of 
> bounds, while `+1` is ok b/c it is one past the end as long as we don't deref.
gcc reject this one but I think their pointer math is wonky here: 
https://godbolt.org/z/fhMqPPefG



Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:26
 
-using IP3 = IntPtr<[3]>; // FIXME expected-error {{refers to subobject}}
-using IP3 = IntPtr; // FIXME expected-error {{refers to subobject}}
+using IP3 = IntPtr<[3]>;
+using IP3 = IntPtr;

We should reject `IntPtr<[5]>;` again b/c it is out of bounds. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140996/new/

https://reviews.llvm.org/D140996

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 SINCE LAST ACTION
  https://reviews.llvm.org/D137003/new/

https://reviews.llvm.org/D137003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 'const 
unsigned long' [-Wsign-compare]
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135547/new/

https://reviews.llvm.org/D135547

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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

So it looks like you can do `SymbolFileDWARF::GetDWARFDeclContext(...)`  and 
`DWARFDeclContext` has `GetQualifiedName()` which I think is doing what you are 
doing below and maybe handles more cases. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135979/new/

https://reviews.llvm.org/D135979

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 use `signed char` for the max case but `char` for the min case. Maybe we 
need a max using `char` and a min using `signed char`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135170/new/

https://reviews.llvm.org/D135170

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: clang/include/clang/AST/ExprCXX.h:733
 
+  static CXXBoolLiteralExpr *Create(const ASTContext , bool Val, QualType Ty,
+SourceLocation Loc) {

I think this makes sense but if we are going to do this refactor then we should 
clean up all the locations we are currently doing `new (Context) 
CXXBoolLiteralExpr()` as well.

Maybe that should be the initial PR and then this change would be a follow-up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135169/new/

https://reviews.llvm.org/D135169

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 << x << "\n";
  }

output:

  4294967295
  4294967295


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134493/new/

https://reviews.llvm.org/D134493

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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*/ _type, /*num_args*/ 1,
+  /*is_variadic*/ false, /*qualifiers*/ 0);

`/*result_type=*/` see 
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html

applies to the rest of them as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132415/new/

https://reviews.llvm.org/D132415

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 between a scoped enum and an enum without a fixed underlying type.




Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:52
   const static ScopedEnum invalid_scoped_enum_val = static_cast(5);
+  const static ScopedEnum invalid_scoped_enum_val_2 =
+  static_cast(7);

I am just going to super nitpick here and say that these are not invalid 
values. When we have a fixed underlying type we are allowed to use the full 
range of the underlying type. The values may be outside the range of the 
enumerators but they are valid values. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131472/new/

https://reviews.llvm.org/D131472

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131460/new/

https://reviews.llvm.org/D131460

Files:
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -47,7 +47,6 @@
   std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
-  const static Enum invalid_enum_val = static_cast(enum_case2 + 5);
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
   const static ScopedEnum invalid_scoped_enum_val = static_cast(5);
   const static ScopedCharEnum scoped_char_enum_val = ScopedCharEnum::case2;
@@ -102,7 +101,6 @@
   int member_copy = ClassWithOnlyConstStatic::member;
 
   Enum e = A::enum_val;
-  e = A::invalid_enum_val;
   ScopedEnum se = A::scoped_enum_val;
   se = A::invalid_scoped_enum_val;
   ScopedCharEnum sce = A::scoped_char_enum_val;
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -53,8 +53,6 @@
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
-# Test an unscoped enum with an invalid enum case.
-self.expect_expr("A::invalid_enum_val", result_value="enum_case1 | 
enum_case2 | 0x4")
 
 # Test a scoped enum.
 self.expect_expr("A::scoped_enum_val", 
result_value="scoped_enum_case2")


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -47,7 +47,6 @@
   std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
-  const static Enum invalid_enum_val = static_cast(enum_case2 + 5);
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
   const static ScopedEnum invalid_scoped_enum_val = static_cast(5);
   const static ScopedCharEnum scoped_char_enum_val = ScopedCharEnum::case2;
@@ -102,7 +101,6 @@
   int member_copy = ClassWithOnlyConstStatic::member;
 
   Enum e = A::enum_val;
-  e = A::invalid_enum_val;
   ScopedEnum se = A::scoped_enum_val;
   se = A::invalid_scoped_enum_val;
   ScopedCharEnum sce = A::scoped_char_enum_val;
Index: lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -53,8 +53,6 @@
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
-# Test an unscoped enum with an invalid enum case.
-self.expect_expr("A::invalid_enum_val", result_value="enum_case1 | enum_case2 | 0x4")
 
 # Test a scoped enum.
 self.expect_expr("A::scoped_enum_val", result_value="scoped_enum_case2")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 are needed to  cover what you want.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131460/new/

https://reviews.llvm.org/D131460

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 needs to be a constant expression and therefore this was always 
ill-formed we just were not diagnosing it before.

See D130058  and D131307 
 for more details.


https://reviews.llvm.org/D131460

Files:
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -47,7 +47,6 @@
   std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
-  const static Enum invalid_enum_val = static_cast(enum_case2 + 5);
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
   const static ScopedEnum invalid_scoped_enum_val = static_cast(5);
   const static ScopedCharEnum scoped_char_enum_val = ScopedCharEnum::case2;
@@ -102,7 +101,6 @@
   int member_copy = ClassWithOnlyConstStatic::member;
 
   Enum e = A::enum_val;
-  e = A::invalid_enum_val;
   ScopedEnum se = A::scoped_enum_val;
   se = A::invalid_scoped_enum_val;
   ScopedCharEnum sce = A::scoped_char_enum_val;
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -53,8 +53,6 @@
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
-# Test an unscoped enum with an invalid enum case.
-self.expect_expr("A::invalid_enum_val", result_value="enum_case1 | 
enum_case2 | 0x4")
 
 # Test a scoped enum.
 self.expect_expr("A::scoped_enum_val", 
result_value="scoped_enum_case2")


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -47,7 +47,6 @@
   std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
-  const static Enum invalid_enum_val = static_cast(enum_case2 + 5);
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
   const static ScopedEnum invalid_scoped_enum_val = static_cast(5);
   const static ScopedCharEnum scoped_char_enum_val = ScopedCharEnum::case2;
@@ -102,7 +101,6 @@
   int member_copy = ClassWithOnlyConstStatic::member;
 
   Enum e = A::enum_val;
-  e = A::invalid_enum_val;
   ScopedEnum se = A::scoped_enum_val;
   se = A::invalid_scoped_enum_val;
   ScopedCharEnum sce = A::scoped_char_enum_val;
Index: lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -53,8 +53,6 @@
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
-# Test an unscoped enum with an invalid enum case.
-self.expect_expr("A::invalid_enum_val", result_value="enum_case1 | enum_case2 | 0x4")
 
 # Test a scoped enum.
 self.expect_expr("A::scoped_enum_val", result_value="scoped_enum_case2")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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?



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp:77
   // wp.: xxEE33221100
   return GetBothEnableBitMask(wp_index) | (0xF << (16 + 4 * wp_index));
 }

You did not update `0xF` to `0xFULL`  like you did the constants above. Was 
that on purpose?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130795/new/

https://reviews.llvm.org/D130795

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Shafik Yaghmour via Phabricator via lldb-commits]

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

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/lang/cpp/auto_return/Makefile
  lldb/test/API/lang/cpp/auto_return/TestCppAutoReturn.py
  lldb/test/API/lang/cpp/auto_return/main.cpp
  lldb/test/API/lang/cpp/auto_return/other.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s
@@ -0,0 +1,257 @@
+# This tests that lldb when using symbol table we are able to find the definition
+# for an auto return function.
+
+# RUN: llvm-mc -triple x86_64-apple-macosx10.15.0 %s -filetype=obj > %t.o
+# RUN: lldb-test symbols --dump-clang-ast %t.o | FileCheck %s
+
+# CHECK: CXXMethodDecl {{.*}} <>  f 'int ()'
+
+# This was compiled from the following code:
+#
+# struct A {
+# auto f();
+# };
+#
+# auto A::f() {
+# return 0;
+# }
+#
+# Compiled using:
+#
+#  -target x86_64-apple-macosx10.15.0
+#
+# and edited to remove some uneeded sections.
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	__ZN1A1fEv  ## -- Begin function _ZN1A1fEv
+	.p2align	4, 0x90
+__ZN1A1fEv: ## @_ZN1A1fEv
+Lfunc_begin0:
+	.cfi_startproc
+## %bb.0:   ## %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movq	%rdi, -8(%rbp)
+Ltmp0:
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	19  ## DW_TAG_structure_type
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	54  ## DW_AT_calling_convention
+	.byte	11  ## DW_FORM_data1
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	11  ## DW_AT_byte_size
+	.byte	11  ## DW_FORM_data1
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	46  ## DW_TAG_subprogram
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	110 ## DW_AT_linkage_name
+	.byte	14  ## DW_FORM_strp
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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::StringRef demangled_ref = demangled.GetStringRef();

dead code?



Comment at: lldb/tools/CMakeLists.txt:11
 
-add_lldb_tool_subdirectory(lldb-instr)
+#add_lldb_tool_subdirectory(lldb-instr)
 add_lldb_tool_subdirectory(lldb-vscode)

Did you mean to delete this line?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124579/new/

https://reviews.llvm.org/D124579

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 class member initialization? It looks like they have default 
values. I am not sure why the rest of the values where not caught the other day 
when I ran clang-tidy.



Comment at: lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp:180
 use_dynamic = target_sp->GetPreferDynamicValue();
-  else {
+std::tie(max_depth, max_depth_is_default) =
+target_sp->GetMaximumDepthOfChildrenToDisplay();

My kingdom for structured bindings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123954/new/

https://reviews.llvm.org/D123954

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D123340/new/

https://reviews.llvm.org/D123340

Files:
  lldb/.clang-tidy
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/unittests/API/SBCommandInterpreterTest.cpp
  lldb/unittests/Interpreter/TestCommandPaths.cpp
  lldb/unittests/Interpreter/TestOptionValue.cpp
  lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Index: lldb/unittests/Target/RemoteAwarePlatformTest.cpp
===
--- lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -37,7 +37,8 @@
const FileSpecList *));
   Status ResolveRemoteExecutable(
   const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-  const FileSpecList *module_search_paths_ptr) /*override*/ {
+  const FileSpecList *module_search_paths_ptr) /*override*/
+  { // NOLINT(modernize-use-override)
 auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
 exe_module_sp = pair.second;
 return pair.first;
Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -111,7 +111,7 @@
   }
 
 private:
-  lldb::OptionValueSP Clone() const {
+  lldb::OptionValueSP Clone() const override {
 return std::make_shared(*this);
   }
 
Index: lldb/unittests/Interpreter/TestCommandPaths.cpp
===
--- lldb/unittests/Interpreter/TestCommandPaths.cpp
+++ lldb/unittests/Interpreter/TestCommandPaths.cpp
@@ -51,7 +51,7 @@
   }
 
 protected:
-  virtual bool DoExecute(Args , CommandReturnObject ) {
+  bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishResult);
 result.AppendMessage("I did nothing");
 return true;
Index: lldb/unittests/API/SBCommandInterpreterTest.cpp
===
--- lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -34,7 +34,7 @@
   DummyCommand(const char *message) : m_message(message) {}
 
   bool DoExecute(SBDebugger dbg, char **command,
- SBCommandReturnObject ) {
+ SBCommandReturnObject ) override {
 result.PutCString(m_message.c_str());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 return result.Succeeded();
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -74,7 +74,7 @@
 m_collection_sp->Initialize(g_processkdp_properties);
   }
 
-  virtual ~PluginProperties() = default;
+  ~PluginProperties() override = default;
 
   uint64_t GetPacketTimeout() {
 const uint32_t idx = ePropertyKDPPacketTimeout;
@@ -880,7 +880,7 @@
 m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessKDPPacketSend() = default;
+  ~CommandObjectProcessKDPPacketSend() override = default;
 
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
@@ -981,7 +981,7 @@
 CommandObjectSP(new CommandObjectProcessKDPPacketSend(interpreter)));
   }
 
-  ~CommandObjectProcessKDPPacket() = default;
+  ~CommandObjectProcessKDPPacket() override = default;
 };
 
 class CommandObjectMultiwordProcessKDP : public CommandObjectMultiword {
@@ -995,7 +995,7 @@
  interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessKDP() = default;
+  ~CommandObjectMultiwordProcessKDP() override = default;
 };
 
 CommandObject *ProcessKDP::GetPluginCommandObject() {
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -193,7 +193,7 @@
 m_collection_sp->Initialize(g_platformdarwinkernel_properties);
   }
 
-  virtual ~PlatformDarwinKernelProperties() = default;
+  ~PlatformDarwinKernelProperties() override = default;
 
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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_"), true);
+  ValueObjectSP size_vo =

nitpick `/*can_create=*/true` and same for the next two.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124113/new/

https://reviews.llvm.org/D124113

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/unittests/API/SBCommandInterpreterTest.cpp
  lldb/unittests/Interpreter/TestCommandPaths.cpp
  lldb/unittests/Interpreter/TestOptionValue.cpp
  lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Index: lldb/unittests/Target/RemoteAwarePlatformTest.cpp
===
--- lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -37,7 +37,8 @@
const FileSpecList *));
   Status ResolveRemoteExecutable(
   const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-  const FileSpecList *module_search_paths_ptr) /*override*/ {
+  const FileSpecList *module_search_paths_ptr) /*override*/
+  { // NOLINT(modernize-use-override)
 auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
 exe_module_sp = pair.second;
 return pair.first;
Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -111,7 +111,7 @@
   }
 
 private:
-  lldb::OptionValueSP Clone() const {
+  lldb::OptionValueSP Clone() const override {
 return std::make_shared(*this);
   }
 
Index: lldb/unittests/Interpreter/TestCommandPaths.cpp
===
--- lldb/unittests/Interpreter/TestCommandPaths.cpp
+++ lldb/unittests/Interpreter/TestCommandPaths.cpp
@@ -51,7 +51,7 @@
   }
 
 protected:
-  virtual bool DoExecute(Args , CommandReturnObject ) {
+  bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishResult);
 result.AppendMessage("I did nothing");
 return true;
Index: lldb/unittests/API/SBCommandInterpreterTest.cpp
===
--- lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -34,7 +34,7 @@
   DummyCommand(const char *message) : m_message(message) {}
 
   bool DoExecute(SBDebugger dbg, char **command,
- SBCommandReturnObject ) {
+ SBCommandReturnObject ) override {
 result.PutCString(m_message.c_str());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 return result.Succeeded();
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -74,7 +74,7 @@
 m_collection_sp->Initialize(g_processkdp_properties);
   }
 
-  virtual ~PluginProperties() = default;
+  ~PluginProperties() override = default;
 
   uint64_t GetPacketTimeout() {
 const uint32_t idx = ePropertyKDPPacketTimeout;
@@ -877,7 +877,7 @@
 m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessKDPPacketSend() = default;
+  ~CommandObjectProcessKDPPacketSend() override = default;
 
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
@@ -978,7 +978,7 @@
 CommandObjectSP(new CommandObjectProcessKDPPacketSend(interpreter)));
   }
 
-  ~CommandObjectProcessKDPPacket() = default;
+  ~CommandObjectProcessKDPPacket() override = default;
 };
 
 class CommandObjectMultiwordProcessKDP : public CommandObjectMultiword {
@@ -992,7 +992,7 @@
  interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessKDP() = default;
+  ~CommandObjectMultiwordProcessKDP() override = default;
 };
 
 CommandObject *ProcessKDP::GetPluginCommandObject() {
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -193,7 +193,7 @@
 m_collection_sp->Initialize(g_platformdarwinkernel_properties);
   }
 
-  virtual ~PlatformDarwinKernelProperties() = default;
+  ~PlatformDarwinKernelProperties() override = default;
 
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 MOCK macros. Then, having override keywords 
> on hand-written methods triggered `-Winconsistent-missing-override`.
>
> Or at least it used to -- have you checked that these changes don't introduce 
> any new compiler warnings?

I missed that, it does indeed trigger the same warning, I guess I will add a 
`NOLINT`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123340/new/

https://reviews.llvm.org/D123340

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.


https://reviews.llvm.org/D123340

Files:
  lldb/.clang-tidy
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/unittests/API/SBCommandInterpreterTest.cpp
  lldb/unittests/Interpreter/TestCommandPaths.cpp
  lldb/unittests/Interpreter/TestOptionValue.cpp
  lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Index: lldb/unittests/Target/RemoteAwarePlatformTest.cpp
===
--- lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -37,7 +37,7 @@
const FileSpecList *));
   Status ResolveRemoteExecutable(
   const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-  const FileSpecList *module_search_paths_ptr) /*override*/ {
+  const FileSpecList *module_search_paths_ptr) override /*override*/ {
 auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
 exe_module_sp = pair.second;
 return pair.first;
Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -111,7 +111,7 @@
   }
 
 private:
-  lldb::OptionValueSP Clone() const {
+  lldb::OptionValueSP Clone() const override {
 return std::make_shared(*this);
   }
 
Index: lldb/unittests/Interpreter/TestCommandPaths.cpp
===
--- lldb/unittests/Interpreter/TestCommandPaths.cpp
+++ lldb/unittests/Interpreter/TestCommandPaths.cpp
@@ -51,7 +51,7 @@
   }
 
 protected:
-  virtual bool DoExecute(Args , CommandReturnObject ) {
+  bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishResult);
 result.AppendMessage("I did nothing");
 return true;
Index: lldb/unittests/API/SBCommandInterpreterTest.cpp
===
--- lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -34,7 +34,7 @@
   DummyCommand(const char *message) : m_message(message) {}
 
   bool DoExecute(SBDebugger dbg, char **command,
- SBCommandReturnObject ) {
+ SBCommandReturnObject ) override {
 result.PutCString(m_message.c_str());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 return result.Succeeded();
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -74,7 +74,7 @@
 m_collection_sp->Initialize(g_processkdp_properties);
   }
 
-  virtual ~PluginProperties() = default;
+  ~PluginProperties() override = default;
 
   uint64_t GetPacketTimeout() {
 const uint32_t idx = ePropertyKDPPacketTimeout;
@@ -877,7 +877,7 @@
 m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessKDPPacketSend() = default;
+  ~CommandObjectProcessKDPPacketSend() override = default;
 
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
@@ -978,7 +978,7 @@
 CommandObjectSP(new CommandObjectProcessKDPPacketSend(interpreter)));
   }
 
-  ~CommandObjectProcessKDPPacket() = default;
+  ~CommandObjectProcessKDPPacket() override = default;
 };
 
 class CommandObjectMultiwordProcessKDP : public CommandObjectMultiword {
@@ -992,7 +992,7 @@
  interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessKDP() = default;
+  ~CommandObjectMultiwordProcessKDP() override = default;
 };
 
 CommandObject *ProcessKDP::GetPluginCommandObject() {
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -193,7 +193,7 @@
 m_collection_sp->Initialize(g_platformdarwinkernel_properties);
   }
 
-  virtual ~PlatformDarwinKernelProperties() = default;
+  ~PlatformDarwinKernelProperties() override = default;
 
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D123098/new/

https://reviews.llvm.org/D123098

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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

nitpick `reinterpret_cast` instead of C-style cast.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122856/new/

https://reviews.llvm.org/D122856

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D121844/new/

https://reviews.llvm.org/D121844

Files:
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Host/macosx/cfcpp/CFCData.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
  lldb/source/Host/macosx/cfcpp/CFCString.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBDataRef.cpp
  lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
  lldb/tools/debugserver/source/MacOSX/CFString.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/StdStringExtractor.cpp
  lldb/tools/debugserver/source/TTYState.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/VSCode.cpp

Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -58,7 +58,7 @@
 log.reset(new std::ofstream(log_file_path));
 }
 
-VSCode::~VSCode() {}
+VSCode::~VSCode() = default;
 
 int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
   auto pos = source_map.find(sourceReference);
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -22,7 +22,7 @@
 
 using namespace lldb_vscode;
 
-StreamDescriptor::StreamDescriptor() {}
+StreamDescriptor::StreamDescriptor() = default;
 
 StreamDescriptor::StreamDescriptor(StreamDescriptor &) {
   *this = std::move(other);
Index: lldb/tools/debugserver/source/TTYState.cpp
===
--- lldb/tools/debugserver/source/TTYState.cpp
+++ lldb/tools/debugserver/source/TTYState.cpp
@@ -18,7 +18,7 @@
 TTYState::TTYState()
 : m_fd(-1), m_tflags(-1), m_ttystateErr(-1), m_processGroup(-1) {}
 
-TTYState::~TTYState() {}
+TTYState::~TTYState() = default;
 
 bool TTYState::GetTTYState(int fd, bool saveProcessGroup) {
   if (fd >= 0 && ::isatty(fd)) {
@@ -62,7 +62,7 @@
 
 TTYStateSwitcher::TTYStateSwitcher() : m_currentState(~0) {}
 
-TTYStateSwitcher::~TTYStateSwitcher() {}
+TTYStateSwitcher::~TTYStateSwitcher() = default;
 
 bool TTYStateSwitcher::GetState(uint32_t idx, int fd, bool saveProcessGroup) {
   if (ValidStateIndex(idx))
Index: lldb/tools/debugserver/source/StdStringExtractor.cpp
===
--- lldb/tools/debugserver/source/StdStringExtractor.cpp
+++ lldb/tools/debugserver/source/StdStringExtractor.cpp
@@ -30,7 +30,7 @@
 }
 
 // Destructor
-StdStringExtractor::~StdStringExtractor() {}
+StdStringExtractor::~StdStringExtractor() = default;
 
 char StdStringExtractor::GetChar(char fail_value) {
   if (m_index < m_packet.size()) {
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -28,7 +28,7 @@
 
 MachVMMemory::MachVMMemory() : m_page_size(kInvalidPageSize), m_err(0) {}
 
-MachVMMemory::~MachVMMemory() {}
+MachVMMemory::~MachVMMemory() = default;
 
 nub_size_t MachVMMemory::PageSize(task_t task) {
   if (m_page_size == kInvalidPageSize) {
Index: 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Host/macosx/cfcpp/CFCData.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
  lldb/source/Host/macosx/cfcpp/CFCString.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBDataRef.cpp
  lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
  lldb/tools/debugserver/source/MacOSX/CFString.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/StdStringExtractor.cpp
  lldb/tools/debugserver/source/TTYState.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/VSCode.cpp

Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -58,7 +58,7 @@
 log.reset(new std::ofstream(log_file_path));
 }
 
-VSCode::~VSCode() {}
+VSCode::~VSCode() = default;
 
 int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
   auto pos = source_map.find(sourceReference);
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -22,7 +22,7 @@
 
 using namespace lldb_vscode;
 
-StreamDescriptor::StreamDescriptor() {}
+StreamDescriptor::StreamDescriptor() = default;
 
 StreamDescriptor::StreamDescriptor(StreamDescriptor &) {
   *this = std::move(other);
Index: lldb/tools/debugserver/source/TTYState.cpp
===
--- lldb/tools/debugserver/source/TTYState.cpp
+++ lldb/tools/debugserver/source/TTYState.cpp
@@ -18,7 +18,7 @@
 TTYState::TTYState()
 : m_fd(-1), m_tflags(-1), m_ttystateErr(-1), m_processGroup(-1) {}
 
-TTYState::~TTYState() {}
+TTYState::~TTYState() = default;
 
 bool TTYState::GetTTYState(int fd, bool saveProcessGroup) {
   if (fd >= 0 && ::isatty(fd)) {
@@ -62,7 +62,7 @@
 
 TTYStateSwitcher::TTYStateSwitcher() : m_currentState(~0) {}
 
-TTYStateSwitcher::~TTYStateSwitcher() {}
+TTYStateSwitcher::~TTYStateSwitcher() = default;
 
 bool TTYStateSwitcher::GetState(uint32_t idx, int fd, bool saveProcessGroup) {
   if (ValidStateIndex(idx))
Index: lldb/tools/debugserver/source/StdStringExtractor.cpp
===
--- lldb/tools/debugserver/source/StdStringExtractor.cpp
+++ lldb/tools/debugserver/source/StdStringExtractor.cpp
@@ -30,7 +30,7 @@
 }
 
 // Destructor
-StdStringExtractor::~StdStringExtractor() {}
+StdStringExtractor::~StdStringExtractor() = default;
 
 char StdStringExtractor::GetChar(char fail_value) {
   if (m_index < m_packet.size()) {
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -28,7 +28,7 @@
 
 MachVMMemory::MachVMMemory() : m_page_size(kInvalidPageSize), m_err(0) {}
 
-MachVMMemory::~MachVMMemory() {}
+MachVMMemory::~MachVMMemory() = default;
 
 nub_size_t MachVMMemory::PageSize(task_t task) {
   if (m_page_size == kInvalidPageSize) {
Index: lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
===
--- 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 3 inline comments as done.
shafik added inline comments.



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

labath wrote:
> shafik wrote:
> > labath wrote:
> > > shafik wrote:
> > > > I have to look into why we return a `const &` here. We do this in a few 
> > > > other places too.
> > > I don't think there's a good reason for that. Most people aren't aware 
> > > that built-in assignment operators return lvalues. And some of the people 
> > > who are aware of that think that it's a bad idea, so they make sure their 
> > > operators don't do it...
> > It produced build errors, so some of the users are relying on this. I 
> > didn't want to plumb into this since it was orthogonal to the change.
> I was curious to see what kind of errors could be produced by that change -- 
> I didn't get any, so I committed (c484857b2e77721a4235b0e2d53d335c09fc6af3) 
> my version. :)
I see that, I am not sure what I did wrong that I got a bunch of errors the 
other day.


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://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 LAST ACTION
  https://reviews.llvm.org/D122753/new/

https://reviews.llvm.org/D122753

Files:
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 regardless of which one is active. I modified it to only call 
`Clear()` on the active member.


https://reviews.llvm.org/D122753

Files:
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 2 inline comments as done.
shafik added inline comments.



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

labath wrote:
> shafik wrote:
> > I have to look into why we return a `const &` here. We do this in a few 
> > other places too.
> I don't think there's a good reason for that. Most people aren't aware that 
> built-in assignment operators return lvalues. And some of the people who are 
> aware of that think that it's a bad idea, so they make sure their operators 
> don't do it...
It produced build errors, so some of the users are relying on this. I didn't 
want to plumb into this since it was orthogonal to the change.



Comment at: lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp:18
 CFCMutableArray::CFCMutableArray(const CFCMutableArray )
-: CFCReleaser(rhs) // NOTE: this won't make a copy of 
the
+: CFCReleaser(rhs) // 
NOLINT(modernize-use-equals-default)
+  // NOTE: this won't make a copy of 
the

labath wrote:
> Why suppress this?
I wanted to preserve the comment since someone thought it was important.


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://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 the places this could matter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121831/new/

https://reviews.llvm.org/D121831

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121831/new/

https://reviews.llvm.org/D121831

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp


Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -560,8 +560,8 @@
 const char *error_str;
   };
   
-  extern void *memcpy(void *, const void *, size_t size);
-  extern size_t strlen(const char *);
+  extern "C" void *memcpy(void *, const void *, size_t size);
+  extern "C" size_t strlen(const char *);
   
 
   void * __lldb_dlopen_wrapper (const char *name, 
@@ -608,7 +608,7 @@
   DiagnosticManager diagnostics;
 
   auto utility_fn_or_error = process->GetTarget().CreateUtilityFunction(
-  std::move(expr), dlopen_wrapper_name, eLanguageTypeObjC, exe_ctx);
+  std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
 error.SetErrorStringWithFormat("dlopen error: could not create utility"


Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -560,8 +560,8 @@
 const char *error_str;
   };
   
-  extern void *memcpy(void *, const void *, size_t size);
-  extern size_t strlen(const char *);
+  extern "C" void *memcpy(void *, const void *, size_t size);
+  extern "C" size_t strlen(const char *);
   
 
   void * __lldb_dlopen_wrapper (const char *name, 
@@ -608,7 +608,7 @@
   DiagnosticManager diagnostics;
 
   auto utility_fn_or_error = process->GetTarget().CreateUtilityFunction(
-  std::move(expr), dlopen_wrapper_name, eLanguageTypeObjC, exe_ctx);
+  std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
 error.SetErrorStringWithFormat("dlopen error: could not create utility"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



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

I have to look into why we return a `const &` here. We do this in a few other 
places too.


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://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D121844

Files:
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Host/macosx/cfcpp/CFCData.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
  lldb/source/Host/macosx/cfcpp/CFCString.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBDataRef.cpp
  lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
  lldb/tools/debugserver/source/MacOSX/CFString.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/StdStringExtractor.cpp
  lldb/tools/debugserver/source/TTYState.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/VSCode.cpp

Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -58,7 +58,7 @@
 log.reset(new std::ofstream(log_file_path));
 }
 
-VSCode::~VSCode() {}
+VSCode::~VSCode() = default;
 
 int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
   auto pos = source_map.find(sourceReference);
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -22,7 +22,7 @@
 
 using namespace lldb_vscode;
 
-StreamDescriptor::StreamDescriptor() {}
+StreamDescriptor::StreamDescriptor() = default;
 
 StreamDescriptor::StreamDescriptor(StreamDescriptor &) {
   *this = std::move(other);
Index: lldb/tools/debugserver/source/TTYState.cpp
===
--- lldb/tools/debugserver/source/TTYState.cpp
+++ lldb/tools/debugserver/source/TTYState.cpp
@@ -18,7 +18,7 @@
 TTYState::TTYState()
 : m_fd(-1), m_tflags(-1), m_ttystateErr(-1), m_processGroup(-1) {}
 
-TTYState::~TTYState() {}
+TTYState::~TTYState() = default;
 
 bool TTYState::GetTTYState(int fd, bool saveProcessGroup) {
   if (fd >= 0 && ::isatty(fd)) {
@@ -62,7 +62,7 @@
 
 TTYStateSwitcher::TTYStateSwitcher() : m_currentState(~0) {}
 
-TTYStateSwitcher::~TTYStateSwitcher() {}
+TTYStateSwitcher::~TTYStateSwitcher() = default;
 
 bool TTYStateSwitcher::GetState(uint32_t idx, int fd, bool saveProcessGroup) {
   if (ValidStateIndex(idx))
Index: lldb/tools/debugserver/source/StdStringExtractor.cpp
===
--- lldb/tools/debugserver/source/StdStringExtractor.cpp
+++ lldb/tools/debugserver/source/StdStringExtractor.cpp
@@ -30,7 +30,7 @@
 }
 
 // Destructor
-StdStringExtractor::~StdStringExtractor() {}
+StdStringExtractor::~StdStringExtractor() = default;
 
 char StdStringExtractor::GetChar(char fail_value) {
   if (m_index < m_packet.size()) {
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -28,7 +28,7 @@
 
 MachVMMemory::MachVMMemory() : m_page_size(kInvalidPageSize), m_err(0) {}
 
-MachVMMemory::~MachVMMemory() {}
+MachVMMemory::~MachVMMemory() = default;
 
 nub_size_t MachVMMemory::PageSize(task_t task) {
   if (m_page_size == kInvalidPageSize) {
Index: lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 `eLanguageTypeC_plus_plus`. So I am modifying it 
to be consistent.


https://reviews.llvm.org/D121831

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp


Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -560,8 +560,8 @@
 const char *error_str;
   };
   
-  extern void *memcpy(void *, const void *, size_t size);
-  extern size_t strlen(const char *);
+  extern "C" void *memcpy(void *, const void *, size_t size);
+  extern "C" size_t strlen(const char *);
   
 
   void * __lldb_dlopen_wrapper (const char *name, 
@@ -608,7 +608,7 @@
   DiagnosticManager diagnostics;
 
   auto utility_fn_or_error = process->GetTarget().CreateUtilityFunction(
-  std::move(expr), dlopen_wrapper_name, eLanguageTypeObjC, exe_ctx);
+  std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
 error.SetErrorStringWithFormat("dlopen error: could not create utility"


Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -560,8 +560,8 @@
 const char *error_str;
   };
   
-  extern void *memcpy(void *, const void *, size_t size);
-  extern size_t strlen(const char *);
+  extern "C" void *memcpy(void *, const void *, size_t size);
+  extern "C" size_t strlen(const char *);
   
 
   void * __lldb_dlopen_wrapper (const char *name, 
@@ -608,7 +608,7 @@
   DiagnosticManager diagnostics;
 
   auto utility_fn_or_error = process->GetTarget().CreateUtilityFunction(
-  std::move(expr), dlopen_wrapper_name, eLanguageTypeObjC, exe_ctx);
+  std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
 error.SetErrorStringWithFormat("dlopen error: could not create utility"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 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_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -0,0 +1,316 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 0
+# CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
+# CHECK: }
+
+# We are testing how DWARFExpression::Evaluate(...) in the case of
+# DW_OP_deref_size deals with static variable.
+#
+# The address will be a ValueType::FileAddress type which we will not be able
+# to turn into a load address but it is a section offset. We should be able to
+# use Target::ReadMemory(...) to read the data in this case.
+# Compiling at -O1 allows us to capture this case and test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
+#
+# Hand modified .s file to remoce various section that are not needed for this
+# test
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -0,0 +1,316 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 0
+# CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
+# CHECK: }
+
+# We are testing how DWARFExpression::Evaluate(...) in the case of
+# DW_OP_deref_size deals with static variable.
+#
+# The address will be a ValueType::FileAddress type which we will not be able
+# to turn into a load address but it is a section offset. We should be able to
+# use Target::ReadMemory(...) to read the data in this case.
+# Compiling at -O1 allows us to capture this case and test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
+#
+# Hand modified .s file to remoce various section that are not needed for this
+# test
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11   

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D121605/new/

https://reviews.llvm.org/D121605

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -3,7 +3,7 @@
 
 # CHECK: (lldb) target variable ug
 # CHECK: (U) ug = {
-# CHECK:   raw = 0 
+# CHECK:   raw = 0
 # CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
 # CHECK: }
 
@@ -48,7 +48,7 @@
 #   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
 #
 # Hand modified .s file to remoce various section that are not needed for this
-# test 
+# test
 
 .zerofill __DATA,__bss,_ug.0,1,2## @ug.0
 .zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
@@ -199,7 +199,7 @@
 	.long	105 ## DW_AT_name
 	.long	129 ## DW_AT_LLVM_sysroot
 	.long	185 ## DW_AT_APPLE_sdk
-	.long 0	
+	.long 0
 	.long	200 ## DW_AT_comp_dir
 ## DW_AT_APPLE_optimized
 	.byte	2   ## Abbrev [2] 0x26:0x15 DW_TAG_variable
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -944,6 +944,80 @@
 }
 } // namespace
 
+/// Helper function to move common code used to resolve a file address and turn
+/// into a load address.
+///
+/// \param exe_ctx Pointer to the execution context
+/// \param module_sp shared_ptr contains the module if we have one
+/// \param error_ptr pointer to Status object if we have one
+/// \param dw_op_type C-style string used to vary the error output
+/// \param file_addr the file address we are trying to resolve and turn into a
+///  load address
+/// \param so_addr out parameter, will be set to load addresss or section offset
+/// \param check_sectionoffset bool which determines if having a section offset
+///but not a load address is considerd a success
+/// \returns llvm::Optional containing the load address if resolving and getting
+///  the load address succeed or an empty Optinal otherwise. If
+///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
+///  success if so_addr.IsSectionOffset() is true.
+static llvm::Optional
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+  Status *error_ptr, const char *dw_op_type,
+  lldb::addr_t file_addr, Address _addr,
+  bool check_sectionoffset = false) {
+  if (!module_sp) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "need module to resolve file address for %s", dw_op_type);
+return {};
+  }
+
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve file address in module");
+return {};
+  }
+
+  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+
+  if (load_addr == LLDB_INVALID_ADDRESS &&
+  (check_sectionoffset && !so_addr.IsSectionOffset())) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve load address");
+return {};
+  }
+
+  return load_addr;
+}
+
+/// Helper function to move common code used to load sized data from a uint8_t
+/// buffer.
+///
+/// \param addr_bytes uint8_t buffer containg raw data
+/// \param size_addr_bytes how large is the underlying raw data
+/// \param byte_order what is the byter order of the underlyig data
+/// \param size How much of the underlying data we want to use
+/// \return The underlying data converted into a Scalar
+static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
+ size_t size_addr_bytes,
+ ByteOrder byte_order, size_t size) {
+  DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
+
+  lldb::offset_t addr_data_offset = 0;
+  switch (size) {
+  case 1:
+return addr_data.GetU8(_data_offset);
+  case 2:
+return addr_data.GetU16(_data_offset);
+  case 4:
+return addr_data.GetU32(_data_offset);
+  case 8:
+return addr_data.GetU64(_data_offset);
+  default:
+return addr_data.GetAddress(_data_offset);
+  }
+}
+
 bool DWARFExpression::Evaluate(
 ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
 lldb::ModuleSP module_sp, const DataExtractor ,
@@ -1026,6 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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
  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.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/VectorType.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Host/common/HostNativeThreadBase.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/source/Utility/Instrumentation.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/source/Utility/Status.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/StringExtractor.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Host/FileSystemTest.cpp
  lldb/unittests/Process/ProcessEventDataTest.cpp

Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -82,28 +82,26 @@
 
 class DummyStopInfo : public StopInfo {
 public:
-  DummyStopInfo(Thread , uint64_t value)
-  : StopInfo(thread, value), m_should_stop(true),
-m_stop_reason(eStopReasonBreakpoint) {}
+  DummyStopInfo(Thread , uint64_t value) : StopInfo(thread, value) {}
 
   bool ShouldStop(Event *event_ptr) override { return m_should_stop; }
 
   StopReason GetStopReason() const override { return m_stop_reason; }
 
-  bool m_should_stop;
-  StopReason m_stop_reason;
+  bool m_should_stop = true;
+  StopReason m_stop_reason = eStopReasonBreakpoint;
 };
 
 class DummyProcessEventData : public Process::ProcessEventData {
 public:
   DummyProcessEventData(ProcessSP _sp, StateType state)
-  : ProcessEventData(process_sp, state), m_should_stop_hit_count(0) {}
+  : ProcessEventData(process_sp, state) {}
   bool ShouldStop(Event *event_ptr, bool _valid_stopinfo) override {
 m_should_stop_hit_count++;
 return false;
   }
 
-  int m_should_stop_hit_count;
+  int m_should_stop_hit_count = 0;
 };
 } // namespace
 
Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -34,7 +34,7 @@
 
 class DummyFileSystem : public vfs::FileSystem {
   int FSID;   // used to produce UniqueIDs
-  int FileID; // used to produce UniqueIDs
+  int FileID = 0; // used to produce UniqueIDs
   std::string cwd;
   std::map FilesAndDirs;
 
@@ -44,7 +44,7 @@
   }
 
 public:
-  DummyFileSystem() : FSID(getNextFSID()), FileID(0) {}
+  DummyFileSystem() : 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/VectorType.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Host/common/HostNativeThreadBase.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/source/Utility/Instrumentation.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/source/Utility/Status.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/StringExtractor.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Host/FileSystemTest.cpp
  lldb/unittests/Process/ProcessEventDataTest.cpp

Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -82,28 +82,26 @@
 
 class DummyStopInfo : public StopInfo {
 public:
-  DummyStopInfo(Thread , uint64_t value)
-  : StopInfo(thread, value), m_should_stop(true),
-m_stop_reason(eStopReasonBreakpoint) {}
+  DummyStopInfo(Thread , uint64_t value) : StopInfo(thread, value) {}
 
   bool ShouldStop(Event *event_ptr) override { return m_should_stop; }
 
   StopReason GetStopReason() const override { return m_stop_reason; }
 
-  bool m_should_stop;
-  StopReason m_stop_reason;
+  bool m_should_stop = true;
+  StopReason m_stop_reason = eStopReasonBreakpoint;
 };
 
 class DummyProcessEventData : public Process::ProcessEventData {
 public:
   DummyProcessEventData(ProcessSP _sp, StateType state)
-  : ProcessEventData(process_sp, state), m_should_stop_hit_count(0) {}
+  : ProcessEventData(process_sp, state) {}
   bool ShouldStop(Event *event_ptr, bool _valid_stopinfo) override {
 m_should_stop_hit_count++;
 return false;
   }
 
-  int m_should_stop_hit_count;
+  int m_should_stop_hit_count = 0;
 };
 } // namespace
 
Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -34,7 +34,7 @@
 
 class DummyFileSystem : public vfs::FileSystem {
   int FSID;   // used to produce UniqueIDs
-  int FileID; // used to produce UniqueIDs
+  int FileID = 0; // used to produce UniqueIDs
   std::string cwd;
   std::map FilesAndDirs;
 
@@ -44,7 +44,7 @@
   }
 
 public:
-  DummyFileSystem() : FSID(getNextFSID()), FileID(0) {}
+  DummyFileSystem() : FSID(getNextFSID()) {}
 
   ErrorOr status(const Twine ) override {
 std::map::iterator I =
Index: lldb/unittests/Editline/EditlineTest.cpp

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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-member-init` clang-tidy check over LLDB.

It appears in many files we had already switched to in class member init but 
never updated the constructors to reflect that.

This check is already present in the `lldb/.clang-tidy` config.


https://reviews.llvm.org/D121481

Files:
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/API/SBListener.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/VectorType.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Host/common/HostNativeThreadBase.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/source/Utility/Instrumentation.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/source/Utility/Status.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/StringExtractor.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Host/FileSystemTest.cpp
  lldb/unittests/Process/ProcessEventDataTest.cpp

Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -83,27 +83,27 @@
 class DummyStopInfo : public StopInfo {
 public:
   DummyStopInfo(Thread , uint64_t value)
-  : StopInfo(thread, value), m_should_stop(true),
-m_stop_reason(eStopReasonBreakpoint) {}
+  : StopInfo(thread, value)
+{}
 
   bool ShouldStop(Event *event_ptr) override { return m_should_stop; }
 
   StopReason GetStopReason() const override { return m_stop_reason; }
 
-  bool m_should_stop;
-  StopReason m_stop_reason;
+  bool m_should_stop = true;
+  StopReason m_stop_reason = eStopReasonBreakpoint;
 };
 
 class DummyProcessEventData : public Process::ProcessEventData {
 public:
   DummyProcessEventData(ProcessSP _sp, StateType state)
-  : ProcessEventData(process_sp, state), m_should_stop_hit_count(0) {}
+  : ProcessEventData(process_sp, state) {}
   bool ShouldStop(Event *event_ptr, bool _valid_stopinfo) override {
 m_should_stop_hit_count++;
 return false;
   }
 
-  int m_should_stop_hit_count;
+  int m_should_stop_hit_count = 0;
 };
 } // namespace
 
Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -34,7 +34,7 @@
 
 class DummyFileSystem : public vfs::FileSystem {
   int FSID;   // used to produce UniqueIDs
-  int FileID; // used to produce UniqueIDs
+  int FileID = 0; // used to produce UniqueIDs

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 wrote:
> Should this also be in the anonymous namespace?
Good catch although `static` is probably more appropriate: 
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces



Comment at: lldb/source/Expression/DWARFExpression.cpp:1141
 Address so_addr;
-if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
-  if (error_ptr)
-error_ptr->SetErrorString(
-"failed to resolve file address in module");
+auto load_Addr_optional = ResolveAndLoadFileAddress(
+exe_ctx, module_sp, error_ptr, "DW_OP_deref", file_addr, so_addr);

aprantl wrote:
> aprantl wrote:
> > why is `Addr` captialized?
> we sometimes call these `maybe_load_addr` or `load_addr_or_err` not sure if 
> that's better :-)
`load_Addr` was what it was before, I didn't even notice it.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1282
+
+  const bool force_live_memory = true;
+  if (exe_ctx->GetTargetRef().ReadMemory(so_addr, _bytes, size,

aprantl wrote:
> Why do we need to force live memory?
Good catch, we don't.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1299
+  }
+} else if (load_Addr == LLDB_INVALID_ADDRESS) {
+  if (error_ptr)

aprantl wrote:
> same here, but maybe we can just sink this into the helper.
Makes sense, it was a little tricky but I think with these changes it is neater.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121408/new/

https://reviews.llvm.org/D121408

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/

https://reviews.llvm.org/D121408

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -3,7 +3,7 @@
 
 # CHECK: (lldb) target variable ug
 # CHECK: (U) ug = {
-# CHECK:   raw = 0 
+# CHECK:   raw = 0
 # CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
 # CHECK: }
 
@@ -48,7 +48,7 @@
 #   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
 #
 # Hand modified .s file to remoce various section that are not needed for this
-# test 
+# test
 
 .zerofill __DATA,__bss,_ug.0,1,2## @ug.0
 .zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
@@ -199,7 +199,7 @@
 	.long	105 ## DW_AT_name
 	.long	129 ## DW_AT_LLVM_sysroot
 	.long	185 ## DW_AT_APPLE_sdk
-	.long 0	
+	.long 0
 	.long	200 ## DW_AT_comp_dir
 ## DW_AT_APPLE_optimized
 	.byte	2   ## Abbrev [2] 0x26:0x15 DW_TAG_variable
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -944,6 +944,80 @@
 }
 } // namespace
 
+/// Helper function to move common code used to resolve a file address and turn
+/// into a load address.
+///
+/// \param exe_ctx Pointer to the execution context
+/// \param module_sp shared_ptr contains the module if we have one
+/// \param error_ptr pointer to Status object if we have one
+/// \param dw_op_type C-style string used to vary the error output
+/// \param file_addr the file address we are trying to resolve and turn into a
+///  load address
+/// \param so_addr out parameter, will be set to load addresss or section offset
+/// \param check_sectionoffset bool which determines if having a section offset
+///but not a load address is considerd a success
+/// \returns llvm::Optional containing the load address if resolving and getting
+///  the load address succeed or an empty Optinal otherwise. If
+///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
+///  success if so_addr.IsSectionOffset() is true.
+static llvm::Optional
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+  Status *error_ptr, const char *dw_op_type,
+  lldb::addr_t file_addr, Address _addr,
+  bool check_sectionoffset = false) {
+  if (!module_sp) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "need module to resolve file address for %s", dw_op_type);
+return {};
+  }
+
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve file address in module");
+return {};
+  }
+
+  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+
+  if (load_addr == LLDB_INVALID_ADDRESS &&
+  (check_sectionoffset && !so_addr.IsSectionOffset())) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve load address");
+return {};
+  }
+
+  return load_addr;
+}
+
+/// Helper function to move common code used to load sized data from a uint8_t
+/// buffer.
+///
+/// \param addr_bytes uint8_t buffer containg raw data
+/// \param size_addr_bytes how large is the underlying raw data
+/// \param byte_order what is the byter order of the underlyig data
+/// \param size How much of the underlying data we want to use
+/// \return The underlying data converted into a Scalar
+static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
+ size_t size_addr_bytes,
+ ByteOrder byte_order, size_t size) {
+  DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
+
+  lldb::offset_t addr_data_offset = 0;
+  switch (size) {
+  case 1:
+return addr_data.GetU8(_data_offset);
+  case 2:
+return addr_data.GetU16(_data_offset);
+  case 4:
+return addr_data.GetU32(_data_offset);
+  case 8:
+return addr_data.GetU64(_data_offset);
+  default:
+return addr_data.GetAddress(_data_offset);
+  }
+}
+
 bool DWARFExpression::Evaluate(
 ExecutionContext 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/new/

https://reviews.llvm.org/D121408

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 and 
does not attempt to handle it. This adds support for this case and a test that 
verifies this support.

I did a little refactoring since `DW_OP_deref` and `DW_OP_deref_size` have some 
overlap in code.


https://reviews.llvm.org/D121408

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -0,0 +1,316 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 0
+# CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
+# CHECK: }
+
+# We are testing how DWARFExpression::Evaluate(...) in the case of
+# DW_OP_deref_size deals with static variable.
+#
+# The address will be a ValueType::FileAddress type which we will not be able
+# to turn into a load address but it is a section offset. We should be able to
+# use Target::ReadMemory(...) to read the data in this case.
+# Compiling at -O1 allows us to capture this case and test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
+#
+# Hand modified .s file to remoce various section that are not needed for this
+# test
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 initialization. Is this being flagged by something?
>
> It is getting flagged by -Wglobal-constructor but you're right, it's 
> constexpr and therefore shouldn't. Might be a bug in clang. I'll try to repro 
> with ToT.

I wonder if for `ArrayRef` the copy constructor does not look `constexpr` and 
so if you change it to `constexpr static` if it will be ill-formed, that would 
mean it is not really `constant initialization` and therefore that would make 
it dynamic since it also not static initialization since you are calling a 
constructor.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121161/new/

https://reviews.llvm.org/D121161

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/D120966/new/

https://reviews.llvm.org/D120966

Files:
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/test/API/api/multithreaded/inferior.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
  lldb/test/API/functionalities/process_save_core_minidump/main.cpp
  lldb/test/API/functionalities/thread/concurrent_events/main.cpp
  lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp

Index: lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
===
--- lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
+++ lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
@@ -2,8 +2,6 @@
 // Link with "link test-pdb-types.obj /debug /nodefaultlib /entry:main
 // /out:test-pdb-types.exe"
 
-using namespace std;
-
 // Sizes of builtin types
 static const int sizeof_char = sizeof(char);
 static const int sizeof_uchar = sizeof(unsigned char);
Index: lldb/test/API/functionalities/thread/concurrent_events/main.cpp
===
--- lldb/test/API/functionalities/thread/concurrent_events/main.cpp
+++ lldb/test/API/functionalities/thread/concurrent_events/main.cpp
@@ -6,7 +6,6 @@
 
 #include "pseudo_barrier.h"
 #include 
-using namespace std;
 
 #include 
 
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- lldb/test/API/functionalities/process_save_core_minidump/main.cpp
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -2,8 +2,6 @@
 #include 
 #include 
 
-using namespace std;
-
 void g() { assert(false); }
 
 void f() { g(); }
@@ -19,12 +17,12 @@
 }
 
 int main() {
-  thread t1(f);
+  std::thread t1(f);
 
   size_t x = h();
 
   t1.join();
 
-  cout << "X is " << x << "\n";
+  std::cout << "X is " << x << "\n";
   return 0;
-}
\ No newline at end of file
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
@@ -1,11 +1,9 @@
 #include 
 #include 
 
-using namespace std;
-
 int main() {
-  tuple<> empty;
-  tuple one_elt{47};
-  tuple three_elts{1, 47l, "foo"};
+  std::tuple<> empty;
+  std::tuple one_elt{47};
+  std::tuple three_elts{1, 47l, "foo"};
   return 0; // break here
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
@@ -1,11 +1,9 @@
 #include 
 #include 
 
-using namespace std;
-
 int main() {
-  queue q1{{1,2,3,4,5}};
-  queue> q2{{1,2,3,4,5}};
+  std::queue q1{{1,2,3,4,5}};
+  std::queue> q2{{1,2,3,4,5}};
   int ret = q1.size() + q2.size(); // break here
   return ret;
 }
Index: lldb/test/API/api/multithreaded/inferior.cpp
===
--- lldb/test/API/api/multithreaded/inferior.cpp
+++ lldb/test/API/api/multithreaded/inferior.cpp
@@ -1,11 +1,11 @@
 
 #include 
 
-using namespace std;
+
 
 int next() {
   static int i = 0;
-  cout << "incrementing " << i << endl;
+  std::cout << "incrementing " << i << std::endl;
   return ++i;
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -26,7 +26,6 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::dwarf;
-using namespace std;
 
 extern int g_verbose;
 
@@ -449,7 +448,7 @@
 
   uint64_t HeaderSize = llvm::DWARFListTableHeader::getHeaderSize(format);
   if (offset < HeaderSize)
-return llvm::createStringError(errc::invalid_argument,
+return llvm::createStringError(std::errc::invalid_argument,
"did not detect a valid"
" list table with base = 0x%" PRIx64 "\n",
  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 
.


https://reviews.llvm.org/D120966

Files:
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/test/API/api/multithreaded/inferior.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
  lldb/test/API/functionalities/process_save_core_minidump/main.cpp
  lldb/test/API/functionalities/thread/concurrent_events/main.cpp
  lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp

Index: lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
===
--- lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
+++ lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
@@ -2,8 +2,6 @@
 // Link with "link test-pdb-types.obj /debug /nodefaultlib /entry:main
 // /out:test-pdb-types.exe"
 
-using namespace std;
-
 // Sizes of builtin types
 static const int sizeof_char = sizeof(char);
 static const int sizeof_uchar = sizeof(unsigned char);
Index: lldb/test/API/functionalities/thread/concurrent_events/main.cpp
===
--- lldb/test/API/functionalities/thread/concurrent_events/main.cpp
+++ lldb/test/API/functionalities/thread/concurrent_events/main.cpp
@@ -6,7 +6,6 @@
 
 #include "pseudo_barrier.h"
 #include 
-using namespace std;
 
 #include 
 
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- lldb/test/API/functionalities/process_save_core_minidump/main.cpp
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -2,8 +2,6 @@
 #include 
 #include 
 
-using namespace std;
-
 void g() { assert(false); }
 
 void f() { g(); }
@@ -19,12 +17,12 @@
 }
 
 int main() {
-  thread t1(f);
+  std::thread t1(f);
 
   size_t x = h();
 
   t1.join();
 
-  cout << "X is " << x << "\n";
+  std::cout << "X is " << x << "\n";
   return 0;
-}
\ No newline at end of file
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/main.cpp
@@ -1,11 +1,9 @@
 #include 
 #include 
 
-using namespace std;
-
 int main() {
-  tuple<> empty;
-  tuple one_elt{47};
-  tuple three_elts{1, 47l, "foo"};
+  std::tuple<> empty;
+  std::tuple one_elt{47};
+  std::tuple three_elts{1, 47l, "foo"};
   return 0; // break here
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/main.cpp
@@ -1,11 +1,9 @@
 #include 
 #include 
 
-using namespace std;
-
 int main() {
-  queue q1{{1,2,3,4,5}};
-  queue> q2{{1,2,3,4,5}};
+  std::queue q1{{1,2,3,4,5}};
+  std::queue> q2{{1,2,3,4,5}};
   int ret = q1.size() + q2.size(); // break here
   return ret;
 }
Index: lldb/test/API/api/multithreaded/inferior.cpp
===
--- lldb/test/API/api/multithreaded/inferior.cpp
+++ lldb/test/API/api/multithreaded/inferior.cpp
@@ -1,11 +1,11 @@
 
 #include 
 
-using namespace std;
+
 
 int next() {
   static int i = 0;
-  cout << "incrementing " << i << endl;
+  std::cout << "incrementing " << i << std::endl;
   return ++i;
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -26,7 +26,6 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::dwarf;
-using namespace std;
 
 extern int g_verbose;
 
@@ -449,7 +448,7 @@
 
   uint64_t HeaderSize = llvm::DWARFListTableHeader::getHeaderSize(format);
   if (offset < HeaderSize)
-return llvm::createStringError(errc::invalid_argument,
+return llvm::createStringError(std::errc::invalid_argument,
"did not detect a valid"
  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D120836/new/

https://reviews.llvm.org/D120836

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/PostfixExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -39,6 +39,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 class SymbolFileDWARFTests : public testing::Test {
   SubsystemRAII Evaluate(llvm::ArrayRef expr,
lldb::ModuleSP module_sp = {},
Index: lldb/source/Symbol/PostfixExpression.cpp
===
--- lldb/source/Symbol/PostfixExpression.cpp
+++ lldb/source/Symbol/PostfixExpression.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
+using namespace lldb_private::dwarf;
 
 static llvm::Optional
 GetBinaryOpType(llvm::StringRef token) {
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -24,6 +24,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 // GetDwarfEHPtr
 //
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -80,6 +80,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 using namespace clang;
 using llvm::StringSwitch;
 
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
+using namespace lldb_private::dwarf;
 using namespace llvm::pdb;
 
 static std::unique_ptr
Index: lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -10,6 +10,8 @@
 
 #include "lldb/Core/Declaration.h"
 
+using namespace lldb_private::dwarf;
+
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -96,6 +96,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 LLDB_PLUGIN_DEFINE(SymbolFileDWARF)
 
Index: 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/

https://reviews.llvm.org/D120836

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/PostfixExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -39,6 +39,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 class SymbolFileDWARFTests : public testing::Test {
   SubsystemRAII Evaluate(llvm::ArrayRef expr,
lldb::ModuleSP module_sp = {},
Index: lldb/source/Symbol/PostfixExpression.cpp
===
--- lldb/source/Symbol/PostfixExpression.cpp
+++ lldb/source/Symbol/PostfixExpression.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
+using namespace lldb_private::dwarf;
 
 static llvm::Optional
 GetBinaryOpType(llvm::StringRef token) {
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -24,6 +24,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 // GetDwarfEHPtr
 //
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -80,6 +80,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 using namespace clang;
 using llvm::StringSwitch;
 
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
+using namespace lldb_private::dwarf;
 using namespace llvm::pdb;
 
 static std::unique_ptr
Index: lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -10,6 +10,8 @@
 
 #include "lldb/Core/Declaration.h"
 
+using namespace lldb_private::dwarf;
+
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -96,6 +96,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::dwarf;
 
 LLDB_PLUGIN_DEFINE(SymbolFileDWARF)
 
Index: 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 that by mistake.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19
 using namespace lldb;
+using namespace dwarf;
 

JDevlieghere wrote:
> Wouldn't it be more consistent to use `lldb_private::dwarf` everywhere? 
Yeah, I was trying to match the style of  how it was done for minidump, see 
`source/Plugins/Process/minidump/MinidumpParser.cpp`.

Since we need access to both namespaces but in files that did already have 
`using namespace lldb_private;` I just used the less verbose version.

I can see arguments for just being verbose everywhere.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:35
+using namespace dwarf;
 using namespace std;
 extern int g_verbose;

JDevlieghere wrote:
> [[ https://llvm.org/docs/CodingStandards.html#do-not-use-using-namespace-std 
> | Oh no... ]]
Yeah I noticed that was going to put that on a list of things to do, didn't 
feel right mixing that in here as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120836/new/

https://reviews.llvm.org/D120836

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/PostfixExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -39,6 +39,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 
 class SymbolFileDWARFTests : public testing::Test {
   SubsystemRAII Evaluate(llvm::ArrayRef expr,
lldb::ModuleSP module_sp = {},
Index: lldb/source/Symbol/PostfixExpression.cpp
===
--- lldb/source/Symbol/PostfixExpression.cpp
+++ lldb/source/Symbol/PostfixExpression.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
+using namespace llvm::dwarf;
 
 static llvm::Optional
 GetBinaryOpType(llvm::StringRef token) {
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -24,6 +24,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm::dwarf;
 
 // GetDwarfEHPtr
 //
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -80,6 +80,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 using namespace clang;
 using llvm::StringSwitch;
 
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
+using namespace lldb_private::dwarf;
 using namespace llvm::pdb;
 
 static std::unique_ptr
Index: lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -10,6 +10,8 @@
 
 #include "lldb/Core/Declaration.h"
 
+using namespace lldb_private::dwarf;
+
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -96,6 +96,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 
 LLDB_PLUGIN_DEFINE(SymbolFileDWARF)
 
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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` -- is not reasonable, but that's 
> because the plugin is very old, and completely unnamespaced.
>
> For the newer plugins, we've started putting them in a sub-namespace of 
> lldb_private, which means they cannot be accidentally referenced from the 
> core code, but they can easily reference (without qualification) symbols 
> defined in the core libraries.
>
> If we put the dwarf plugin into a (e.g.) `lldb_private::dwarf` namespace, 
> then I think it would be ok to put a `using namespace llvm::dwarf` into that 
> namespace, even in a header -- because those symbols would only be visible to 
> symbols which are in that namespace.
>
> In other words, I'd have the dwarf plugin do what the minidump plugin is 
> doing (disclaimer: I wrote most of that plugin).
>
> Anyway, I'm not married to that approach, but that's what I would do...

If I understand correctly you are proposing that I do:

  namespace lldb_private {
  namespace dwarf {
using namespace llvm::dwarf;
  }
  }

in `lldb/include/lldb/Core/dwarf.h` and then in the `.cpp` files:

  using namespace lldb_private;
  using namespace dwarf;

That sounds reasonable to me.

Then I would revert the changes to the minidump files, since they are already 
using this strategy.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120836/new/

https://reviews.llvm.org/D120836

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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` in a few header files but I wanted to 
tackle that separately based on the feedback on this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120836/new/

https://reviews.llvm.org/D120836

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.h` and none 
of the files it includes directly use `using namespace llvm;`.

It looks like across llvm-project we have 54 headers that use `using namespace 
llvm;` I thought it was a lot more.

I personally dislike it even in `.cpp` files using `llvm::` or `lldb_private::` 
does not feel like an undue burden. Although the count for `.cpp` files is 
4000+ 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120718/new/

https://reviews.llvm.org/D120718

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120718/new/

https://reviews.llvm.org/D120718

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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
  https://reviews.llvm.org/D120105/new/

https://reviews.llvm.org/D120105

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -15,7 +15,6 @@
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServer.h"
-#include "GDBRemoteCommunicationServerCommon.h"
 
 class StringExtractorGDBRemote;
 


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -15,7 +15,6 @@
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServer.h"
-#include "GDBRemoteCommunicationServerCommon.h"
 
 class StringExtractorGDBRemote;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/GDBRemoteCommunicationServerCommon.h


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -15,7 +15,6 @@
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServer.h"
-#include "GDBRemoteCommunicationServerCommon.h"
 
 class StringExtractorGDBRemote;
 


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -15,7 +15,6 @@
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServer.h"
-#include "GDBRemoteCommunicationServerCommon.h"
 
 class StringExtractorGDBRemote;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119178/new/

https://reviews.llvm.org/D119178

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-structured-binding.cpp
  lldb/test/API/lang/cpp/structured-binding/Makefile
  lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
  lldb/test/API/lang/cpp/structured-binding/main.cpp

Index: lldb/test/API/lang/cpp/structured-binding/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/main.cpp
@@ -0,0 +1,69 @@
+// Structured binding in C++ can bind identifiers to subobjects of an object.
+//
+// There are three cases we need to test:
+// 1) arrays
+// 2) tuples like objects
+// 3) non-static data members
+//
+// They can also bind by copy, reference or rvalue reference.
+
+#include 
+
+struct A {
+  int x;
+  int y;
+};
+
+// We want to cover a mix of types and also different sizes to make sure we
+// hande the offsets correctly.
+struct MixedTypesAndSizesStruct {
+  A a;
+  char b1;
+  char b2;
+  short b3;
+  int b4;
+  char b5;
+};
+
+int main() {
+  MixedTypesAndSizesStruct b{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  auto [a1, b1, c1, d1, e1, f1] = b;
+  auto &[a2, b2, c2, d2, e2, f2] = b;
+  auto &&[a3, b3, c3, d3, e3, f3] =
+  MixedTypesAndSizesStruct{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  // Array with different sized types
+  char carr[]{'a', 'b', 'c'};
+  short sarr[]{11, 12, 13};
+  int iarr[]{22, 33, 44};
+
+  auto [carr_copy1, carr_copy2, carr_copy3] = carr;
+  auto [sarr_copy1, sarr_copy2, sarr_copy3] = sarr;
+  auto [iarr_copy1, iarr_copy2, iarr_copy3] = iarr;
+
+  auto &[carr_ref1, carr_ref2, carr_ref3] = carr;
+  auto &[sarr_ref1, sarr_ref2, sarr_ref3] = sarr;
+  auto &[iarr_ref1, iarr_ref2, iarr_ref3] = iarr;
+
+  auto &&[carr_rref1, carr_rref2, carr_rref3] = carr;
+  auto &&[sarr_rref1, sarr_rref2, sarr_rref3] = sarr;
+  auto &&[iarr_rref1, iarr_rref2, iarr_rref3] = iarr;
+
+  float x{4.0};
+  char y{'z'};
+  int z{10};
+
+  std::tuple tpl(x, y, z);
+  auto [tx1, ty1, tz1] = tpl;
+  auto &[tx2, ty2, tz2] = tpl;
+
+  return a1.x + b1 + c1 + d1 + e1 + f1 + a2.y + b2 + c2 + d2 + e2 + f2 + a3.x +
+ b3 + c3 + d3 + e3 + f3 + carr_copy1 + carr_copy2 + carr_copy3 +
+ sarr_copy1 + sarr_copy2 + sarr_copy3 + iarr_copy1 + iarr_copy2 +
+ iarr_copy3 + carr_ref1 + carr_ref2 + carr_ref3 + sarr_ref1 +
+ sarr_ref2 + sarr_ref3 + iarr_ref1 + iarr_ref2 + iarr_ref3 +
+ carr_rref1 + carr_rref2 + carr_rref3 + sarr_rref1 + sarr_rref2 +
+ sarr_rref3 + iarr_rref1 + iarr_rref2 + iarr_rref3 + tx1 + ty1 + tz1 +
+ tx2 + ty2 + tz2; // break here
+}
Index: lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
@@ -0,0 +1,84 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestStructuredBinding(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(compiler="clang", compiler_version=['<', '14.0'])
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("a1", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b1", result_type="char", result_value="'a'")
+self.expect_expr("c1", result_type="char", result_value="'b'")
+self.expect_expr("d1", result_type="short", result_value="50")
+self.expect_expr("e1", result_type="int", result_value="60")
+self.expect_expr("f1", result_type="char", result_value="'c'")
+
+self.expect_expr("a2", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b2", result_type="char", result_value="'a'")
+self.expect_expr("c2", result_type="char", result_value="'b'")
+self.expect_expr("d2", result_type="short", result_value="50")
+self.expect_expr("e2", result_type="int", result_value="60")
+self.expect_expr("f2", result_type="char", result_value="'c'")
+
+self.expect_expr("a3", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119501/new/

https://reviews.llvm.org/D119501

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.llvm.org/D119915/new/

https://reviews.llvm.org/D119915

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSError.cpp
  lldb/source/Plugins/Language/ObjC/NSException.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -3011,8 +3011,8 @@
   if (m_CFBoolean_values)
 return true;
 
-  static ConstString g___kCFBooleanFalse("__kCFBooleanFalse");
-  static ConstString g___kCFBooleanTrue("__kCFBooleanTrue");
+  static ConstString g_dunder_kCFBooleanFalse("__kCFBooleanFalse");
+  static ConstString g_dunder_kCFBooleanTrue("__kCFBooleanTrue");
   static ConstString g_kCFBooleanFalse("kCFBooleanFalse");
   static ConstString g_kCFBooleanTrue("kCFBooleanTrue");
 
@@ -3045,8 +3045,8 @@
 return addr;
   };
 
-  lldb::addr_t false_addr = get_symbol(g___kCFBooleanFalse, g_kCFBooleanFalse);
-  lldb::addr_t true_addr = get_symbol(g___kCFBooleanTrue, g_kCFBooleanTrue);
+  lldb::addr_t false_addr = get_symbol(g_dunder_kCFBooleanFalse, g_kCFBooleanFalse);
+  lldb::addr_t true_addr = get_symbol(g_dunder_kCFBooleanTrue, g_kCFBooleanTrue);
 
   return (m_CFBoolean_values = {false_addr, true_addr}).operator bool();
 }
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -32,7 +32,7 @@
 Symbol callable_symbol;
 Address callable_address;
 LineEntry callable_line_entry;
-lldb::addr_t member__f_pointer_value = 0u;
+lldb::addr_t member_f_pointer_value = 0u;
 LibCppStdFunctionCallableCase callable_case =
 LibCppStdFunctionCallableCase::Invalid;
   };
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -63,7 +63,7 @@
 CPPLanguageRuntime::LibCppStdFunctionCallableInfo
 line_entry_helper(Target , const SymbolContext , Symbol *symbol,
   llvm::StringRef first_template_param_sref,
-  bool has___invoke) {
+  bool has_invoke) {
 
   CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
 
@@ -78,7 +78,7 @@
 LineEntry line_entry;
 addr.CalculateSymbolContextLineEntry(line_entry);
 
-if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+if (contains_lambda_identifier(first_template_param_sref) || has_invoke) {
   // Case 1 and 2
   optional_info.callable_case = lldb_private::CPPLanguageRuntime::
   LibCppStdFunctionCallableCase::Lambda;
@@ -138,25 +138,25 @@
   //we will obtain the name from this pointer.
   // 5) a free function. A pointer to the function will stored after the vtable
   //we will obtain the name from this pointer.
-  ValueObjectSP member__f_(
+  ValueObjectSP member_f_(
   valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
 
-  if (member__f_) {
-ValueObjectSP sub_member__f_(
-   member__f_->GetChildMemberWithName(ConstString("__f_"), true));
+  if (member_f_) {
+ValueObjectSP sub_member_f_(
+   member_f_->GetChildMemberWithName(ConstString("__f_"), true));
 
-if (sub_member__f_)
-member__f_ = sub_member__f_;
+if (sub_member_f_)
+member_f_ = sub_member_f_;
   }
 
-  if (!member__f_)
+  if (!member_f_)
 return optional_info;
 
-  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+  lldb::addr_t member_f_pointer_value = member_f_->GetValueAsUnsigned(0);
 
-  optional_info.member__f_pointer_value = member__f_pointer_value;
+  optional_info.member_f_pointer_value = member_f_pointer_value;
 
-  if (!member__f_pointer_value)
+  if 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSError.cpp
  lldb/source/Plugins/Language/ObjC/NSException.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -3011,8 +3011,8 @@
   if (m_CFBoolean_values)
 return true;
 
-  static ConstString g___kCFBooleanFalse("__kCFBooleanFalse");
-  static ConstString g___kCFBooleanTrue("__kCFBooleanTrue");
+  static ConstString g_dunder_kCFBooleanFalse("__kCFBooleanFalse");
+  static ConstString g_dunder_kCFBooleanTrue("__kCFBooleanTrue");
   static ConstString g_kCFBooleanFalse("kCFBooleanFalse");
   static ConstString g_kCFBooleanTrue("kCFBooleanTrue");
 
@@ -3045,8 +3045,8 @@
 return addr;
   };
 
-  lldb::addr_t false_addr = get_symbol(g___kCFBooleanFalse, g_kCFBooleanFalse);
-  lldb::addr_t true_addr = get_symbol(g___kCFBooleanTrue, g_kCFBooleanTrue);
+  lldb::addr_t false_addr = get_symbol(g_dunder_kCFBooleanFalse, g_kCFBooleanFalse);
+  lldb::addr_t true_addr = get_symbol(g_dunder_kCFBooleanTrue, g_kCFBooleanTrue);
 
   return (m_CFBoolean_values = {false_addr, true_addr}).operator bool();
 }
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -32,7 +32,7 @@
 Symbol callable_symbol;
 Address callable_address;
 LineEntry callable_line_entry;
-lldb::addr_t member__f_pointer_value = 0u;
+lldb::addr_t member_f_pointer_value = 0u;
 LibCppStdFunctionCallableCase callable_case =
 LibCppStdFunctionCallableCase::Invalid;
   };
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -63,7 +63,7 @@
 CPPLanguageRuntime::LibCppStdFunctionCallableInfo
 line_entry_helper(Target , const SymbolContext , Symbol *symbol,
   llvm::StringRef first_template_param_sref,
-  bool has___invoke) {
+  bool has_invoke) {
 
   CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
 
@@ -78,7 +78,7 @@
 LineEntry line_entry;
 addr.CalculateSymbolContextLineEntry(line_entry);
 
-if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+if (contains_lambda_identifier(first_template_param_sref) || has_invoke) {
   // Case 1 and 2
   optional_info.callable_case = lldb_private::CPPLanguageRuntime::
   LibCppStdFunctionCallableCase::Lambda;
@@ -138,25 +138,25 @@
   //we will obtain the name from this pointer.
   // 5) a free function. A pointer to the function will stored after the vtable
   //we will obtain the name from this pointer.
-  ValueObjectSP member__f_(
+  ValueObjectSP member_f_(
   valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
 
-  if (member__f_) {
-ValueObjectSP sub_member__f_(
-   member__f_->GetChildMemberWithName(ConstString("__f_"), true));
+  if (member_f_) {
+ValueObjectSP sub_member_f_(
+   member_f_->GetChildMemberWithName(ConstString("__f_"), true));
 
-if (sub_member__f_)
-member__f_ = sub_member__f_;
+if (sub_member_f_)
+member_f_ = sub_member_f_;
   }
 
-  if (!member__f_)
+  if (!member_f_)
 return optional_info;
 
-  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+  lldb::addr_t member_f_pointer_value = member_f_->GetValueAsUnsigned(0);
 
-  optional_info.member__f_pointer_value = member__f_pointer_value;
+  optional_info.member_f_pointer_value = member_f_pointer_value;
 
-  if (!member__f_pointer_value)
+  if (!member_f_pointer_value)
 return optional_info;
 
   ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
@@ 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.


https://reviews.llvm.org/D119915

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSError.cpp
  lldb/source/Plugins/Language/ObjC/NSException.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -3011,8 +3011,8 @@
   if (m_CFBoolean_values)
 return true;
 
-  static ConstString g___kCFBooleanFalse("__kCFBooleanFalse");
-  static ConstString g___kCFBooleanTrue("__kCFBooleanTrue");
+  static ConstString g_double_underscore_kCFBooleanFalse("__kCFBooleanFalse");
+  static ConstString g_double_underscore_kCFBooleanTrue("__kCFBooleanTrue");
   static ConstString g_kCFBooleanFalse("kCFBooleanFalse");
   static ConstString g_kCFBooleanTrue("kCFBooleanTrue");
 
@@ -3045,8 +3045,8 @@
 return addr;
   };
 
-  lldb::addr_t false_addr = get_symbol(g___kCFBooleanFalse, g_kCFBooleanFalse);
-  lldb::addr_t true_addr = get_symbol(g___kCFBooleanTrue, g_kCFBooleanTrue);
+  lldb::addr_t false_addr = get_symbol(g_double_underscore_kCFBooleanFalse, g_kCFBooleanFalse);
+  lldb::addr_t true_addr = get_symbol(g_double_underscore_kCFBooleanTrue, g_kCFBooleanTrue);
 
   return (m_CFBoolean_values = {false_addr, true_addr}).operator bool();
 }
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -32,7 +32,7 @@
 Symbol callable_symbol;
 Address callable_address;
 LineEntry callable_line_entry;
-lldb::addr_t member__f_pointer_value = 0u;
+lldb::addr_t member_double_underscore_f_pointer_value = 0u;
 LibCppStdFunctionCallableCase callable_case =
 LibCppStdFunctionCallableCase::Invalid;
   };
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -63,7 +63,7 @@
 CPPLanguageRuntime::LibCppStdFunctionCallableInfo
 line_entry_helper(Target , const SymbolContext , Symbol *symbol,
   llvm::StringRef first_template_param_sref,
-  bool has___invoke) {
+  bool has_double_underscore_invoke) {
 
   CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
 
@@ -78,7 +78,7 @@
 LineEntry line_entry;
 addr.CalculateSymbolContextLineEntry(line_entry);
 
-if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+if (contains_lambda_identifier(first_template_param_sref) || has_double_underscore_invoke) {
   // Case 1 and 2
   optional_info.callable_case = lldb_private::CPPLanguageRuntime::
   LibCppStdFunctionCallableCase::Lambda;
@@ -138,25 +138,25 @@
   //we will obtain the name from this pointer.
   // 5) a free function. A pointer to the function will stored after the vtable
   //we will obtain the name from this pointer.
-  ValueObjectSP member__f_(
+  ValueObjectSP member_double_underscore_f_(
   valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
 
-  if (member__f_) {
-ValueObjectSP sub_member__f_(
-   member__f_->GetChildMemberWithName(ConstString("__f_"), true));
+  if (member_double_underscore_f_) {
+ValueObjectSP sub_member_double_underscore_f_(
+   member_double_underscore_f_->GetChildMemberWithName(ConstString("__f_"), true));
 
-if (sub_member__f_)
-member__f_ = sub_member__f_;
+if (sub_member_double_underscore_f_)
+member_double_underscore_f_ = sub_member_double_underscore_f_;
   }
 
-  if (!member__f_)
+  if (!member_double_underscore_f_)
 return optional_info;
 
-  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+  lldb::addr_t 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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, stack empty!");

aprantl wrote:
> do you need a 
> ```
>  if (DebugKind > codegenoptions::LimitedDebugInfo)
>   return
> ```
> 
> here?
That kind of check is only used in limited places, why would it apply here as 
let's say opposed to the `EmitDeclare` for `VarDecl` case?



Comment at: clang/test/CodeGenCXX/debug-info-structured-binding.cpp:3
+
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A* %[[F:[0-9]+]], 
metadata ![[F:[0-9]+]], metadata !DIExpression())
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A* %[[F:[0-9]+]], 
metadata ![[F:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, [[F:[0-9]+]]))

aprantl wrote:
> We should check what F is, too, right?
Actually I should have used a different match, that was a mistake.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119178/new/

https://reviews.llvm.org/D119178

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-structured-binding.cpp
  lldb/test/API/lang/cpp/structured-binding/Makefile
  lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
  lldb/test/API/lang/cpp/structured-binding/main.cpp

Index: lldb/test/API/lang/cpp/structured-binding/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/main.cpp
@@ -0,0 +1,69 @@
+// Structured binding in C++ can bind identifiers to subobjects of an object.
+//
+// There are three cases we need to test:
+// 1) arrays
+// 2) tuples like objects
+// 3) non-static data members
+//
+// They can also bind by copy, reference or rvalue reference.
+
+#include 
+
+struct A {
+  int x;
+  int y;
+};
+
+// We want to cover a mix of types and also different sizes to make sure we
+// hande the offsets correctly.
+struct MixedTypesAndSizesStruct {
+  A a;
+  char b1;
+  char b2;
+  short b3;
+  int b4;
+  char b5;
+};
+
+int main() {
+  MixedTypesAndSizesStruct b{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  auto [a1, b1, c1, d1, e1, f1] = b;
+  auto &[a2, b2, c2, d2, e2, f2] = b;
+  auto &&[a3, b3, c3, d3, e3, f3] =
+  MixedTypesAndSizesStruct{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  // Array with different sized types
+  char carr[]{'a', 'b', 'c'};
+  short sarr[]{11, 12, 13};
+  int iarr[]{22, 33, 44};
+
+  auto [carr_copy1, carr_copy2, carr_copy3] = carr;
+  auto [sarr_copy1, sarr_copy2, sarr_copy3] = sarr;
+  auto [iarr_copy1, iarr_copy2, iarr_copy3] = iarr;
+
+  auto &[carr_ref1, carr_ref2, carr_ref3] = carr;
+  auto &[sarr_ref1, sarr_ref2, sarr_ref3] = sarr;
+  auto &[iarr_ref1, iarr_ref2, iarr_ref3] = iarr;
+
+  auto &&[carr_rref1, carr_rref2, carr_rref3] = carr;
+  auto &&[sarr_rref1, sarr_rref2, sarr_rref3] = sarr;
+  auto &&[iarr_rref1, iarr_rref2, iarr_rref3] = iarr;
+
+  float x{4.0};
+  char y{'z'};
+  int z{10};
+
+  std::tuple tpl(x, y, z);
+  auto [tx1, ty1, tz1] = tpl;
+  auto &[tx2, ty2, tz2] = tpl;
+
+  return a1.x + b1 + c1 + d1 + e1 + f1 + a2.y + b2 + c2 + d2 + e2 + f2 + a3.x +
+ b3 + c3 + d3 + e3 + f3 + carr_copy1 + carr_copy2 + carr_copy3 +
+ sarr_copy1 + sarr_copy2 + sarr_copy3 + iarr_copy1 + iarr_copy2 +
+ iarr_copy3 + carr_ref1 + carr_ref2 + carr_ref3 + sarr_ref1 +
+ sarr_ref2 + sarr_ref3 + iarr_ref1 + iarr_ref2 + iarr_ref3 +
+ carr_rref1 + carr_rref2 + carr_rref3 + sarr_rref1 + sarr_rref2 +
+ sarr_rref3 + iarr_rref1 + iarr_rref2 + iarr_rref3 + tx1 + ty1 + tz1 +
+ tx2 + ty2 + tz2; // break here
+}
Index: lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
@@ -0,0 +1,84 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestStructuredBinding(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(compiler="clang", compiler_version=['<', '14.0'])
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("a1", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b1", result_type="char", result_value="'a'")
+self.expect_expr("c1", result_type="char", result_value="'b'")
+self.expect_expr("d1", result_type="short", result_value="50")
+self.expect_expr("e1", result_type="int", result_value="60")
+self.expect_expr("f1", result_type="char", result_value="'c'")
+
+self.expect_expr("a2", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b2", result_type="char", result_value="'a'")
+self.expect_expr("c2", result_type="char", result_value="'b'")
+self.expect_expr("d2", result_type="short", result_value="50")
+self.expect_expr("e2", result_type="int", result_value="60")
+self.expect_expr("f2", result_type="char", result_value="'c'")
+
+self.expect_expr("a3", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b3", result_type="char", result_value="'a'")
+self.expect_expr("c3", result_type="char", result_value="'b'")
+

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/scripted/ScriptedThread.cpp:195
+
+lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+bool cfa_is_valid = false;

`const`



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:196
+lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+bool cfa_is_valid = false;
+const bool behaves_like_zeroth_frame = false;

`const`



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:273
   GetRegisterContext()->InvalidateIfNeeded(/*force=*/false);
+  LoadArtificialStackFrames();
 }

mib wrote:
> jingham wrote:
> > LoadArtificialStackFrames returns a bool for success/failure.  It always 
> > bugs me when returns signifying errors get dropped on the floor like this.
> `Thread::RefreshStateAfterStop` doesn't return anything, and the failure case 
> is already handled in `LoadArtificialStackFrames` by logging an error message 
> in the appropriate channel.
> 
> Do you have any other suggestion on how I could improve this ?
If we are never going to check the return value, we should not have one, it is 
meaningless.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119388/new/

https://reviews.llvm.org/D119388

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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_deref?
Good catch, I used the `VarDecl` version of `EmitDeclare` as an example and I 
copied it from there.

I will also be pushing the `DW_OP_plus_uconst` and the value later on as well. 
So maybe 3 is a the right value.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4713
+   .toCharUnitsFromBits(value * typeSize)
+   .getQuantity());
+  }

aprantl wrote:
> Can C++ arrays ever have a non-zero stride? (perhaps due to element 
> alignment?)
Elements must be contiguous and start at `0` see 
[dcl.array/p6](http://eel.is/c++draft/dcl.array#6):

>An object of type “array of N U” consists of a contiguously allocated 
>non-empty set of N subobjects of type U, known as the elements of the array, 
>and numbered 0 to N-1.

The size of the type should include any padding. 



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4716
+}
+  }
+

aprantl wrote:
> Should there be an else { assert("unhandled binding expressions"); } here or 
> are there other expressions that just don't need special handling?
We should only have three cases:

- struct
- array
- tuple-like 

We exit early for the tuple-like case at the top b/c we handle that elsewhere. 

So maybe asserting we never end up with a not handled case may make sense.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119178/new/

https://reviews.llvm.org/D119178

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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` at least my testing seems to show that.

Otherwise this looks good.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113498/new/

https://reviews.llvm.org/D113498

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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

nitpick: Is there a better way to format this code or rewrite it so that is 
reads better?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119168/new/

https://reviews.llvm.org/D119168

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 represented by a `DecompositionDecl` AST node and the 
binding are represented by a `BindingDecl`. It looks the original implementation
only covered the tuple like case which be represented by a `DeclRefExpr` which 
contains a `VarDecl`.

If the binding is to a subobject of the struct the binding will contain a 
`MemberExpr` and in the case of arrays it will contain an `ArraySubscriptExpr`.
This PR adds support emitting debug-info for the `MemberExpr` and 
`ArraySubscriptExpr` cases as well as llvm and lldb tests for these cases as 
well as the tuple case.


https://reviews.llvm.org/D119178

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-structured-binding.cpp
  lldb/test/API/lang/cpp/structured-binding/Makefile
  lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
  lldb/test/API/lang/cpp/structured-binding/main.cpp

Index: lldb/test/API/lang/cpp/structured-binding/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/main.cpp
@@ -0,0 +1,69 @@
+// Structured binding in C++ can bind identifiers to subobjects of an object.
+//
+// There are three cases we need to test:
+// 1) arrays
+// 2) tuples like objects
+// 3) non-static data members
+//
+// They can also bind by copy, reference or rvalue reference.
+
+#include 
+
+struct A {
+  int x;
+  int y;
+};
+
+// We want to cover a mix of types and also different sizes to make sure we
+// hande the offsets correctly.
+struct MixedTypesAndSizesStruct {
+  A a;
+  char b1;
+  char b2;
+  short b3;
+  int b4;
+  char b5;
+};
+
+int main() {
+  MixedTypesAndSizesStruct b{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  auto [a1, b1, c1, d1, e1, f1] = b;
+  auto &[a2, b2, c2, d2, e2, f2] = b;
+  auto &&[a3, b3, c3, d3, e3, f3] =
+  MixedTypesAndSizesStruct{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  // Array with different sized types
+  char carr[]{'a', 'b', 'c'};
+  short sarr[]{11, 12, 13};
+  int iarr[]{22, 33, 44};
+
+  auto [carr_copy1, carr_copy2, carr_copy3] = carr;
+  auto [sarr_copy1, sarr_copy2, sarr_copy3] = sarr;
+  auto [iarr_copy1, iarr_copy2, iarr_copy3] = iarr;
+
+  auto &[carr_ref1, carr_ref2, carr_ref3] = carr;
+  auto &[sarr_ref1, sarr_ref2, sarr_ref3] = sarr;
+  auto &[iarr_ref1, iarr_ref2, iarr_ref3] = iarr;
+
+  auto &&[carr_rref1, carr_rref2, carr_rref3] = carr;
+  auto &&[sarr_rref1, sarr_rref2, sarr_rref3] = sarr;
+  auto &&[iarr_rref1, iarr_rref2, iarr_rref3] = iarr;
+
+  float x{4.0};
+  char y{'z'};
+  int z{10};
+
+  std::tuple tpl(x, y, z);
+  auto [tx1, ty1, tz1] = tpl;
+  auto &[tx2, ty2, tz2] = tpl;
+
+  return a1.x + b1 + c1 + d1 + e1 + f1 + a2.y + b2 + c2 + d2 + e2 + f2 + a3.x +
+ b3 + c3 + d3 + e3 + f3 + carr_copy1 + carr_copy2 + carr_copy3 +
+ sarr_copy1 + sarr_copy2 + sarr_copy3 + iarr_copy1 + iarr_copy2 +
+ iarr_copy3 + carr_ref1 + carr_ref2 + carr_ref3 + sarr_ref1 +
+ sarr_ref2 + sarr_ref3 + iarr_ref1 + iarr_ref2 + iarr_ref3 +
+ carr_rref1 + carr_rref2 + carr_rref3 + sarr_rref1 + sarr_rref2 +
+ sarr_rref3 + iarr_rref1 + iarr_rref2 + iarr_rref3 + tx1 + ty1 + tz1 +
+ tx2 + ty2 + tz2; // break here
+}
Index: lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
@@ -0,0 +1,84 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestStructuredBinding(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(compiler="clang", compiler_version=['<', '14.0'])
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("a1", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b1", result_type="char", result_value="'a'")
+self.expect_expr("c1", result_type="char", result_value="'b'")
+self.expect_expr("d1", result_type="short", result_value="50")
+self.expect_expr("e1", result_type="int", result_value="60")
+self.expect_expr("f1", result_type="char", result_value="'c'")
+
+self.expect_expr("a2", result_type="A",
+result_children=[ValueCheck(name="x", type="int"),
+ ValueCheck(name="y", type="int")])
+self.expect_expr("b2", result_type="char", 

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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`



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:123
+
+  uint64_t val = obj->GetIntegerValue(LLDB_INVALID_INDEX32);
+

`const`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118482/new/

https://reviews.llvm.org/D118482

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D116982/new/

https://reviews.llvm.org/D116982

Files:
  lldb/source/Target/TraceInstructionDumper.cpp


Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -148,9 +148,8 @@
   if (!insn.instruction)
 return;
   s.Printf("");
-  insn.instruction->Dump(, /*show_address=*/false, /*show_bytes=*/false,
- /*max_opcode_byte_size=*/false, _ctx,
- ,
+  insn.instruction->Dump(, /*max_opcode_byte_size=*/0, 
/*show_address=*/false,
+ /*show_bytes=*/false, _ctx, ,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
  /*max_address_text_size=*/0);


Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -148,9 +148,8 @@
   if (!insn.instruction)
 return;
   s.Printf("");
-  insn.instruction->Dump(, /*show_address=*/false, /*show_bytes=*/false,
- /*max_opcode_byte_size=*/false, _ctx,
- ,
+  insn.instruction->Dump(, /*max_opcode_byte_size=*/0, /*show_address=*/false,
+ /*show_bytes=*/false, _ctx, ,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
  /*max_address_text_size=*/0);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/8afcfbfb8fc1e53023ffac9d9bdc424248d6d2ff

which was changing booleans to use `true` and `false` and in the this case the 
comment and the type looked mismatched.


https://reviews.llvm.org/D116982

Files:
  lldb/source/Target/TraceInstructionDumper.cpp


Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -148,9 +148,8 @@
   if (!insn.instruction)
 return;
   s.Printf("");
-  insn.instruction->Dump(, /*show_address=*/false, /*show_bytes=*/false,
- /*max_opcode_byte_size=*/false, _ctx,
- ,
+  insn.instruction->Dump(, /*max_opcode_byte_size=*/0, 
/*show_address=*/false,
+ /*show_bytes=*/false, _ctx, ,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
  /*max_address_text_size=*/0);


Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -148,9 +148,8 @@
   if (!insn.instruction)
 return;
   s.Printf("");
-  insn.instruction->Dump(, /*show_address=*/false, /*show_bytes=*/false,
- /*max_opcode_byte_size=*/false, _ctx,
- ,
+  insn.instruction->Dump(, /*max_opcode_byte_size=*/0, /*show_address=*/false,
+ /*show_bytes=*/false, _ctx, ,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
  /*max_address_text_size=*/0);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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.org/D116768/new/

https://reviews.llvm.org/D116768

Files:
  lldb/source/Host/common/Socket.cpp


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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/D116768

Files:
  lldb/source/Host/common/Socket.cpp


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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`.


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@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D112709/new/

https://reviews.llvm.org/D112709

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115062/new/

https://reviews.llvm.org/D115062

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115662/new/

https://reviews.llvm.org/D115662

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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://reviews.llvm.org/D115308

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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 list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits