Sorry, mail from lldb-commits has been very flakey for me for some reason, I didn't see this part of the thread till just now...
I still think this is a solution in search of a problem, and since we certainly shouldn't change all the log statements in lldb to use some macro - that is code churn for no very good reason - it will leave us with an inconsistent use of logging through-out lldb, which is unfortunate. I don't in general like macros that wrap syntax since then you can't reason about the code without knowing what the macro transformation is... But it is better than the NullLog solution. Jim > On Apr 30, 2015, at 11:39 AM, Zachary Turner <[email protected]> wrote: > > Also judging from Pavel's email it sounds like I'm not the only one who wants > to see more concise logging statements. So in that regard, locking this away > in ProcessWindows doesn't seem like the ideal solution either, because it > will just lead to other people who own different plugins or different areas > re-inventing their own method to shorten the log statements. So I think it > would be great if we could come to some sort of compromise. I feel like the > macro solution is the best compromise I've seen as of yet. While there would > still be 2 ways to log (the current approach of checking the pointer very > time, vs using the macro), they would have basically identical performance > characteristics, and we could always begin migrating existing code toward the > macro method as we touch surrounding code. > > On Thu, Apr 30, 2015 at 10:55 AM Zachary Turner <[email protected]> wrote: > Even ignoring the 3 extra lines issue (which I disagree is a problem of my > own making, I think you'll find it's quite common for developers to want to > maximize the density of code on their screen), there is still the indentation > issue. > > Did you dislike the macro suggestion by Pavel (and my followup) as well? > There are no performance implications, no extraneous goo in front of the > actual content of the log statement, and nobody has to worry about using the > wrong method. Only reduced indentation and fewer lines of code. > > I haven't yet seen a good argument for at least that to not go in. > > On Thu, Apr 30, 2015 at 10:42 AM <[email protected]> wrote: > > > On Apr 29, 2015, at 7:23 PM, Zachary Turner <[email protected]> wrote: > > > > The problem is when the log statements aren't simple, which turns out to be > > most of the time in my case. They log statement itself wraps a couple > > times, and when i have an if statement with only one statement inside, i > > use curly braces if the line wraps. So I'm adding 3 additional lines with > > each log statement that are removed by having the NullLog pattern. Add to > > that it decreases indentation (which is important for me since i have two > > or three windows opened side by side) and the difference between reading > > code that uses if (log) versus code that just calls the print method is > > night and day for me. > > > > What do you think about Enrico's suggestion? If there's a way to make this > > useful for everyone it's certainly better than me doing my own thing in > > ProcessWindows, but when i look at my code decorated with log statements > > using the existing paradigm, it's significantly harder to read than using > > the NullLog paradigm, so i guess i will still move it to ProcessWindows if > > that's the only way. > > > I don't find Enrico's suggestion particularly appealing. After all it means > the log statements you wanted to make cleaner looking are going to have a > bunch of extraneous goo in front of the actual content to set up the > std::function that gets passed. In Enrico's example it is 30 or so chicken > tracks before you get to the log statement, and that's probably going to push > it far enough to the right in most cases you will want to wrap it anyway, so > I doubt it will make things look any nicer. > > I don't find writing the log messages out to 120 characters a problem for > viewing code side by side to be a big problem. If your monitor forces you to > shorten two windows in order to set them side by side, that just means the > ends of the log lines aren't visible. But when you are scanning code for > logic or comparing two implementations it's generally not the ends of the log > lines that you are scrutinizing, so that they run off the end of the window > is not a big deal. And I don't see where: > > if (log) > log->Printf("some text" > " some more text"); > > really requires brackets to be clear, the alignment of the pieces of text > (and printf arguments) generally makes what is going on clear... So it seems > to me you are solving a problem of your own making. > > If I had all my druthers I would rather not have two patterns for logging > hanging around in the code, especially when one of them requires more care to > keep it from having performance implications - if I were more fanatical about > insisting on my opinions I'd go remove the LogIf calls as well... But in any > case, I would really rather you didn't introduce this into general lldb code. > > Jim > > > > On Wed, Apr 29, 2015 at 6:49 PM <[email protected]> wrote: > > In general, for simple logs we do: > > > > if (log) > > log->Printf(""); > > > > So you are only saving one line and a bit of indentation per log. I'm not > > sure I buy that this makes the code that much uglier - especially because > > it's really the noise of the characters in the log string that distract > > your eye and you can't do anything about that. > > > > I would really rather not have to have two log printing routes, one of > > which you would use if you KNOW that the data you are marshalling is free > > to assemble, and one where you might have more complex data. It is often > > surprising how a simple lookup can end up in a slog through lots of debug > > info. And while it is true we have to think about that carefully in the > > algorithms we write for say stepping or variable printing or whatever, I > > would really rather not have folks have to consider that for logging. > > > > This is actually somewhat important because we deal with lots of folks who > > can't give us their projects when they run into debugging problems. So > > logging is the only way we can sort out what are sometimes very gnarly > > problems. I want logging to be constant cost regardless of the information > > you've decided to gather so that people will be free to make the logging as > > rich and informative as it needs to be. I just don't see enough benefit > > from this to warrant the cognitive burden it imposes on something that > > should be simple. > > > > If you really want this in the ProcessWindows plugin, that's your baby > > so... But please put in some comment saying "Watch out for the costs of > > the arguments you might muster here" and "Don't use this for generic > > logging" so the policy is clear... > > > > Jim > > > > > On Apr 29, 2015, at 6:35 PM, Zachary Turner <[email protected]> wrote: > > > > > > If you don't want it in Core, I can move it to ProcessWindows. But > > > locally I've added logging where you can't even look at the function and > > > tell what it does anymore because of all the logging. I really don't > > > want so much boilerplate code taking up screen space in important > > > methods, preventing you from understanding what the methods do because > > > each log statement is 4 lines instead of 1. > > > > > > I'm open to suggestions, but ultimately in the code I'm writing in the > > > windows plugin, I don't want to check if statements every time I write a > > > log statement, so I need some kind of solution that makes that not > > > necessary. > > > > > > On Wed, Apr 29, 2015 at 6:29 PM <[email protected]> wrote: > > > Yes, but that call is only used a handful of times, and only from code > > > written back before this was checked into llvm.org, so that's more an > > > argument for removing those calls than making that pattern more common. > > > > > > I really don't want people to have to reason about the expense of > > > logging. I want there to be full rich log output that costs nothing when > > > not turned on. It's so easy to add something that looks harmless to a > > > log, but that ends up pulling in a whole bunch of debug information that > > > we were trying hard not to touch. You probably won't notice that till > > > you run on something really big, but then we will get slowdowns for no > > > benefit. > > > > > > Jim > > > > > > > > > > On Apr 29, 2015, at 6:18 PM, Zachary Turner <[email protected]> wrote: > > > > > > > > It's also worth pointing out that the method I've implemented here is a > > > > more general case of something already in LLDB. > > > > lldb_private::LogIfXXXCategoriesSet(). These 2 methods are equivalent > > > > to getting the log, checking for null, and logging if non null, except > > > > they always evaluate the arguments, as does my method. But the method > > > > I've implemented here allows access to every method of Log. Like Warn, > > > > Error, etc, so is even more flexible > > > > On Wed, Apr 29, 2015 at 6:14 PM Enrico Granata <[email protected]> > > > > wrote: > > > > One possible avenue to fix the performance issue would be to have your > > > > log calls take a lambda expression that returns the string to be > > > > printed, e.g. > > > > > > > > #include <functional> > > > > #include <string> > > > > #include <iostream> > > > > #include <stdlib.h> > > > > > > > > typedef std::function<std::string()> LogFunction; > > > > > > > > void log(bool dolog, LogFunction f) { > > > > if (dolog) > > > > std::cout << f() << std::endl; > > > > } > > > > > > > > int main() { > > > > log(true, [] () -> std::string { return "hello world"; } ); > > > > log(false, [] {sleep(10); return "hi"; }); > > > > return 0; > > > > } > > > > > > > > I don’t think the code gets much better doing this compared to the > > > > previous if (log) model we were using - but that’s at least arguable > > > > > > > > > > > >> More importantly, I don't think this is a good change. I want to be > > > >> able to freely put complex log statements where ever I need without > > > >> having to worry about the performance implications. That was always > > > >> possible, because the cost of not logging was checking if log was NULL. > > > >> > > > >> With this change I'm always going to get back a log, so I'm always > > > >> going to marshall the arguments, and do whatever work was going to go > > > >> into printing, etc and then call a virtual function that does nothing > > > >> with the results. > > > >> > > > >> That seems undesirable to me. You could work around this by having > > > >> any code that does logging in a loop or that has complex arguments > > > >> check if the log is a null log and not do the work if it is, but then > > > >> we're right back where we started except now we only do the check > > > >> sometimes, making things even more confusing. > > > >> > > > >> Jim > > > >> > > > >> On Apr 29, 2015, at 4:30 PM, Sean Callanan <[email protected]> wrote: > > > >> > > > >> Zachary, > > > >> > > > >> Log.cpp doesn’t build anymore with this change. In particular, you > > > >> have > > > >> – > > > >> + virtual void > > > >> + VAPrintf(const char *format, va_list args); > > > >> – > > > >> but at several places VAPrintf is called with no va_list, e.g., > > > >> – > > > >> void > > > >> -Log::Error (const char *format, ...) > > > >> +Log::Error(const char *format, ...) > > > >> { > > > >> - char *arg_msg = NULL; > > > >> + char *arg_msg = nullptr; > > > >> va_list args; > > > >> - va_start (args, format); > > > >> - ::vasprintf (&arg_msg, format, args); > > > >> - va_end (args); > > > >> + va_start(args, format); > > > >> + ::vasprintf(&arg_msg, format, args); > > > >> + va_end(args); > > > >> > > > >> - if (arg_msg != NULL) > > > >> - { > > > >> - PrintfWithFlags (LLDB_LOG_FLAG_ERROR, "error: %s", arg_msg); > > > >> - free (arg_msg); > > > >> - } > > > >> + if (arg_msg == nullptr) > > > >> + return; > > > >> + > > > >> + VAPrintf("error: %s", arg_msg); > > > >> + free(arg_msg); > > > >> } > > > >> – > > > >> Do you have more commits coming that are going to resolve this problem? > > > >> > > > >> Sean > > > >> > > > >> > > > >>> On Apr 29, 2015, at 3:55 PM, Zachary Turner <[email protected]> > > > >>> wrote: > > > >>> > > > >>> Author: zturner > > > >>> Date: Wed Apr 29 17:55:28 2015 > > > >>> New Revision: 236174 > > > >>> > > > >>> URL: http://llvm.org/viewvc/llvm-project?rev=236174&view=rev > > > >>> Log: > > > >>> Introduce a NullLog class, which ignores all messages. > > > >>> > > > >>> The purpose of this class is so that GetLogIfAllCategoriesSet > > > >>> can always return an instance of some class, whether it be a real > > > >>> logging class or a "null" class, which ignores messages. Code > > > >>> that is littered with if statements that only log if the pointer > > > >>> is non-null can get very unwieldy very quickly, so this should > > > >>> help code readability in such circumstances. > > > >>> > > > >>> Since I'm in this code anyway, I'm also deleting the > > > >>> PrintfWithFlags methods, as well as all the flags, since they > > > >>> appear to be dead code that have been superceded by newer > > > >>> mechanisms and all the flags are simply ignored. > > > >>> > > > >>> Added: > > > >>> lldb/trunk/include/lldb/Core/NullLog.h > > > >>> lldb/trunk/source/Core/NullLog.cpp > > > >>> Modified: > > > >>> lldb/trunk/include/lldb/Core/Log.h > > > >>> lldb/trunk/source/Core/CMakeLists.txt > > > >>> lldb/trunk/source/Core/Log.cpp > > > >>> > > > >>> Modified: lldb/trunk/include/lldb/Core/Log.h > > > >>> URL: > > > >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Log.h?rev=236174&r1=236173&r2=236174&view=diff > > > >>> ============================================================================== > > > >>> --- lldb/trunk/include/lldb/Core/Log.h (original) > > > >>> +++ lldb/trunk/include/lldb/Core/Log.h Wed Apr 29 17:55:28 2015 > > > >>> @@ -26,17 +26,6 @@ > > > >>> #include "lldb/Core/PluginInterface.h" > > > >>> > > > >>> //---------------------------------------------------------------------- > > > >>> -// Logging types > > > >>> -//---------------------------------------------------------------------- > > > >>> -#define LLDB_LOG_FLAG_STDOUT (1u << 0) > > > >>> -#define LLDB_LOG_FLAG_STDERR (1u << 1) > > > >>> -#define LLDB_LOG_FLAG_FATAL (1u << 2) > > > >>> -#define LLDB_LOG_FLAG_ERROR (1u << 3) > > > >>> -#define LLDB_LOG_FLAG_WARNING (1u << 4) > > > >>> -#define LLDB_LOG_FLAG_DEBUG (1u << 5) > > > >>> -#define LLDB_LOG_FLAG_VERBOSE (1u << 6) > > > >>> - > > > >>> -//---------------------------------------------------------------------- > > > >>> // Logging Options > > > >>> //---------------------------------------------------------------------- > > > >>> #define LLDB_LOG_OPTION_THREADSAFE (1u << 0) > > > >>> @@ -61,12 +50,10 @@ public: > > > >>> //------------------------------------------------------------------ > > > >>> // Callback definitions for abstracted plug-in log access. > > > >>> //------------------------------------------------------------------ > > > >>> - typedef void (*DisableCallback) (const char **categories, Stream > > > >>> *feedback_strm); > > > >>> - typedef Log * (*EnableCallback) (lldb::StreamSP &log_stream_sp, > > > >>> - uint32_t log_options, > > > >>> - const char **categories, > > > >>> - Stream *feedback_strm); > > > >>> - typedef void (*ListCategoriesCallback) (Stream *strm); > > > >>> + typedef void (*DisableCallback)(const char **categories, Stream > > > >>> *feedback_strm); > > > >>> + typedef Log *(*EnableCallback)(lldb::StreamSP &log_stream_sp, > > > >>> uint32_t log_options, const char **categories, > > > >>> + Stream *feedback_strm); > > > >>> + typedef void (*ListCategoriesCallback)(Stream *strm); > > > >>> > > > >>> struct Callbacks > > > >>> { > > > >>> @@ -79,86 +66,78 @@ public: > > > >>> // Static accessors for logging channels > > > >>> //------------------------------------------------------------------ > > > >>> static void > > > >>> - RegisterLogChannel (const ConstString &channel, > > > >>> - const Log::Callbacks &log_callbacks); > > > >>> + RegisterLogChannel(const ConstString &channel, const > > > >>> Log::Callbacks &log_callbacks); > > > >>> > > > >>> static bool > > > >>> - UnregisterLogChannel (const ConstString &channel); > > > >>> + UnregisterLogChannel(const ConstString &channel); > > > >>> > > > >>> static bool > > > >>> - GetLogChannelCallbacks (const ConstString &channel, > > > >>> - Log::Callbacks &log_callbacks); > > > >>> - > > > >>> + GetLogChannelCallbacks(const ConstString &channel, > > > >>> Log::Callbacks &log_callbacks); > > > >>> > > > >>> static void > > > >>> - EnableAllLogChannels (lldb::StreamSP &log_stream_sp, > > > >>> - uint32_t log_options, > > > >>> - const char **categories, > > > >>> - Stream *feedback_strm); > > > >>> + EnableAllLogChannels(lldb::StreamSP &log_stream_sp, uint32_t > > > >>> log_options, const char **categories, > > > >>> + Stream *feedback_strm); > > > >>> > > > >>> static void > > > >>> - DisableAllLogChannels (Stream *feedback_strm); > > > >>> + DisableAllLogChannels(Stream *feedback_strm); > > > >>> > > > >>> static void > > > >>> - ListAllLogChannels (Stream *strm); > > > >>> + ListAllLogChannels(Stream *strm); > > > >>> > > > >>> static void > > > >>> - Initialize (); > > > >>> + Initialize(); > > > >>> > > > >>> static void > > > >>> - Terminate (); > > > >>> - > > > >>> + Terminate(); > > > >>> + > > > >>> //------------------------------------------------------------------ > > > >>> // Auto completion > > > >>> //------------------------------------------------------------------ > > > >>> static void > > > >>> - AutoCompleteChannelName (const char *channel_name, > > > >>> - StringList &matches); > > > >>> + AutoCompleteChannelName(const char *channel_name, StringList > > > >>> &matches); > > > >>> > > > >>> //------------------------------------------------------------------ > > > >>> // Member functions > > > >>> //------------------------------------------------------------------ > > > >>> - Log (); > > > >>> + Log(); > > > >>> > > > >>> - Log (const lldb::StreamSP &stream_sp); > > > >>> + Log(const lldb::StreamSP &stream_sp); > > > >>> > > > >>> - ~Log (); > > > >>> - > > > >>> - void > > > >>> - PutCString (const char *cstr); > > > >>> + virtual > > > >>> + ~Log(); > > > >>> > > > >>> - void > > > >>> - Printf (const char *format, ...) __attribute__ ((format > > > >>> (printf, 2, 3))); > > > >>> + virtual void > > > >>> + PutCString(const char *cstr); > > > >>> > > > >>> - void > > > >>> - VAPrintf (const char *format, va_list args); > > > >>> + virtual void > > > >>> + Printf(const char *format, ...) __attribute__((format(printf, 2, > > > >>> 3))); > > > >>> > > > >>> - void > > > >>> - PrintfWithFlags( uint32_t flags, const char *format, ...) > > > >>> __attribute__ ((format (printf, 3, 4))); > > > >>> + virtual void > > > >>> + VAPrintf(const char *format, va_list args); > > > >>> > > > >>> - void > > > >>> - LogIf (uint32_t mask, const char *fmt, ...) __attribute__ > > > >>> ((format (printf, 3, 4))); > > > >>> + virtual void > > > >>> + LogIf(uint32_t mask, const char *fmt, ...) > > > >>> __attribute__((format(printf, 3, 4))); > > > >>> > > > >>> - void > > > >>> - Debug (const char *fmt, ...) __attribute__ ((format (printf, 2, > > > >>> 3))); > > > >>> + virtual void > > > >>> + Debug(const char *fmt, ...) __attribute__((format(printf, 2, > > > >>> 3))); > > > >>> > > > >>> - void > > > >>> - DebugVerbose (const char *fmt, ...) __attribute__ ((format > > > >>> (printf, 2, 3))); > > > >>> + virtual void > > > >>> + DebugVerbose(const char *fmt, ...) __attribute__((format(printf, > > > >>> 2, 3))); > > > >>> > > > >>> - void > > > >>> - Error (const char *fmt, ...) __attribute__ ((format (printf, 2, > > > >>> 3))); > > > >>> + virtual void > > > >>> + Error(const char *fmt, ...) __attribute__((format(printf, 2, > > > >>> 3))); > > > >>> > > > >>> - void > > > >>> - FatalError (int err, const char *fmt, ...) __attribute__ > > > >>> ((format (printf, 3, 4))); > > > >>> + virtual void > > > >>> + FatalError(int err, const char *fmt, ...) > > > >>> __attribute__((format(printf, 3, 4))); > > > >>> > > > >>> - void > > > >>> - Verbose (const char *fmt, ...) __attribute__ ((format (printf, > > > >>> 2, 3))); > > > >>> + virtual void > > > >>> + Verbose(const char *fmt, ...) __attribute__((format(printf, 2, > > > >>> 3))); > > > >>> > > > >>> - void > > > >>> - Warning (const char *fmt, ...) __attribute__ ((format (printf, > > > >>> 2, 3))); > > > >>> + virtual void > > > >>> + Warning(const char *fmt, ...) __attribute__((format(printf, 2, > > > >>> 3))); > > > >>> > > > >>> - void > > > >>> - WarningVerbose (const char *fmt, ...) __attribute__ ((format > > > >>> (printf, 2, 3))); > > > >>> + virtual void > > > >>> + WarningVerbose(const char *fmt, ...) > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> > > > >>> Flags & > > > >>> GetOptions(); > > > >>> @@ -179,7 +158,7 @@ public: > > > >>> GetDebug() const; > > > >>> > > > >>> void > > > >>> - SetStream (const lldb::StreamSP &stream_sp) > > > >>> + SetStream(const lldb::StreamSP &stream_sp) > > > >>> { > > > >>> m_stream_sp = stream_sp; > > > >>> } > > > >>> @@ -192,43 +171,35 @@ protected: > > > >>> Flags m_options; > > > >>> Flags m_mask_bits; > > > >>> > > > >>> - void > > > >>> - PrintfWithFlagsVarArg (uint32_t flags, const char *format, > > > >>> va_list args); > > > >>> - > > > >>> private: > > > >>> - DISALLOW_COPY_AND_ASSIGN (Log); > > > >>> + DISALLOW_COPY_AND_ASSIGN(Log); > > > >>> }; > > > >>> > > > >>> > > > >>> class LogChannel : public PluginInterface > > > >>> { > > > >>> public: > > > >>> - LogChannel (); > > > >>> + LogChannel(); > > > >>> > > > >>> - virtual > > > >>> - ~LogChannel (); > > > >>> + virtual ~LogChannel(); > > > >>> > > > >>> - static lldb::LogChannelSP > > > >>> - FindPlugin (const char *plugin_name); > > > >>> + static lldb::LogChannelSP FindPlugin(const char *plugin_name); > > > >>> > > > >>> // categories is a an array of chars that ends with a NULL element. > > > >>> - virtual void > > > >>> - Disable (const char **categories, Stream *feedback_strm) = 0; > > > >>> + virtual void Disable(const char **categories, Stream > > > >>> *feedback_strm) = 0; > > > >>> > > > >>> - virtual bool > > > >>> - Enable (lldb::StreamSP &log_stream_sp, > > > >>> - uint32_t log_options, > > > >>> - Stream *feedback_strm, // Feedback stream for > > > >>> argument errors etc > > > >>> - const char **categories) = 0;// The categories to enable > > > >>> within this logging stream, if empty, enable default set > > > >>> + virtual bool Enable( > > > >>> + lldb::StreamSP &log_stream_sp, uint32_t log_options, > > > >>> + Stream *feedback_strm, // Feedback stream for argument > > > >>> errors etc > > > >>> + const char **categories) = 0; // The categories to enable > > > >>> within this logging stream, if empty, enable default set > > > >>> > > > >>> - virtual void > > > >>> - ListCategories (Stream *strm) = 0; > > > >>> + virtual void ListCategories(Stream *strm) = 0; > > > >>> > > > >>> protected: > > > >>> std::unique_ptr<Log> m_log_ap; > > > >>> > > > >>> private: > > > >>> - DISALLOW_COPY_AND_ASSIGN (LogChannel); > > > >>> + DISALLOW_COPY_AND_ASSIGN(LogChannel); > > > >>> }; > > > >>> > > > >>> > > > >>> > > > >>> Added: lldb/trunk/include/lldb/Core/NullLog.h > > > >>> URL: > > > >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/NullLog.h?rev=236174&view=auto > > > >>> ============================================================================== > > > >>> --- lldb/trunk/include/lldb/Core/NullLog.h (added) > > > >>> +++ lldb/trunk/include/lldb/Core/NullLog.h Wed Apr 29 17:55:28 2015 > > > >>> @@ -0,0 +1,58 @@ > > > >>> +//===-- NullLog.h -----------------------------------------------*- > > > >>> C++ -*-===// > > > >>> +// > > > >>> +// The LLVM Compiler Infrastructure > > > >>> +// > > > >>> +// This file is distributed under the University of Illinois Open > > > >>> Source > > > >>> +// License. See LICENSE.TXT for details. > > > >>> +// > > > >>> +//===----------------------------------------------------------------------===// > > > >>> + > > > >>> +#ifndef liblldb_Core_NullLog_H_ > > > >>> +#define liblldb_Core_NullLog_H_ > > > >>> + > > > >>> +#include "lldb/Core/Log.h" > > > >>> + > > > >>> +//---------------------------------------------------------------------- > > > >>> +// Logging Functions > > > >>> +//---------------------------------------------------------------------- > > > >>> +namespace lldb_private > > > >>> +{ > > > >>> + > > > >>> +class NullLog : public Log > > > >>> +{ > > > >>> + NullLog(NullLog &) = delete; > > > >>> + NullLog &operator=(NullLog &) = delete; > > > >>> + > > > >>> + public: > > > >>> + > > > >>> //------------------------------------------------------------------ > > > >>> + // Member functions > > > >>> + > > > >>> //------------------------------------------------------------------ > > > >>> + NullLog(); > > > >>> + ~NullLog(); > > > >>> + > > > >>> + void PutCString(const char *cstr) override; > > > >>> + > > > >>> + void Printf(const char *format, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void VAPrintf(const char *format, va_list args) override; > > > >>> + > > > >>> + void LogIf(uint32_t mask, const char *fmt, ...) override > > > >>> __attribute__((format(printf, 3, 4))); > > > >>> + > > > >>> + void Debug(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void DebugVerbose(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void Error(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void FatalError(int err, const char *fmt, ...) override > > > >>> __attribute__((format(printf, 3, 4))); > > > >>> + > > > >>> + void Verbose(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void Warning(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> + > > > >>> + void WarningVerbose(const char *fmt, ...) override > > > >>> __attribute__((format(printf, 2, 3))); > > > >>> +}; > > > >>> + > > > >>> +} // namespace lldb_private > > > >>> + > > > >>> +#endif // liblldb_Core_NullLog_H_ > > > >>> > > > >>> Modified: lldb/trunk/source/Core/CMakeLists.txt > > > >>> URL: > > > >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/CMakeLists.txt?rev=236174&r1=236173&r2=236174&view=diff > > > >>> ============================================================================== > > > >>> --- lldb/trunk/source/Core/CMakeLists.txt (original) > > > >>> +++ lldb/trunk/source/Core/CMakeLists.txt Wed Apr 29 17:55:28 2015 > > > >>> @@ -38,6 +38,7 @@ add_lldb_library(lldbCore > > > >>> Module.cpp > > > >>> ModuleChild.cpp > > > >>> ModuleList.cpp > > > >>> + NullLog.cpp > > > >>> Opcode.cpp > > > >>> PluginManager.cpp > > > >>> RegisterValue.cpp > > > >>> > > > >>> Modified: lldb/trunk/source/Core/Log.cpp > > > >>> URL: > > > >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Log.cpp?rev=236174&r1=236173&r2=236174&view=diff > > > >>> ============================================================================== > > > >>> --- lldb/trunk/source/Core/Log.cpp (original) > > > >>> +++ lldb/trunk/source/Core/Log.cpp Wed Apr 29 17:55:28 2015 > > > >>> @@ -77,6 +77,23 @@ Log::GetMask() const > > > >>> return m_mask_bits; > > > >>> } > > > >>> > > > >>> +void > > > >>> +Log::PutCString(const char *cstr) > > > >>> +{ > > > >>> + Printf("%s", cstr); > > > >>> +} > > > >>> + > > > >>> +//---------------------------------------------------------------------- > > > >>> +// Simple variable argument logging with flags. > > > >>> +//---------------------------------------------------------------------- > > > >>> +void > > > >>> +Log::Printf(const char *format, ...) > > > >>> +{ > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + VAPrintf(format, args); > > > >>> + va_end(args); > > > >>> +} > > > >>> > > > >>> //---------------------------------------------------------------------- > > > >>> // All logging eventually boils down to this function call. If we have > > > >>> @@ -84,7 +101,7 @@ Log::GetMask() const > > > >>> // a valid file handle, we also log to the file. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::PrintfWithFlagsVarArg (uint32_t flags, const char *format, > > > >>> va_list args) > > > >>> +Log::VAPrintf(const char *format, va_list args) > > > >>> { > > > >>> // Make a copy of our stream shared pointer in case someone > > > >>> disables our > > > >>> // log while we are logging and releases the stream > > > >>> @@ -136,59 +153,20 @@ Log::PrintfWithFlagsVarArg (uint32_t fla > > > >>> } > > > >>> } > > > >>> > > > >>> - > > > >>> -void > > > >>> -Log::PutCString (const char *cstr) > > > >>> -{ > > > >>> - Printf ("%s", cstr); > > > >>> -} > > > >>> - > > > >>> - > > > >>> -//---------------------------------------------------------------------- > > > >>> -// Simple variable argument logging with flags. > > > >>> -//---------------------------------------------------------------------- > > > >>> -void > > > >>> -Log::Printf(const char *format, ...) > > > >>> -{ > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (0, format, args); > > > >>> - va_end (args); > > > >>> -} > > > >>> - > > > >>> -void > > > >>> -Log::VAPrintf (const char *format, va_list args) > > > >>> -{ > > > >>> - PrintfWithFlagsVarArg (0, format, args); > > > >>> -} > > > >>> - > > > >>> - > > > >>> -//---------------------------------------------------------------------- > > > >>> -// Simple variable argument logging with flags. > > > >>> -//---------------------------------------------------------------------- > > > >>> -void > > > >>> -Log::PrintfWithFlags (uint32_t flags, const char *format, ...) > > > >>> -{ > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (flags, format, args); > > > >>> - va_end (args); > > > >>> -} > > > >>> - > > > >>> //---------------------------------------------------------------------- > > > >>> // Print debug strings if and only if the global debug option is set > > > >>> to > > > >>> // a non-zero value. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::Debug (const char *format, ...) > > > >>> +Log::Debug(const char *format, ...) > > > >>> { > > > >>> - if (GetOptions().Test(LLDB_LOG_OPTION_DEBUG)) > > > >>> - { > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (LLDB_LOG_FLAG_DEBUG, format, args); > > > >>> - va_end (args); > > > >>> - } > > > >>> + if (!GetOptions().Test(LLDB_LOG_OPTION_DEBUG)) > > > >>> + return; > > > >>> + > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + VAPrintf(format, args); > > > >>> + va_end(args); > > > >>> } > > > >>> > > > >>> > > > >>> @@ -197,15 +175,15 @@ Log::Debug (const char *format, ...) > > > >>> // a non-zero value. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::DebugVerbose (const char *format, ...) > > > >>> +Log::DebugVerbose(const char *format, ...) > > > >>> { > > > >>> - if (GetOptions().AllSet (LLDB_LOG_OPTION_DEBUG | > > > >>> LLDB_LOG_OPTION_VERBOSE)) > > > >>> - { > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (LLDB_LOG_FLAG_DEBUG | > > > >>> LLDB_LOG_FLAG_VERBOSE, format, args); > > > >>> - va_end (args); > > > >>> - } > > > >>> + if (!GetOptions().AllSet(LLDB_LOG_OPTION_DEBUG | > > > >>> LLDB_LOG_OPTION_VERBOSE)) > > > >>> + return; > > > >>> + > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + VAPrintf(format, args); > > > >>> + va_end(args); > > > >>> } > > > >>> > > > >>> > > > >>> @@ -213,34 +191,34 @@ Log::DebugVerbose (const char *format, . > > > >>> // Log only if all of the bits are set > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::LogIf (uint32_t bits, const char *format, ...) > > > >>> +Log::LogIf(uint32_t bits, const char *format, ...) > > > >>> { > > > >>> - if (m_options.AllSet (bits)) > > > >>> - { > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (0, format, args); > > > >>> - va_end (args); > > > >>> - } > > > >>> + if (!m_options.AllSet(bits)) > > > >>> + return; > > > >>> + > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + VAPrintf(format, args); > > > >>> + va_end(args); > > > >>> } > > > >>> > > > >>> //---------------------------------------------------------------------- > > > >>> // Printing of errors that are not fatal. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::Error (const char *format, ...) > > > >>> +Log::Error(const char *format, ...) > > > >>> { > > > >>> - char *arg_msg = NULL; > > > >>> + char *arg_msg = nullptr; > > > >>> va_list args; > > > >>> - va_start (args, format); > > > >>> - ::vasprintf (&arg_msg, format, args); > > > >>> - va_end (args); > > > >>> + va_start(args, format); > > > >>> + ::vasprintf(&arg_msg, format, args); > > > >>> + va_end(args); > > > >>> > > > >>> - if (arg_msg != NULL) > > > >>> - { > > > >>> - PrintfWithFlags (LLDB_LOG_FLAG_ERROR, "error: %s", arg_msg); > > > >>> - free (arg_msg); > > > >>> - } > > > >>> + if (arg_msg == nullptr) > > > >>> + return; > > > >>> + > > > >>> + VAPrintf("error: %s", arg_msg); > > > >>> + free(arg_msg); > > > >>> } > > > >>> > > > >>> //---------------------------------------------------------------------- > > > >>> @@ -248,20 +226,20 @@ Log::Error (const char *format, ...) > > > >>> // immediately. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::FatalError (int err, const char *format, ...) > > > >>> +Log::FatalError(int err, const char *format, ...) > > > >>> { > > > >>> - char *arg_msg = NULL; > > > >>> + char *arg_msg = nullptr; > > > >>> va_list args; > > > >>> - va_start (args, format); > > > >>> - ::vasprintf (&arg_msg, format, args); > > > >>> - va_end (args); > > > >>> + va_start(args, format); > > > >>> + ::vasprintf(&arg_msg, format, args); > > > >>> + va_end(args); > > > >>> > > > >>> - if (arg_msg != NULL) > > > >>> + if (arg_msg != nullptr) > > > >>> { > > > >>> - PrintfWithFlags (LLDB_LOG_FLAG_ERROR | LLDB_LOG_FLAG_FATAL, > > > >>> "error: %s", arg_msg); > > > >>> - ::free (arg_msg); > > > >>> + VAPrintf("error: %s", arg_msg); > > > >>> + ::free(arg_msg); > > > >>> } > > > >>> - ::exit (err); > > > >>> + ::exit(err); > > > >>> } > > > >>> > > > >>> > > > >>> @@ -270,15 +248,15 @@ Log::FatalError (int err, const char *fo > > > >>> // enabled. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::Verbose (const char *format, ...) > > > >>> +Log::Verbose(const char *format, ...) > > > >>> { > > > >>> - if (m_options.Test(LLDB_LOG_OPTION_VERBOSE)) > > > >>> - { > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - PrintfWithFlagsVarArg (LLDB_LOG_FLAG_VERBOSE, format, args); > > > >>> - va_end (args); > > > >>> - } > > > >>> + if (!m_options.Test(LLDB_LOG_OPTION_VERBOSE)) > > > >>> + return; > > > >>> + > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + VAPrintf(format, args); > > > >>> + va_end(args); > > > >>> } > > > >>> > > > >>> //---------------------------------------------------------------------- > > > >>> @@ -286,40 +264,40 @@ Log::Verbose (const char *format, ...) > > > >>> // enabled. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::WarningVerbose (const char *format, ...) > > > >>> +Log::WarningVerbose(const char *format, ...) > > > >>> { > > > >>> - if (m_options.Test(LLDB_LOG_OPTION_VERBOSE)) > > > >>> - { > > > >>> - char *arg_msg = NULL; > > > >>> - va_list args; > > > >>> - va_start (args, format); > > > >>> - ::vasprintf (&arg_msg, format, args); > > > >>> - va_end (args); > > > >>> + if (!m_options.Test(LLDB_LOG_OPTION_VERBOSE)) > > > >>> + return; > > > >>> > > > >>> - if (arg_msg != NULL) > > > >>> - { > > > >>> - PrintfWithFlags (LLDB_LOG_FLAG_WARNING | > > > >>> LLDB_LOG_FLAG_VERBOSE, "warning: %s", arg_msg); > > > >>> - free (arg_msg); > > > >>> - } > > > >>> - } > > > >>> + char *arg_msg = nullptr; > > > >>> + va_list args; > > > >>> + va_start(args, format); > > > >>> + ::vasprintf(&arg_msg, format, args); > > > >>> + va_end(args); > > > >>> + > > > >>> + if (arg_msg == nullptr) > > > >>> + return; > > > >>> + > > > >>> + VAPrintf("warning: %s", arg_msg); > > > >>> + free(arg_msg); > > > >>> } > > > >>> //---------------------------------------------------------------------- > > > >>> // Printing of warnings that are not fatal. > > > >>> //---------------------------------------------------------------------- > > > >>> void > > > >>> -Log::Warning (const char *format, ...) > > > >>> +Log::Warning(const char *format, ...) > > > >>> { > > > >>> - char *arg_msg = NULL; > > > >>> + char *arg_msg = nullptr; > > > >>> va_list args; > > > >>> - va_start (args, format); > > > >>> - ::vasprintf (&arg_msg, format, args); > > > >>> - va_end (args); > > > >>> + va_start(args, format); > > > >>> + ::vasprintf(&arg_msg, format, args); > > > >>> + va_end(args); > > > >>> > > > >>> - if (arg_msg != NULL) > > > >>> - { > > > >>> - PrintfWithFlags (LLDB_LOG_FLAG_WARNING, "warning: %s", > > > >>> arg_msg); > > > >>> - free (arg_msg); > > > >>> - } > > > >>> + if (arg_msg == nullptr) > > > >>> + return; > > > >>> + > > > >>> + VAPrintf("warning: %s", arg_msg); > > > >>> + free(arg_msg); > > > >>> } > > > >>> > > > >>> typedef std::map <ConstString, Log::Callbacks> CallbackMap; > > > >>> > > > >>> Added: lldb/trunk/source/Core/NullLog.cpp > > > >>> URL: > > > >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/NullLog.cpp?rev=236174&view=auto > > > >>> ============================================================================== > > > >>> --- lldb/trunk/source/Core/NullLog.cpp (added) > > > >>> +++ lldb/trunk/source/Core/NullLog.cpp Wed Apr 29 17:55:28 2015 > > > >>> @@ -0,0 +1,74 @@ > > > >>> +//===-- NullLog.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/NullLog.h" > > > >>> + > > > >>> +using namespace lldb_private; > > > >>> + > > > >>> +NullLog::NullLog() > > > >>> +{ > > > >>> +} > > > >>> +NullLog::~NullLog() > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::PutCString(const char *cstr) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::Printf(const char *format, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::VAPrintf(const char *format, va_list args) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::LogIf(uint32_t mask, const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::Debug(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::DebugVerbose(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::Error(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::FatalError(int err, const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::Verbose(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::Warning(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> + > > > >>> +void > > > >>> +NullLog::WarningVerbose(const char *fmt, ...) > > > >>> +{ > > > >>> +} > > > >>> > > > >>> > > > >>> _______________________________________________ > > > >>> lldb-commits mailing list > > > >>> [email protected] > > > >>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > >> > > > >> > > > >> _______________________________________________ > > > >> lldb-commits mailing list > > > >> [email protected] > > > >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > > > > > > > > > _______________________________________________ > > > > lldb-commits mailing list > > > > [email protected] > > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > > > > > Thanks, > > > > - Enrico > > > > 📩 egranata@.com ☎️ 27683 > > > > > > > > > > _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
