date:20230514

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-05-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

This patch adds test coverage for existing functionality. It was ready to land 
6 weeks ago. There was plenty of opportunity to raise concerns and discuss 
details over the last 2 months of review. I assume the topic is just too niche 
to attract a lot of attention.

I am planning to land this patch tomorrow and D146154 
 soon after, since I have some time on 
Tuesday and Wednesday to cope with build bot reports. I am happy to answer any 
questions in the remaining time. Otherwise, don't hesitate to follow up 
post-commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

2023-05-14 Thread Eli Friedman via Phabricator via lldb-commits

efriedma added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4579
+  if (CE->hasAPValueResult())
+mangleValueInTemplateArg(ParamType, CE->getResultAsAPValue(), false,
+ /*NeedExactType=*/true);

I'm not sure what the point of the `if (CE->hasAPValueResult())` is; are you 
just trying to avoid copying the APValue?  (If this is going to be a repeating 
pattern, maybe we can add some sort of utility class to represent the pattern.)



Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+// argument.
+// As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+auto *SNTTPE = cast(E);

bolshakov-a wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > We should get this nailed down. It was proposed in Nov 2020 and the 
> > > > issue is still open. CC @rjmccall 
> > > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any idea 
> > > what the actual mangling should be?
> > This is still an open, and we need @rjmccall @eli.friedman or @asl to help 
> > out here.
> Ping @efriedma, @rjmccall, @asl.
I'm not really familiar with the mangling implications for this particular 
construct, nor am I actively involved with the Itanium ABI specification, so 
I'm not sure how I can help you directly.

That said, as a general opinion, I don't think it's worth waiting for updates 
to the Itanuim ABI  document to be merged; such updates are happening slowly at 
the moment, and having a consistent mangling is clearly an improvement even if 
it's not specified.  My suggested plan of action:

- Make sure you're satisfied the proposed mangling doesn't have any holes 
you're concerned about (i.e. it produces a unique mangling for all the relevant 
cases).  If you're not sure, I can try to spend some time understanding this, 
but it doesn't sound like you have any concerns about this.
- Put a note on the issue in the Itanium ABI repo that you're planning to go 
ahead with using this mangling in clang.  Also send an email directly to 
@rjmccall and @rsmith in case they miss the notifications.
- Go ahead with this.


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.

2023-05-14 Thread Andrey Ali Khan Bolshakov via Phabricator via lldb-commits

bolshakov-a added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4579
+  if (CE->hasAPValueResult())
+mangleValueInTemplateArg(ParamType, CE->getResultAsAPValue(), false,
+ /*NeedExactType=*/true);

efriedma wrote:
> I'm not sure what the point of the `if (CE->hasAPValueResult())` is; are you 
> just trying to avoid copying the APValue?  (If this is going to be a 
> repeating pattern, maybe we can add some sort of utility class to represent 
> the pattern.)
I don't know too, it is Richard's code. Looks really like an optimization.


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.

2023-05-14 Thread Andrey Ali Khan Bolshakov via Phabricator via lldb-commits

bolshakov-a added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+// argument.
+// As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+auto *SNTTPE = cast(E);

efriedma wrote:
> bolshakov-a wrote:
> > erichkeane wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > We should get this nailed down. It was proposed in Nov 2020 and the 
> > > > > issue is still open. CC @rjmccall 
> > > > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any 
> > > > idea what the actual mangling should be?
> > > This is still an open, and we need @rjmccall @eli.friedman or @asl to 
> > > help out here.
> > Ping @efriedma, @rjmccall, @asl.
> I'm not really familiar with the mangling implications for this particular 
> construct, nor am I actively involved with the Itanium ABI specification, so 
> I'm not sure how I can help you directly.
> 
> That said, as a general opinion, I don't think it's worth waiting for updates 
> to the Itanuim ABI  document to be merged; such updates are happening slowly 
> at the moment, and having a consistent mangling is clearly an improvement 
> even if it's not specified.  My suggested plan of action:
> 
> - Make sure you're satisfied the proposed mangling doesn't have any holes 
> you're concerned about (i.e. it produces a unique mangling for all the 
> relevant cases).  If you're not sure, I can try to spend some time 
> understanding this, but it doesn't sound like you have any concerns about 
> this.
> - Put a note on the issue in the Itanium ABI repo that you're planning to go 
> ahead with using this mangling in clang.  Also send an email directly to 
> @rjmccall and @rsmith in case they miss the notifications.
> - Go ahead with this.
> Put a note on the issue in the Itanium ABI repo that you're planning to go 
> ahead with using this mangling in clang. Also send an email directly to 
> @rjmccall and @rsmith in case they miss the notifications.

I'm sorry for noting one more time that Richard already pushed these changes in 
clang upstream, but they had been just reverted.

Maybe, I should make a PR into Itanium API repository, but I probably need some 
time to dig into the theory and all the discussions. But yes, even NTTP 
argument mangling rules are not still merged: 
https://github.com/itanium-cxx-abi/cxx-abi/pull/140


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] [lldb] 3ebb336 - [lldb] Complete OptionValue cleanup (NFC)

2023-05-14 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-05-14T20:18:47-07:00
New Revision: 3ebb33632a1509450fdbc1fb6a21107a0a513072

URL: 
https://github.com/llvm/llvm-project/commit/3ebb33632a1509450fdbc1fb6a21107a0a513072
DIFF: 
https://github.com/llvm/llvm-project/commit/3ebb33632a1509450fdbc1fb6a21107a0a513072.diff

LOG: [lldb] Complete OptionValue cleanup (NFC)

Make the `Get.*Value` and `Set.*Value` function private and migrate the
last remaining call sites to the new overloaded/templated functions.

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Interpreter/OptionValue.h
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectRegister.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Interpreter/OptionValue.cpp
lldb/source/Interpreter/OptionValueArgs.cpp
lldb/source/Interpreter/OptionValueArray.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Target/Process.cpp
lldb/unittests/Interpreter/TestOptionValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index f05d8d4e6b231..54f7d5c0edb4a 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -285,7 +285,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   uint64_t GetTerminalWidth() const;
 
-  bool SetTerminalWidth(uint32_t term_width);
+  bool SetTerminalWidth(uint64_t term_width);
 
   llvm::StringRef GetPrompt() const;
 
@@ -351,7 +351,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   uint64_t GetTabSize() const;
 
-  bool SetTabSize(uint32_t tab_size);
+  bool SetTabSize(uint64_t tab_size);
 
   lldb::DWIMPrintVerbosity GetDWIMPrintVerbosity() const;
 

diff  --git a/lldb/include/lldb/Interpreter/OptionValue.h 
b/lldb/include/lldb/Interpreter/OptionValue.h
index 730ec65dd054b..9e65c802b3706 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -17,6 +17,8 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/FileSpecList.h"
 #include "lldb/Utility/Status.h"
+#include "lldb/Utility/StringList.h"
+#include "lldb/Utility/UUID.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-interfaces.h"
@@ -260,58 +262,8 @@ class OptionValue {
 
   const OptionValueFormatEntity *GetAsFormatEntity() const;
 
-  std::optional GetBooleanValue() const;
-
-  bool SetBooleanValue(bool new_value);
-
-  std::optional GetCharValue() const;
-
-  char SetCharValue(char new_value);
-
-  std::optional GetEnumerationValue() const;
-
-  bool SetEnumerationValue(int64_t value);
-
-  std::optional GetFileSpecValue() const;
-
-  bool SetFileSpecValue(FileSpec file_spec);
-
   bool AppendFileSpecValue(FileSpec file_spec);
 
-  std::optional GetFileSpecListValue() const;
-
-  std::optional GetFormatValue() const;
-
-  bool SetFormatValue(lldb::Format new_value);
-
-  std::optional GetLanguageValue() const;
-
-  bool SetLanguageValue(lldb::LanguageType new_language);
-
-  const FormatEntity::Entry *GetFormatEntity() const;
-
-  const RegularExpression *GetRegexValue() const;
-
-  std::optional GetSInt64Value() const;
-
-  bool SetSInt64Value(int64_t new_value);
-
-  std::optional GetStringValue() const;
-
-  bool SetStringValue(llvm::StringRef new_value);
-
-  std::optional GetUInt64Value() const;
-
-  bool SetUInt64Value(uint64_t new_value);
-
-  UUID GetUUIDValue() const;
-
-  bool SetUUIDValue(const UUID &uuid);
-
-  std::optional GetArchSpecValue() const;
-
-  bool SetArchSpecValue(ArchSpec arch_spec);
-
   bool OptionWasSet() const { return m_value_was_set; }
 
   void SetOptionWasSet() { m_value_was_set = true; }
@@ -373,10 +325,20 @@ class OptionValue {
 
   bool SetValueAs(bool v) { return SetBooleanValue(v); }
 
+  bool SetValueAs(char v) { return SetCharValue(v); }
+
+  bool SetValueAs(uint64_t v) { return SetUInt64Value(v); }
+
+  bool SetValueAs(int64_t v) { return SetSInt64Value(v); }
+
+  bool SetValueAs(UUID v) { return SetUUIDValue(v); }
+
   bool SetValueAs(llvm::StringRef v) { return SetStringValue(v); }
 
   bool SetValueAs(lldb::LanguageType v) { return SetLanguageValue(v); }
 
+  bool SetValueAs(lldb::Format v) { return SetFormatValue(v); }
+
   bool SetValueAs(FileSpec v) { return SetFileSpecValue(v); }
 
   bool SetValueAs(ArchSpec v) { return SetArchSpecValue(v); }
@@ -401,6 +363,44 @@ class OptionValue {
 // set from the command line or as a setting,
 // versus if we just have the default value 
that

[Lldb-commits] [lldb] bf76a6e - [lldb] Cleanup OptionValue header and implenentation (NFC)

2023-05-14 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-05-14T20:30:29-07:00
New Revision: bf76a6e44723b73940ca81c3b17077225b3c4118

URL: 
https://github.com/llvm/llvm-project/commit/bf76a6e44723b73940ca81c3b17077225b3c4118
DIFF: 
https://github.com/llvm/llvm-project/commit/bf76a6e44723b73940ca81c3b17077225b3c4118.diff

LOG: [lldb] Cleanup OptionValue header and implenentation (NFC)

Group related functions together and remove inconsistencies between them
in the implementation.

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValue.h
lldb/source/Interpreter/OptionValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValue.h 
b/lldb/include/lldb/Interpreter/OptionValue.h
index 9e65c802b370..cd587ed463b1 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,7 +114,8 @@ class OptionValue {
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue", 
name.str().c_str());
+error.SetErrorStringWithFormat("'%s' is not a value subvalue",
+   name.str().c_str());
 return lldb::OptionValueSP();
   }
 
@@ -187,79 +188,60 @@ class OptionValue {
 Status &error);
 
   OptionValueArch *GetAsArch();
-
   const OptionValueArch *GetAsArch() const;
 
   OptionValueArray *GetAsArray();
-
   const OptionValueArray *GetAsArray() const;
 
   OptionValueArgs *GetAsArgs();
-
   const OptionValueArgs *GetAsArgs() const;
 
   OptionValueBoolean *GetAsBoolean();
-
-  OptionValueChar *GetAsChar();
-
   const OptionValueBoolean *GetAsBoolean() const;
 
+  OptionValueChar *GetAsChar();
   const OptionValueChar *GetAsChar() const;
 
   OptionValueDictionary *GetAsDictionary();
-
   const OptionValueDictionary *GetAsDictionary() const;
 
   OptionValueEnumeration *GetAsEnumeration();
-
   const OptionValueEnumeration *GetAsEnumeration() const;
 
   OptionValueFileSpec *GetAsFileSpec();
-
   const OptionValueFileSpec *GetAsFileSpec() const;
 
   OptionValueFileSpecList *GetAsFileSpecList();
-
   const OptionValueFileSpecList *GetAsFileSpecList() const;
 
   OptionValueFormat *GetAsFormat();
-
   const OptionValueFormat *GetAsFormat() const;
 
   OptionValueLanguage *GetAsLanguage();
-
   const OptionValueLanguage *GetAsLanguage() const;
 
   OptionValuePathMappings *GetAsPathMappings();
-
   const OptionValuePathMappings *GetAsPathMappings() const;
 
   OptionValueProperties *GetAsProperties();
-
   const OptionValueProperties *GetAsProperties() const;
 
   OptionValueRegex *GetAsRegex();
-
   const OptionValueRegex *GetAsRegex() const;
 
   OptionValueSInt64 *GetAsSInt64();
-
   const OptionValueSInt64 *GetAsSInt64() const;
 
   OptionValueString *GetAsString();
-
   const OptionValueString *GetAsString() const;
 
   OptionValueUInt64 *GetAsUInt64();
-
   const OptionValueUInt64 *GetAsUInt64() const;
 
   OptionValueUUID *GetAsUUID();
-
   const OptionValueUUID *GetAsUUID() const;
 
   OptionValueFormatEntity *GetAsFormatEntity();
-
   const OptionValueFormatEntity *GetAsFormatEntity() const;
 
   bool AppendFileSpecValue(FileSpec file_spec);

diff  --git a/lldb/source/Interpreter/OptionValue.cpp 
b/lldb/source/Interpreter/OptionValue.cpp
index 7fa34c55d0f4..2f110d53d958 100644
--- a/lldb/source/Interpreter/OptionValue.cpp
+++ b/lldb/source/Interpreter/OptionValue.cpp
@@ -258,8 +258,7 @@ std::optional OptionValue::GetBooleanValue() const {
 }
 
 bool OptionValue::SetBooleanValue(bool new_value) {
-  OptionValueBoolean *option_value = GetAsBoolean();
-  if (option_value) {
+  if (OptionValueBoolean *option_value = GetAsBoolean()) {
 option_value->SetCurrentValue(new_value);
 return true;
   }
@@ -273,8 +272,7 @@ std::optional OptionValue::GetCharValue() const {
 }
 
 bool OptionValue::SetCharValue(char new_value) {
-  OptionValueChar *option_value = GetAsChar();
-  if (option_value) {
+  if (OptionValueChar *option_value = GetAsChar()) {
 option_value->SetCurrentValue(new_value);
 return true;
   }
@@ -288,8 +286,7 @@ std::optional OptionValue::GetEnumerationValue() 
const {
 }
 
 bool OptionValue::SetEnumerationValue(int64_t value) {
-  OptionValueEnumeration *option_value = GetAsEnumeration();
-  if (option_value) {
+  if (OptionValueEnumeration *option_value = GetAsEnumeration()) {
 option_value->SetCurrentValue(value);
 return true;
   }
@@ -297,15 +294,13 @@ bool OptionValue::SetEnumerationValue(int64_t value) {
 }
 
 std::optional OptionValue::GetFileSpecValue() const {
-  const OptionValueFileSpec *option_value = GetAsFileSpec();
-  if (option_value)
+  if (const OptionValueFileSpec *option_value = GetAsFileSpec())
 return option_value->GetCurrentValue()

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

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

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

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

[Lldb-commits] [lldb] 3ebb336 - [lldb] Complete OptionValue cleanup (NFC)

[Lldb-commits] [lldb] bf76a6e - [lldb] Cleanup OptionValue header and implenentation (NFC)

6 matches

Site Navigation

Mail list logo

Footer information