Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upstd::string std::string_view support #23
Comments
|
Hm... I think the answer depends on which version of NanoLog you intend to modify. It should be fairly simple to do in C++17, and a bit harder in Preprocessor NanoLog. I'll start with the C++17 answer. I suspect you'll want to be able to pass in the string_view object to the log function and have the interface resemble something like ``NANO_LOG(WARNING, "%s", stringView)''. In this case, you'll need to modify/add 2-3 functions to make this work.
I'll skip the preprocessor version of NanoLog for now. Let me know if you need additional help! |
|
Thanks, that makes sense. I had a look at the code and I think the sticking
point may be the printf attribute checking. An I correct in saying all
format string/args validation is handle by the compiler using the printf
__attribute__? If so does that mean a complete new implementation would
have to be done in order to add string_view?! That would be rather a lot of
work, especially given the lack of a starting point.
…On Thu, 13 Feb 2020, 6:31 am syang0, ***@***.***> wrote:
Hm... I think the answer depends on which version of NanoLog you intend to
modify. It should be fairly simple to do in C++17, and a bit harder in
Preprocessor NanoLog.
I'll start with the C++17 answer. I suspect you'll want to be able to pass
in the string_view object to the log function and have the interface
resemble something like ``NANO_LOG(WARNING, "%s", stringView)''. In this
case, you'll need to modify/add 2-3 functions to make this work.
1.
First, NanoLog needs to understand how to get the length of the
string. To do this you'll need to add another overloaded getArgSize()
<https://github.com/PlatformLab/NanoLog/blob/955fc375dc2a152584e4468f3284885fd12218d1/runtime/NanoLogCpp17.h#L600>
function to accept std::string_view objects. This is the only function that
will ever get the length of the string. Afterwards, the information is
stored internally.
2.
Next, NanoLog needs to know how to copy the string_view data into its
internal buffers. To do this, you'll have to add another template
specialization for the store_argument()
<https://github.com/PlatformLab/NanoLog/blob/955fc375dc2a152584e4468f3284885fd12218d1/runtime/NanoLogCpp17.h#L397>
function to specialize on std::string_view. Conceptually, this function
reads the string data from "T arg" and stores a length field plus the raw
character data into "char** storage." I think the simplest way to do this
one is probably to add a new store_argument function that specializes on
string_view's and then immediately invokes store_argument<const
char*>(storage, arg.data, paramType, stringSize). In other words, it's just
a wrapper that peels the string_view off and passes the char* pointer down
to the same logic. You'll probably need to add more of the
std::enable_if<std::is_same<...>> conditions to make sure the compiler
properly separates other specializations from std::string_view's.
3.
At this point, everything will probably work. But if not, you may also
need to add a specialization for the compressSingle
<https://github.com/PlatformLab/NanoLog/blob/955fc375dc2a152584e4468f3284885fd12218d1/runtime/NanoLogCpp17.h#L742>
function. This function copies the string from an input buffer to an output
buffer. However, I suspect the included function will just work.
I'll skip the preprocessor version of NanoLog for now. Let me know if you
need additional help!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#23?email_source=notifications&email_token=AL5LS3CFGAZB73UFTACKB4TRCSWH7A5CNFSM4KTY3PEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELTE7QI#issuecomment-585519041>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL5LS3AYRHJBHDDEERXKXDLRCSWH7ANCNFSM4KTY3PEA>
.
|
|
Ah! That is true, C++17 NanoLog relies on the GNU format attribute to check for errors. That is a complication I did not foresee. Hm... I think it is doable though. I already have a function that parses the format strings at compile-time to extract type information called getParamInfo(). I could imagine one using this function as a starting point and modifying where it checks for terminals (Line 203) to also do a type check. |
How much work would it be to add support for std::string and std::string_view and thus avoid the use of strlen() in the hotpath?
Currently using std::string_view is quite problematic as there's no guarantee that it's null terminated so you have to construct a std::string and then call c_str() on it before passing it into the logger. Considering the length is already encoded into the std::strig_view this is very wasteful.