[Lldb-commits] [PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Hello! Thanks for the patch, generally it looks good to me, the only thing I 
worry about is scoped enums (please, see the comment below).




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7324-7331
+  // If the variable is an enum type, take the underlying integer type as
+  // the type of the integer literal.
+  if (const EnumType *enum_type = llvm::dyn_cast(qt.getTypePtr())) {
+const EnumDecl *enum_decl = enum_type->getDecl();
+qt = enum_decl->getIntegerType();
+  }
+  var->setInit(IntegerLiteral::Create(ast, init_value, qt.getUnqualifiedType(),

I'm not sure, can we initialize a member this way if it has a scoped enum type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160



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

[Lldb-commits] [PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7324-7331
+  // If the variable is an enum type, take the underlying integer type as
+  // the type of the integer literal.
+  if (const EnumType *enum_type = llvm::dyn_cast(qt.getTypePtr())) {
+const EnumDecl *enum_decl = enum_type->getDecl();
+qt = enum_decl->getIntegerType();
+  }
+  var->setInit(IntegerLiteral::Create(ast, init_value, qt.getUnqualifiedType(),

teemperor wrote:
> aleksandr.urakov wrote:
> > I'm not sure, can we initialize a member this way if it has a scoped enum 
> > type?
> (That code is from D81471 so I think I should answer that)
> 
> Well, it works and but it relies on CodeGen/Sema not double-checking that we 
> get the enum type (and not the underlying int type). I can inject a 
> CXXStaticCastExpr too and will update D81471. Thanks!
Ok, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160



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

[Lldb-commits] [PATCH] D82155: [WIP][lldb/interpreter] Add ability to save lldb session to a file

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This seems like it could be useful in some circumstances, though for the use 
cases I am imagining (bug reporting) it would be easier to just copy-paste the 
terminal contents.

As for the implementation, if the intention is for this to eventually capture 
all debugger output, then I am not sure if this is the right design. I think it 
would be fine for python/lua interpreters as those are invoked from the lldb 
command interpreter, but I have a feeling that routing the output printed via 
`Debugger::PrintAsync` back to the command interpreter would look pretty weird. 
It may make sense for the core logic of this to live in the Debugger or the 
IOHandler(Stack) classes -- though I am not exactly sure about that either as 
the Debugger and CommandIntepreter classes are fairly tightly coupled. However, 
I think that would be consistent with the long term goal of reimplementing the 
command interpreter on top of the SB API (in which case the `Debugger` object 
should not know anything about the command interpreter (but it would still need 
to to "something" with the PrintAsync output).

The test plan sounds fairly straight forward -- run lldb, execute a bunch of 
commands, and finish it off with "session save". Then, verify that the file has 
the "right" contents (e.g. with FileCheck). Besides multiline commands, 
commands which do recursive processing are also interesting to exercise -- e.g. 
"command source" or breakpoint callbacks. You also should decide what do you 
want to happen with commands that are executed through the SB interface 
(SBCommandInterpreter::HandleCommand) -- those will not normally go to the 
debugger output, but I believe they will still be captured in the current 
design...




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2815
 
+  if (line.find("session save") == line.npos) {
+*m_session_transcripts << io_handler.GetPrompt() << ' ' << line.c_str()

JDevlieghere wrote:
> this won't work for things like unambiguous abbreviations like `sess save`. 
> The command should do the saving.
I don't think it's unreasonable to write to the "transcript" here, but the 
string matching is obviously suboptimal. However, it's not clear to me why is 
it even needed -- having the "save" command in the transcript does not 
necessarily seem like a bad thing, and I believe the way it is implemented 
means that the command will not show up in the session file that is currently 
being saved (only in the subsequent ones).



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2820
+llvm::StringRef result_output = result.GetOutputData();
+if (!result_output.empty())
+  *m_session_transcripts << result_output;

These `!empty()` checks are pointless.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2827
+
+m_session_transcripts->Flush();
+  }

As is this flush call.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2929
+  Status error;
+  user_id_t fd_dst = FileCache::GetInstance().OpenFile(
+  FileSpec(output_file), flags, lldb::eFilePermissionsFileDefault, error);

mib wrote:
> JDevlieghere wrote:
> > Why are you going through the `FileCache`? 
> I was thinking if the user saves several times during his session to the same 
> file, that might be useful. Looking at the implementation, it uses the 
> FileSystem instance, so I'm not sure why that would a problem ...
> 
> May be you could elaborate why I shouldn't use it ?
`FileCache` is a very specialist class, so I believe the default should be to 
_not_ use it. However, if you are looking for reasons, I can name a few:
- the way you are using it means you get no caching benefits whatsoever -- each 
`OpenFile` call creates a new file descriptor
- it makes it very easy to leak that descriptor (as you did here)



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2945
+  auto string_stream =
+  std::static_pointer_cast(m_session_transcripts);
+  size_t stream_size = string_stream->GetSize();

Why not make `m_session_transcripts` a `shared_ptr` (or even a 
plain `StreamString`) in the first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82155



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

[Lldb-commits] [PATCH] D82272: [lldb/Lua] Recognize "quit" as a way to exit the interactive script interpreter.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

It seems reasonable. Btw, have you figured out how the embedded python does it? 
-- I haven't been able to find the code for handling that.

The python interpreter also intercepts `os.exit()` to avoid terminating the 
program (it just terminates the python session). I suppose we should do the 
same with lua's `os.exit`...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D82272



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

[Lldb-commits] [PATCH] D82273: [lldb/Lua] Use the debugger's output and error file for Lua's I/O library.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I have some questions (but no definitive answers) inline...




Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:64
+
+  if (out) {
+lua_pushstring(m_lua_state, "stdout");

What should be the behavior if this is null? Can it even be null (should we be 
asserting that it isn't)?
It's not clear to me that we should be reusing the previously set stdout if 
these arguments are null... Maybe we should be saving the original stdout value 
and restoring it when we are done? That's what the python interpreter seems to 
be doing..



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:65-66
+  if (out) {
+lua_pushstring(m_lua_state, "stdout");
+lua_gettable(m_lua_state, -2);
+luaL_Stream *s =

`lua_getfield(m_lua_state, -1, "stdout")`



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:68
+luaL_Stream *s =
+static_cast(lua_touserdata(m_lua_state, -1));
+if (!s) {

I guess it would be better to use `luaL_testudata(m_lua_state, -1, 
LUA_FILEHANDLE)` as that also checks that the user data is of the expected type 
(and e.g. it was not replaced by the user). 

Or it might actually be better to just overwrite the value of `io.stdout/err` 
with a new value/userdata object, instead of fiddling with the existing one.


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

https://reviews.llvm.org/D82273



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

[Lldb-commits] [PATCH] D82297: [lldb] Replace StringConvert with llvm::to_integer when parsing integer values in CommandObjects

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: JDevlieghere, abidh.

This replaces the current use of LLDB's own `StringConvert` with LLVM's 
`to_integer` which
has a less error-prone API and doesn't use special 'error values' to designate 
parsing problems.

Where needed I also added missing error handling code that prints a parsing 
error instead of continuing
with the error value returned from `StringConvert` (which either gave a cryptic 
error message or just took
the error value performed an incorrect action with it. For example, `frame 
recognizer delete -1`
just deleted the frame recognizer at index 0).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D82297

Files:
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
  lldb/test/API/commands/platform/file/close/TestPlatformFileClose.py
  lldb/test/API/commands/platform/file/read/TestPlatformFileRead.py
  lldb/test/API/commands/process/signal/Makefile
  lldb/test/API/commands/process/signal/TestProcessSignal.py
  lldb/test/API/commands/process/signal/main.cpp
  lldb/test/API/commands/target/modules/search-paths/insert/Makefile
  
lldb/test/API/commands/target/modules/search-paths/insert/TestTargetModulesSearchpathsInsert.py
  lldb/test/API/commands/target/modules/search-paths/insert/main.cpp
  lldb/test/API/commands/target/select/TestTargetSelect.py
  lldb/test/API/commands/target/stop-hook/delete/TestTargetStopHookDelete.py
  lldb/test/API/commands/target/stop-hook/disable/TestTargetStopHookDisable.py
  lldb/test/API/commands/target/stop-hook/enable/TestTargetStopHookEnable.py
  lldb/test/API/commands/thread/select/Makefile
  lldb/test/API/commands/thread/select/TestThreadSelect.py
  lldb/test/API/commands/thread/select/main.cpp

Index: lldb/test/API/commands/thread/select/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/thread/select/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/commands/thread/select/TestThreadSelect.py
===
--- /dev/null
+++ lldb/test/API/commands/thread/select/TestThreadSelect.py
@@ -0,0 +1,18 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_invalid_arg(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here', lldb.SBFileSpec("main.cpp"))
+
+self.expect("thread select -1", error=True, startstr="error: Invalid thread index '-1'")
+self.expect("thread select 0x1", error=True, startstr="error: Invalid thread index '0x1'")
+# Parses but not a valid thread id.
+self.expect("thread select 0x", error=True, startstr="error: invalid thread #0x.")
Index: lldb/test/API/commands/thread/select/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/thread/select/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/commands/target/stop-hook/enable/TestTargetStopHookEnable.py
===
--- /dev/null
+++ lldb/test/API/commands/target/stop-hook/enable/TestTargetStopHookEnable.py
@@ -0,0 +1,15 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test_invalid_arg(self):
+self.expect("target stop-hook enable -1", error=True,
+startstr="error: invalid stop hook id: \"-1\".")
+self.expect("target stop-hook enable abcdfx", error=True,
+startstr="error: invalid stop hook id: \"abcdfx\".")
Index: lldb/test/API/commands/target/stop-hook/disable/TestTargetStopHookDisable.py
===
--- /dev/null
+++ lldb/test/API/commands/target/stop-hook/disable/TestTargetStopHookDisable.py
@@ -0,0 +1,15 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test_invalid_arg(self):
+self.expect("target stop-hook disable -1", error=True,
+startstr="error: invalid stop hook id: \"-1\".")
+self.expect("target stop-hook disable abcdfx", error=True

[Lldb-commits] [lldb] 7960837 - [lldb][NFC] Add more test for builtin formats

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-06-22T15:13:41+02:00
New Revision: 79608371f1fd7f5aa6f8c05d007845e2e1cb7841

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

LOG: [lldb][NFC] Add more test for builtin formats

Reland 90c1af106a20785ffd01c0d6a41db8bc0160fd11 . This changes the char format
tests which were printing the pointer value of the C-string instead of its
contents, so this test failed on other machines. Now they just print the
bytes in a uint128_t.

Original commit description:

The previous tests apparently missed a few code branches in DumpDataExtractor
code. Also renames the 'test_instruction' which had the same name as another
test (and Python therefore ignored the test entirely).

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
 
b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
index 9fefae6bbf5c..643ce6f29da5 100644
--- 
a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
+++ 
b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
@@ -42,6 +42,9 @@ def test(self):
 self.assertIn("= 0\n", self.getFormatted("float", "0"))
 self.assertIn("= 2\n", self.getFormatted("float", "0x4000"))
 self.assertIn("= NaN\n", self.getFormatted("float", "-1"))
+# Checks the float16 code.
+self.assertIn("= 2\n", self.getFormatted("float", 
"(__UINT16_TYPE__)0x4000"))
+self.assertIn("= error: unsupported byte size (1) for float format\n", 
self.getFormatted("float", "'a'"))
 
 # enumeration
 self.assertIn("= 0\n", self.getFormatted("enumeration", "0"))
@@ -59,6 +62,13 @@ def test(self):
 
 # octal
 self.assertIn("= 04553207\n", self.getFormatted("octal", "1234567"))
+self.assertIn("= 0221505317046536757\n", self.getFormatted("octal", 
"(__uint128_t)0x123456789ABDEFull"))
+
+# complex float
+self.assertIn("= error: unsupported byte size (1) for complex float 
format\n", self.getFormatted("complex float", "'a'"))
+
+# complex integer
+self.assertIn("= error: unsupported byte size (1) for complex integer 
format\n", self.getFormatted("complex integer", "'a'"))
 
 # hex
 self.assertIn("= 0x00abc123\n", self.getFormatted("hex", "0xABC123"))
@@ -79,28 +89,33 @@ def test(self):
 self.assertIn(" = 0b1001000110100010101100000\n", 
self.getFormatted("binary", "(__uint128_t)0x12345678ll"))
 
 # Different character arrays.
+# FIXME: Passing a 'const char *' will ignore any given format,
 self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("character array", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("character", "cstring"))
 self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("c-string", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("printable character", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("OSType", "cstring"))
-self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
self.getFormatted("unicode8", "cstring"))
-
-self.assertIn('= \\0\\0\\0\\0\\0\\0\\0\\0\n', 
self.getFormatted("character array", "(__UINT64_TYPE__)0"))
-self.assertIn('= \\0\\0\\0\\0\\0\\0\\0\\0\n', 
self.getFormatted("character", "(__UINT64_TYPE__)0"))
 self.assertIn('=\n', self.getFormatted("c-string", 
"(__UINT64_TYPE__)0"))
-self.assertIn('= \n', self.getFormatted("printable character", 
"(__UINT64_TYPE__)0"))
-self.assertIn('= \'\\0\\0\\0\\0\\0\\0\\0\\0\'\n', 
self.getFormatted("OSType", "(__UINT64_TYPE__)0"))
-# FIXME: This seems wrong.
-self.assertIn('= 0x\n', self.getFormatted("unicode8", 
"(__UINT64_TYPE__)0"))
-
-self.assertIn('= xV4\\x12\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\n', 
self.getFormatted("character array", "(__uint128_t)0x12345678ll"))
-self.assertIn('= xV4\\x12\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\n', 
self.getFormatted("character", "(__uint128_t)0x12345678ll"))
-self.assertIn('=\n', self.getFormatted("c-string", 
"(__uint128_t)0x12345678ll"))
-self.assertIn('= xV4.\n', self.getFormatted("printable 
character", "(__uint128_t)0x12345678ll"))
-self.assertIn('= 0x12345678\n', 
self.getFormatted("OSType", "(__uint128_t)0x12345678ll"))
-# FIXME: This seems wrong.
-   

[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Yes, this looks better, though I am mildly worried about the use of timeouts.

There probably isn't anything better we can do for the "launch" case, but we 
could definitely come up with something better for the "attach" case. I think 
we can leave the attach code as-is for now, as the symmetry with the launch 
code is nice, but this will need to be revisited if the test turns out to be 
flaky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81200



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

[Lldb-commits] [PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

The only remaining issue with the LLDB expression part is that now that 
`SetFloatingInitializerForVariable` is based on the D81471 
 version it no longer makes the member 
variable a `constexpr`. Int/Enum members with initialisers can just be const, 
but for any floating point types this ends ups creating an AST that we usually 
can't get from the Clang parser (as floats can't be initialised this way). 
Clang will still codegen that for us, but I think making the floating point 
VarDecls `constexpr` could save us from bad surprises in the future (e.g., 
someone adds an assert to Clang/Sema that const static data members are only 
initialised when they have integer/enum type). I think marking the variable as 
constexpr in the two places where you call `SetFloatingInitializerForVariable` 
seems like the right place for this (`SetFloatingInitializerForVariable` 
probably shouldn't set that flag on its own, that seems like unexpected 
behaviour from this function).

Otherwise I think `auto` is used quite frequently and doesn't really fit to 
LLVM's "auto only if it makes code more readable" policy. I think all the 
integer types for the type widths should just be their own type (`uint32_t` 
makes the code easier to understand than auto). And I don't think LLDB usually 
does `auto` in LLDB for types/decls as LLDB has its own type/decl abstraction 
(`CompilerType` and `CompilerDecl`), so that's also confusing.

Beside that this patch looks good to me. Maybe someone that knows more about 
PDB should also take a look about the PDB-specific parts of the patch.




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7324-7331
+  // If the variable is an enum type, take the underlying integer type as
+  // the type of the integer literal.
+  if (const EnumType *enum_type = llvm::dyn_cast(qt.getTypePtr())) {
+const EnumDecl *enum_decl = enum_type->getDecl();
+qt = enum_decl->getIntegerType();
+  }
+  var->setInit(IntegerLiteral::Create(ast, init_value, qt.getUnqualifiedType(),

aleksandr.urakov wrote:
> I'm not sure, can we initialize a member this way if it has a scoped enum 
> type?
(That code is from D81471 so I think I should answer that)

Well, it works and but it relies on CodeGen/Sema not double-checking that we 
get the enum type (and not the underlying int type). I can inject a 
CXXStaticCastExpr too and will update D81471. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

This is already looking quite reasonable, I think we're getting closer to 
something we can merge.

In D81001#2096702 , @gedatsu217 wrote:

> Implementation all ascii characters for TypedCharacter.
>  Making m_use_autosuggestion in Editline.cpp. (I did not know a way to pass 
> the bool value of IOHandlerEditline to Editline constructor, so I made a 
> function, Editline::UseAutosuggestion, in Editline class. If the 
> autosuggestion is valid, this function is called from IOHandlerEditline 
> constructor. Is this implementation good?)


You could just have just passed it to the constructor call a few lines above 
your change, but this seems fine (and maybe even better as the constructor 
already has a lot of arguments).

> 
> 
>> You can test it by just doing settings set show-autosuggestion true (which 
>> should activate this) and settings set show-autosuggestion true (which 
>> should deactivate this). It's fine if LLDB requires a restart to do activate 
>> this setting (that's to my knowledge a limitation of the current way the 
>> IOHandlers work).
> 
> "restart" means that excuting "quit" in LLDB and executing "bin/lldb" again 
> in Terminal?  I made m_use_autosuggestion in Editline.cpp and it goes well 
> when show-autosuggestion is DefaultTrue. However, when show-autosuggestion is 
> DefaultFalse and executing "settings set show-autosuggestion true", 
> autosuggestion does not work well. Do you know what causes it?

The Editline instance isn't recreated when you change the settings, so you need 
to restart LLDB for this to work. However, settings aren't automatically saved, 
so I think the right way to do this by putting the `settings set 
show-autosuggestion true` into the `~/.lldbinit` file (which will be 
automatically executed before LLDB's command line frontend starts). It probably 
requires a bunch of work to get that working without a restart, so I think this 
can be fixed as a follow-up.

On a more general note: I'm not sure why we need `m_current_autosuggestion`. 
There is a bunch of code that tries to keep that variable up-to-date with what 
is typed, but unless I'm missing something this is just the text the user has 
entered so far? If yes, then you can also just get the current user input from 
Editline (see the first few lines of the `Editline::TabCommand` function for 
how to do that).




Comment at: lldb/source/Core/IOHandler.cpp:277
 }
+if (debugger.GetUseAutosuggestion())
+  m_editline_up->UseAutosuggestion();

If this was a normal setter you could just do 
`m_editline_up->SetShowAutosuggestion(debugger.GetShowAutosuggestions());`.



Comment at: lldb/source/Host/common/Editline.cpp:1009
+  if (m_use_autosuggestion) {
+int length = (int)(to_add.length());
+if ((int)(m_current_autosuggestion.length()) > length &&

If you make it `size_t` then you also don't need all the casting below.



Comment at: lldb/source/Host/common/Editline.cpp:1447
+
+bool Editline::UseAutosuggestion() {
+  m_use_autosuggestion = true;

This shouldn't return a `bool` and have a normal setter name 
(`SetShowAutosuggestion`). Also the better name for this setting should be 
`ShowAutosuggestion` and not `UseAutosuggestion` (the same goes for all other 
variables/functions in the rest of this patch).


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [lldb] 1728dec - [lldb/Lua] Recognize "quit" as a way to exit the script interpreter.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-22T09:27:12-07:00
New Revision: 1728dec255a5336303b301e26ec22eca2000b593

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

LOG: [lldb/Lua] Recognize "quit" as a way to exit the script interpreter.

Add a way to quit the interactive script interpreter from a shell tests.
Currently, the only way (that I know) to exit the interactive Lua
interpreter is to send a EOF with CTRL-D. I noticed that the embedded
Python script interpreter accepts quit (while the regular python
interpreter doesn't). I've added a special case to the Lua interpreter
to do the same.

Differential revision: https://reviews.llvm.org/D82272

Added: 
lldb/test/Shell/ScriptInterpreter/Lua/quit.test

Modified: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
index cbd639417990..f8fd8ff94aa3 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -39,6 +39,11 @@ class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &data) override {
+if (llvm::StringRef(data).rtrim() == "quit") {
+  io_handler.SetIsDone(true);
+  return;
+}
+
 if (llvm::Error error = m_script_interpreter.GetLua().Run(data)) {
   *GetOutputStreamFileSP() << llvm::toString(std::move(error));
 }

diff  --git a/lldb/test/Shell/ScriptInterpreter/Lua/quit.test 
b/lldb/test/Shell/ScriptInterpreter/Lua/quit.test
new file mode 100644
index ..e47c66c5a10a
--- /dev/null
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/quit.test
@@ -0,0 +1,10 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+script
+print(95000 + 126)
+quit
+target list
+# CHECK: 95126
+# CHECK: No targets



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

[Lldb-commits] [PATCH] D82272: [lldb/Lua] Recognize "quit" as a way to exit the interactive script interpreter.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D82272#2106402 , @labath wrote:

> It seems reasonable. Btw, have you figured out how the embedded python does 
> it? -- I haven't been able to find the code for handling that.


No, I couldn't find the code either.

> The python interpreter also intercepts `os.exit()` to avoid terminating the 
> program (it just terminates the python session). I suppose we should do the 
> same with lua's `os.exit`...

Yep, that sounds good.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D82272



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

[Lldb-commits] [PATCH] D82272: [lldb/Lua] Recognize "quit" as a way to exit the interactive script interpreter.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1728dec255a5: [lldb/Lua] Recognize "quit" as a way 
to exit the script interpreter. (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82272

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/quit.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/quit.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/quit.test
@@ -0,0 +1,10 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+script
+print(95000 + 126)
+quit
+target list
+# CHECK: 95126
+# CHECK: No targets
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -39,6 +39,11 @@
 
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &data) override {
+if (llvm::StringRef(data).rtrim() == "quit") {
+  io_handler.SetIsDone(true);
+  return;
+}
+
 if (llvm::Error error = m_script_interpreter.GetLua().Run(data)) {
   *GetOutputStreamFileSP() << llvm::toString(std::move(error));
 }


Index: lldb/test/Shell/ScriptInterpreter/Lua/quit.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/quit.test
@@ -0,0 +1,10 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+script
+print(95000 + 126)
+quit
+target list
+# CHECK: 95126
+# CHECK: No targets
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -39,6 +39,11 @@
 
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &data) override {
+if (llvm::StringRef(data).rtrim() == "quit") {
+  io_handler.SetIsDone(true);
+  return;
+}
+
 if (llvm::Error error = m_script_interpreter.GetLua().Run(data)) {
   *GetOutputStreamFileSP() << llvm::toString(std::move(error));
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 updated this revision to Diff 272479.
gedatsu217 added a comment.

Change the name and return of the function( bool UseAutosuggestion() -> void 
SetShowAutosuggestion (bool) ) (ll. 1447 in Editline.cpp and ll.194 in 
Editline.h).
int -> size_t (ll. 1009 in Editline.cpp).
Fix for normal setter (ll. 269 in IOHandler.cpp).

> The Editline instance isn't recreated when you change the settings, so you 
> need to restart LLDB for this to work. However, settings aren't automatically 
> saved, so I think the right way to do this by putting the settings set 
> show-autosuggestion true into the ~/.lldbinit file (which will be 
> automatically executed before LLDB's command line frontend starts). It 
> probably requires a bunch of work to get that working without a restart, so I 
> think this can be fixed as a follow-up.

There is no ~/.lldbinit in my environment (I do not why). Instead, there is 
~/.lldb directory (but there are just command history file there.).

> On a more general note: I'm not sure why we need m_current_autosuggestion. 
> There is a bunch of code that tries to keep that variable up-to-date with 
> what is typed, but unless I'm missing something this is just the text the 
> user has entered so far? If yes, then you can also just get the current user 
> input from Editline (see the first few lines of the Editline::TabCommand 
> function for how to do that).

I think "m_current_autosuggestion" is needed. That is because it keeps the 
characters for the suggestion, not just user input. For example, when "b" is 
typed, "reakpoint" is saved in m_current_autosuggestion (when "breakpoint" is 
typed before).

When a character is typed, Editline::TypedCharacter is called and 
m_current_autosuggestion is renewed every time. On the other hand, 
Editline::ApplyCompleteCommand, which execute suggestion actually, is not 
called unless C^f is typed. Therefore I think the suggestion parts should be 
saved until it is called. Indeed, I can get current user input by the first few 
lines of the Editline::TabCommand function, but it cannot save the suggestion 
parts probably.

However, I noticed that "to_add" in Editline::TypedCharacter is unnecessary, so 
removeed it.


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

https://reviews.llvm.org/D81001

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Host/common/Editline.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1866,6 +1866,21 @@
   HandleCompletionMatches(request);
 }
 
+llvm::Optional
+CommandInterpreter::GetAutoSuggestionForCommand(llvm::StringRef line,
+std::string &result) {
+  const size_t s = m_command_history.GetSize();
+  for (size_t i = 0; i < s; ++i) {
+llvm::StringRef entry = m_command_history.GetStringAtIndex(i);
+if (entry.startswith(line)) {
+  llvm::StringRef res = entry.substr(line.size());
+  result = res.str();
+  return result;
+}
+  }
+  return llvm::None;
+}
+
 CommandInterpreter::~CommandInterpreter() {}
 
 void CommandInterpreter::UpdatePrompt(llvm::StringRef new_prompt) {
Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Host/Editline.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/CompletionRequest.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBAssert.h"
@@ -1004,6 +1005,17 @@
   to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
   if (request.GetParsedArg().IsQuoted())
 to_add.push_back(request.GetParsedArg().GetQuoteChar());
+  if (m_use_autosuggestion && !to_add.empty()) {
+size_t length = to_add.length();
+if (m_current_autosuggestion.length() > length &&
+to_add == m_current_autosuggestion.substr(0, length)) {
+  to_add.push_back(' ');
+  el_insertstr(m_editline, to_add.c_str());
+  m_current_autosuggestion =
+  m_current_autosuggestion.substr(length + 1);
+  return CC_REFRESH;
+}
+  }
   to_add.push_back(' ');
   el_insertstr(m_editline, to_add.c_str());
   break;
@@ -1020,6 +1032,7 @@
   break;
 }
 }
+m_current_autosuggestion = "";
 return CC_REDISPLAY;
   }
 
@@ -1040,6 +1053,36 @@
   return CC_REDISPLAY;
 }
 
+unsigned char Editline::ApplyCom

[Lldb-commits] [PATCH] D82273: [lldb/Lua] Use the debugger's output and error file for Lua's I/O library.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:64
+
+  if (out) {
+lua_pushstring(m_lua_state, "stdout");

labath wrote:
> What should be the behavior if this is null? Can it even be null (should we 
> be asserting that it isn't)?
> It's not clear to me that we should be reusing the previously set stdout if 
> these arguments are null... Maybe we should be saving the original stdout 
> value and restoring it when we are done? That's what the python interpreter 
> seems to be doing..
I don't really see a benefit to storing it, I'm fine with just asserting that 
the file is not null. 


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

https://reviews.llvm.org/D82273



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

[Lldb-commits] [PATCH] D82273: [lldb/Lua] Use the debugger's output and error file for Lua's I/O library.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 272579.

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

https://reviews.llvm.org/D82273

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/io.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/io.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/io.test
@@ -0,0 +1,16 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+io.write(95000 + 126, "\n")
+quit
+script
+io.write(95000 + 14, "\n")
+# STDOUT: 95126
+# STDOUT-NOT: 95014
+# STDERR: 95014
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -30,6 +30,9 @@
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
 m_script_interpreter(script_interpreter) {
+llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
+debugger.GetOutputFile().GetStream(),
+debugger.GetErrorFile().GetStream()));
 llvm::cantFail(m_script_interpreter.EnterSession(debugger.GetID()));
   }
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -38,6 +38,7 @@
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
+  llvm::Error ChangeIO(FILE *out = nullptr, FILE *err = nullptr);
 
 private:
   lua_State *m_lua_state;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -57,3 +57,35 @@
   lua_setglobal(m_lua_state, module_name.GetCString());
   return llvm::Error::success();
 }
+
+llvm::Error Lua::ChangeIO(FILE *out, FILE *err) {
+  assert(out != nullptr);
+  assert(err != nullptr);
+
+  lua_getglobal(m_lua_state, "io");
+
+  lua_getfield(m_lua_state, -1, "stdout");
+  if (luaL_Stream *s = static_cast(
+  luaL_testudata(m_lua_state, -1, LUA_FILEHANDLE))) {
+s->f = out;
+lua_pop(m_lua_state, 1);
+  } else {
+lua_pop(m_lua_state, 2);
+return llvm::make_error("could not get stdout",
+   llvm::inconvertibleErrorCode());
+  }
+
+  lua_getfield(m_lua_state, -1, "stdout");
+  if (luaL_Stream *s = static_cast(
+  luaL_testudata(m_lua_state, -1, LUA_FILEHANDLE))) {
+s->f = out;
+lua_pop(m_lua_state, 1);
+  } else {
+lua_pop(m_lua_state, 2);
+return llvm::make_error("could not get stderr",
+   llvm::inconvertibleErrorCode());
+  }
+
+  lua_pop(m_lua_state, 1);
+  return llvm::Error::success();
+}


Index: lldb/test/Shell/ScriptInterpreter/Lua/io.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/io.test
@@ -0,0 +1,16 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+io.write(95000 + 126, "\n")
+quit
+script
+io.write(95000 + 14, "\n")
+# STDOUT: 95126
+# STDOUT-NOT: 95014
+# STDERR: 95014
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -30,6 +30,9 @@
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
 m_script_interpreter(script_interpreter) {
+llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
+debugger.GetOutputFile().GetStream(),
+debugger.GetErrorFile().GetStream()));
 llvm::cantFail(m_script_interpreter.EnterSession(debugger.GetID()));
   }
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterp

[Lldb-commits] [PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

[Jack Andersen via Phabricator via lldb-commits]

jackoalan updated this revision to Diff 272597.
jackoalan added a comment.

- Added a test for scoped enums (works as-is but still worth testing).
- Less frivolous use of `auto`
- Made the floating point vars constexpr to maintain validity in clang's 
internals.
- AstRestoreTest CLASS tests run for both DIA and Native. A pure NativePDB test 
that does not depend on MSVC would require extending `s_constant.cpp` 
(`S_CONSTANT` not generated by clang-cl) but I'm not certain how to regenerate 
the listing file with comments (unless that's done by hand). For now, this 
change should only be considered relevant for MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/Shell/SymbolFile/PDB/Inputs/AstRestoreTest.cpp
  lldb/test/Shell/SymbolFile/PDB/ast-restore.test
  llvm/include/llvm/DebugInfo/PDB/PDBTypes.h

Index: llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
===
--- llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
+++ llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_DEBUGINFO_PDB_PDBTYPES_H
 #define LLVM_DEBUGINFO_PDB_PDBTYPES_H
 
+#include "llvm/ADT/APFloat.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBFrameData.h"
@@ -464,6 +465,88 @@
 char *String;
   } Value;
 
+  bool isIntegralType() const {
+switch (Type) {
+case Bool:
+case Int8:
+case Int16:
+case Int32:
+case Int64:
+case UInt8:
+case UInt16:
+case UInt32:
+case UInt64:
+  return true;
+default:
+  return false;
+}
+  }
+
+#define VARIANT_WIDTH(Enum, NumBits)   \
+  case PDB_VariantType::Enum:  \
+return NumBits;
+
+  unsigned getBitWidth() const {
+switch (Type) {
+  VARIANT_WIDTH(Bool, 1u)
+  VARIANT_WIDTH(Int8, 8u)
+  VARIANT_WIDTH(Int16, 16u)
+  VARIANT_WIDTH(Int32, 32u)
+  VARIANT_WIDTH(Int64, 64u)
+  VARIANT_WIDTH(Single, 32u)
+  VARIANT_WIDTH(Double, 64u)
+  VARIANT_WIDTH(UInt8, 8u)
+  VARIANT_WIDTH(UInt16, 16u)
+  VARIANT_WIDTH(UInt32, 32u)
+  VARIANT_WIDTH(UInt64, 64u)
+default:
+  assert(false && "Variant::toAPSInt called on non-numeric type");
+  return 0u;
+}
+  }
+
+#undef VARIANT_WIDTH
+
+#define VARIANT_APSINT(Enum, NumBits, IsUnsigned)  \
+  case PDB_VariantType::Enum:  \
+return APSInt(APInt(NumBits, Value.Enum), IsUnsigned);
+
+  APSInt toAPSInt() const {
+switch (Type) {
+  VARIANT_APSINT(Bool, 1u, true)
+  VARIANT_APSINT(Int8, 8u, false)
+  VARIANT_APSINT(Int16, 16u, false)
+  VARIANT_APSINT(Int32, 32u, false)
+  VARIANT_APSINT(Int64, 64u, false)
+  VARIANT_APSINT(UInt8, 8u, true)
+  VARIANT_APSINT(UInt16, 16u, true)
+  VARIANT_APSINT(UInt32, 32u, true)
+  VARIANT_APSINT(UInt64, 64u, true)
+default:
+  assert(false && "Variant::toAPSInt called on non-integral type");
+  return APSInt();
+}
+  }
+
+#undef VARIANT_APSINT
+
+  APFloat toAPFloat() const {
+// Float constants may be tagged as integers.
+switch (Type) {
+case PDB_VariantType::Single:
+case PDB_VariantType::UInt32:
+case PDB_VariantType::Int32:
+  return APFloat(Value.Single);
+case PDB_VariantType::Double:
+case PDB_VariantType::UInt64:
+case PDB_VariantType::Int64:
+  return APFloat(Value.Double);
+default:
+  assert(false && "Variant::toAPFloat called on non-floating-point type");
+  return APFloat::getZero(APFloat::IEEEsingle());
+}
+  }
+
 #define VARIANT_EQUAL_CASE(Enum)   \
   case PDB_VariantType::Enum:  \
 return Value.Enum == Other.Value.Enum;
Index: lldb/test/Shell/SymbolFile/PDB/ast-restore.test
===
--- lldb/test/Shell/SymbolFile/PDB/ast-restore.test
+++ lldb/test/Shell/SymbolFile/PDB/ast-restore.test
@@ -4,7 +4,8 @@
 RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix=ENUM %s
 RUN: lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix=GLOBAL %s
 RUN: lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix=BASE %s
-RUN: lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix=CLASS %s
+RUN: env LLDB_USE_NATIVE

[Lldb-commits] [PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

[Jack Andersen via Phabricator via lldb-commits]

jackoalan marked 4 inline comments as done.
jackoalan added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7324-7331
+  // If the variable is an enum type, take the underlying integer type as
+  // the type of the integer literal.
+  if (const EnumType *enum_type = llvm::dyn_cast(qt.getTypePtr())) {
+const EnumDecl *enum_decl = enum_type->getDecl();
+qt = enum_decl->getIntegerType();
+  }
+  var->setInit(IntegerLiteral::Create(ast, init_value, qt.getUnqualifiedType(),

aleksandr.urakov wrote:
> teemperor wrote:
> > aleksandr.urakov wrote:
> > > I'm not sure, can we initialize a member this way if it has a scoped enum 
> > > type?
> > (That code is from D81471 so I think I should answer that)
> > 
> > Well, it works and but it relies on CodeGen/Sema not double-checking that 
> > we get the enum type (and not the underlying int type). I can inject a 
> > CXXStaticCastExpr too and will update D81471. Thanks!
> Ok, thank you!
Added a test just in case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160



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