[Lldb-commits] [PATCH] D28519: Add format_provider for the Error class
labath added a comment. In https://reviews.llvm.org/D28519#641938, @clayborg wrote: > You can't add anything extra to the AsCString() since it returns a "const > char *". You can't return a StringRef because it isn't backed by anything. > You could return a std::string. > > My vote would be to leave AsCString() alone and have it just return a pointer > to the string buffer that it owns, and let the formatv stuff do the extra > formatting. The way I would add it is to add whatever text we want directly to the m_string variable that backs the returned `const char *`. Basically, I believe that if a function has a "natural" string conversion function (which `AsCString` is) then the **default** formatv conversion should be just that (obviously the formatv conversion can do other fancy stuff with the format modifiers, but that's another story). I believe we have converged to keeping this patch as is. If I don't hear any comments, I am going to land this tomorrow. https://reviews.llvm.org/D28519 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax
labath updated this revision to Diff 83967.
labath added a comment.
Herald added a subscriber: mgorny.
- Replace the hacky proof-of-concept implementation with something more
production-ready
- Add a option to control adding of source information (defaulting to on)
- Add some unit tests
- Add an example of how would ProcessWindowsLog look like with this syntax
https://reviews.llvm.org/D27459
Files:
include/lldb/Core/Log.h
source/Commands/CommandObjectLog.cpp
source/Core/Log.cpp
source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
tools/lldb-server/lldb-gdbserver.cpp
unittests/Core/CMakeLists.txt
unittests/Core/LogTest.cpp
Index: unittests/Core/LogTest.cpp
===
--- /dev/null
+++ unittests/Core/LogTest.cpp
@@ -0,0 +1,56 @@
+//===-- LogTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/Log.h"
+#include "lldb/Core/StreamString.h"
+#include "lldb/Host/Host.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static std::string GetLogString(uint32_t log_options, const char *format,
+int arg) {
+ std::shared_ptr stream_sp(new StreamString());
+ Log log_(stream_sp);
+ log_.GetOptions().Reset(log_options);
+ Log *log = &log_;
+ LLDB_LOG(log, format, arg);
+ return stream_sp->GetString();
+}
+
+TEST(LogTest, LLDB_LOG_nullptr) {
+ Log *log = nullptr;
+ LLDB_LOG(log, "{0}", 0); // Shouldn't crash
+}
+
+TEST(LogTest, log_options) {
+ EXPECT_EQ("Hello World 47\n", GetLogString(0, "Hello World {0}", 47));
+ EXPECT_EQ("Hello World 47\n",
+GetLogString(LLDB_LOG_OPTION_THREADSAFE, "Hello World {0}", 47));
+
+ {
+std::string msg =
+GetLogString(LLDB_LOG_OPTION_PREPEND_SEQUENCE, "Hello World {0}", 47);
+int seq_no;
+EXPECT_EQ(1, sscanf(msg.c_str(), "%d Hello World 47", &seq_no));
+ }
+
+ EXPECT_EQ(
+ "LogTest.cpp:GetLogString Hello "
+ "World 47\n",
+ GetLogString(LLDB_LOG_OPTION_PREPEND_ORIGIN, "Hello World {0}", 47));
+
+ EXPECT_EQ(llvm::formatv("[{0}/{1}] Hello World 47\n",
+ Host::GetCurrentProcessID(),
+ Host::GetCurrentThreadID())
+.str(),
+GetLogString(LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD,
+ "Hello World {0}", 47));
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -4,6 +4,7 @@
DataExtractorTest.cpp
ErrorTest.cpp
ListenerTest.cpp
+ LogTest.cpp
ScalarTest.cpp
StructuredDataTest.cpp
TimerTest.cpp
Index: tools/lldb-server/lldb-gdbserver.cpp
===
--- tools/lldb-server/lldb-gdbserver.cpp
+++ tools/lldb-server/lldb-gdbserver.cpp
@@ -425,7 +425,8 @@
}
if (!LLDBServerUtilities::SetupLogging(log_file, log_channels,
- LLDB_LOG_OPTION_PREPEND_TIMESTAMP))
+ LLDB_LOG_OPTION_PREPEND_TIMESTAMP |
+ LLDB_LOG_OPTION_PREPEND_ORIGIN))
return -1;
Log *log(lldb_private::GetLogIfAnyCategoriesSet(GDBR_LOG_VERBOSE));
Index: source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
===
--- source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
+++ source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
@@ -176,6 +176,7 @@
bool RegisterContextWindows_x86::ReadRegister(const RegisterInfo *reg_info,
RegisterValue ®_value) {
+ Log *log = ProcessWindowsLog::GetLogIfAllCategoriesSet(WINDOWS_LOG_REGISTERS);
if (!CacheAllRegisterValues())
return false;
@@ -203,7 +204,7 @@
return ReadRegisterHelper(CONTEXT_CONTROL, "EFLAGS", m_context.EFlags,
reg_value);
default:
-WINWARN_IFALL(WINDOWS_LOG_REGISTERS, "Requested unknown register %u", reg);
+LLDB_LOG(log, "Requested unknown register %u", reg);
break;
}
return false;
@@ -219,62 +220,52 @@
if (!CacheAllRegisterValues())
return false;
+ Log *log = ProcessWindowsLog::GetLogIfAllCategoriesSet(WINDOWS_LOG_REGISTERS);
uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
switch (reg) {
case lldb_eax_i386:
-WINLOG_IFALL(WINDOWS_LOG_REGISTERS,
Re: [Lldb-commits] [PATCH] D28519: Add format_provider for the Error class
Official Lgtm On Wed, Jan 11, 2017 at 4:48 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath added a comment. > > In https://reviews.llvm.org/D28519#641938, @clayborg wrote: > > > You can't add anything extra to the AsCString() since it returns a > "const char *". You can't return a StringRef because it isn't backed by > anything. You could return a std::string. > > > > My vote would be to leave AsCString() alone and have it just return a > pointer to the string buffer that it owns, and let the formatv stuff do the > extra formatting. > > > The way I would add it is to add whatever text we want directly to the > m_string variable that backs the returned `const char *`. Basically, I > believe that if a function has a "natural" string conversion function > (which `AsCString` is) then the **default** formatv conversion should be > just that (obviously the formatv conversion can do other fancy stuff with > the format modifiers, but that's another story). > > I believe we have converged to keeping this patch as is. If I don't hear > any comments, I am going to land this tomorrow. > > > https://reviews.llvm.org/D28519 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax
clayborg requested changes to this revision.
clayborg added a reviewer: clayborg.
clayborg added a comment.
This revision now requires changes to proceed.
Looks fine except I would rather not have file + line on by default.
Comment at: source/Commands/CommandObjectLog.cpp:51-52
{ LLDB_OPT_SET_1, false, "append", 'a', OptionParser::eNoArgument,
nullptr, nullptr, 0, eArgTypeNone, "Append to the log file instead of
overwriting." },
+ { LLDB_OPT_SET_1, false, "no-origin", 'O', OptionParser::eNoArgument,
nullptr, nullptr, 0, eArgTypeNone, "Do not prepend log origin (source file)
information." },
// clang-format on
};
This shouldn't be on by default. Maybe it should be renamed to "file-line".
Comment at: source/Commands/CommandObjectLog.cpp:103
+ log_options |= LLDB_LOG_OPTION_PREPEND_ORIGIN; // Defaults to on.
switch (short_option) {
Don't default this to on.
Comment at: source/Commands/CommandObjectLog.cpp:135-136
break;
+ case 'O':
+log_options &= ~LLDB_LOG_OPTION_PREPEND_ORIGIN;
default:
Reverse
https://reviews.llvm.org/D27459
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28519: Add format_provider for the Error class
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine. https://reviews.llvm.org/D28519 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291738 - Update to match clang r291737.
Author: rsmith
Date: Wed Jan 11 20:37:54 2017
New Revision: 291738
URL: http://llvm.org/viewvc/llvm-project?rev=291738&view=rev
Log:
Update to match clang r291737.
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp?rev=291738&r1=291737&r2=291738&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
Wed Jan 11 20:37:54 2017
@@ -346,8 +346,7 @@ bool ASTResultSynthesizer::SynthesizeBod
ExprResult address_of_expr =
m_sema->CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, last_expr);
if (address_of_expr.get())
- m_sema->AddInitializerToDecl(result_decl, address_of_expr.get(), true,
- false);
+ m_sema->AddInitializerToDecl(result_decl, address_of_expr.get(), true);
else
return false;
} else {
@@ -359,7 +358,7 @@ bool ASTResultSynthesizer::SynthesizeBod
if (!result_decl)
return false;
-m_sema->AddInitializerToDecl(result_decl, last_expr, true, false);
+m_sema->AddInitializerToDecl(result_decl, last_expr, true);
}
DC->addDecl(result_decl);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
