[utility] Proposal to extract some components from Boost.Log
Hi, I'd like to extract some general-purpose components from Boost.Log to Boost.Utility so that they could be used in other libraries and user's code that is not related to logging. The components are mostly standalone already and are not tied to Boost.Log specifics. 1. BOOST_EXPLICIT_OPERATOR_BOOL() macro for defining explicit operator bool() for a class. For C++03 compilers the macro implements safe bool idiom or just a regular operator bool(), if the compiler doesn't handle safe bool well. The generated operator is implemented in terms of operator!(), which should be defined for the class: template< typename T > class my_ptr { T* m_p; public: BOOST_EXPLICIT_OPERATOR_BOOL() bool operator! () const { return !m_p; } }; Various different implementations of explicit/safe bool operator are present in different libraries. It would be good to unify them. Implementation: boost/log/utility/explicit_operator_bool.hpp 2. basic_string_literal< CharT, CharTraitsT > class template. String literal is similar to string_ref, but it only allows to be constructed from a string literal. Obviously, it doesn't allow to modify the literal and it doesn't allocate memory for the literal. It supports clear() though, which makes the literal object to refer to an empty literal. Otherwise it provides interface similar to std::string. Docs: < http://www.boost.org/doc/libs/release/libs/log/doc/html/log/detailed/utiliti...
Implementation: boost/log/utility/string_literal.hpp 3. type_info_wrapper class. The class wraps a reference to std::type_info and provides equality and ordering operators. The wrapper implements value semantics (i.e. it can be constructed and copied), which allows it to be stored in containers, including as keys. Docs: < http://www.boost.org/doc/libs/release/libs/log/doc/html/log/detailed/utiliti...
Implementation: boost/log/utility/type_info_wrapper.hpp 4. empty_deleter class. The class can be used as a deleter (e.g. for shared_ptr) that does nothing. It can be useful in some cases, when you need a shared_ptr but you know you will delete the referred object by other means. For example, with Boost.Log this deleter can be used to create shared_ptr< std::ostream > to std::cout. Another use case is to create a weak_ptr to an object to be able to test whether the object has already been deleted. Implementation: boost/log/utility/empty_deleter.hpp 5. BOOST_LOG_UNIQUE_IDENTIFIER_NAME(prefix) macro. This macro expands in a unique (within the current TU) token that starts with prefix. This can be useful for declaring unique variable and type names within other macros. Not that this macro has much value but it could be useful for other libraries that define macros. Implementation: boost/log/utility/unique_identifier_name.hpp
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
Hi,
I'd like to extract some general-purpose components from Boost.Log to Boost.Utility
<...>
1. BOOST_EXPLICIT_OPERATOR_BOOL() macro for defining explicit operator bool() for a class.
<...> I like that!
2. basic_string_literal< CharT, CharTraitsT > class template.
<...> Could you please describe the difference between this class and string_ref in more details? Why can't you use string_ref instead of basic_string_literal? 3. type_info_wrapper class.
<...> Nice thing, but I'd rather wait for review of more functional https://github.com/apolukhin/type_index library and include that library into utility.
4. empty_deleter class.
<...> May be it is better to move that into Boost.SmartPtr library?
5. BOOST_LOG_UNIQUE_IDENTIFIER_NAME(prefix) macro.
<...> Boost.PP is a better place for that macro. I'd like to have it, but I'm not sure that `prefix ## __LINE__` is enough for some cases. -- Best regards, Antony Polukhin
On Wed, Aug 28, 2013 at 12:56 PM, Antony Polukhin <antoshkka@gmail.com>wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
2. basic_string_literal< CharT, CharTraitsT > class template.
<...>
Could you please describe the difference between this class and string_ref in more details? Why can't you use string_ref instead of basic_string_literal?
I use basic_string_literal when I need to be sure the string is a literal. This gives the guarantee that the string won't disappear or change all of a sudden, which allows to safely store string_literals in containers, etc. In Boost.Log string_literals are used to maintain the list of scopes (which have scope names and source file names as literals), and the mentioned guarantee allows to avoid copying the strings in most cases. 3. type_info_wrapper class.
<...>
Nice thing, but I'd rather wait for review of more functional https://github.com/apolukhin/type_index library and include that library into utility.
Interesting, although I'd like type_index and template_index to be separated. My type_info_wrapper is close to your type_index, so if I had to port to your library I wouldn't want any additional unneeded code to be included. One question: you're using strcmp to compare type_info names on some compilers to solve problems with cross-library comparisons. If symbol visibility is configured properly, cross-library comparisons should work properly, shouldn't they? Or are you aware of platforms that do not support cross-library comparisons at all? I'm asking because the list of such faulty platforms in your code looks overly long. E.g. on Linux type_info can be compared just fine, as long as the type info symbols have default visibility, and the comparison is faster than strcmp. Also, when the review is planned?
4. empty_deleter class.
<...>
May be it is better to move that into Boost.SmartPtr library?
I don't know, may be. boost::checked_deleter is not in Boost.SmartPtr and is not only used with pointers. My usage of empty_deleter was with pointers so far, but who knows?..
5. BOOST_LOG_UNIQUE_IDENTIFIER_NAME(prefix) macro.
<...>
Boost.PP is a better place for that macro. I'd like to have it, but I'm not sure that `prefix ## __LINE__` is enough for some cases.
Maybe. The macro worked just fine for my cases so far, but I'm not insisting.
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
On Wed, Aug 28, 2013 at 12:56 PM, Antony Polukhin <antoshkka@gmail.com
wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
2. basic_string_literal< CharT, CharTraitsT > class template.
<...>
Could you please describe the difference between this class and string_ref in more details? Why can't you use string_ref instead of basic_string_literal?
I use basic_string_literal when I need to be sure the string is a literal. This gives the guarantee that the string won't disappear or change all of a sudden, which allows to safely store string_literals in containers, etc. In Boost.Log string_literals are used to maintain the list of scopes (which have scope names and source file names as literals), and the mentioned guarantee allows to avoid copying the strings in most cases.
May be I'm missing something, but this could be resolved in a following way. Instead of a separate basic_string_literal container: v.push_back(string_literal("Meow")); it is possible to make it a function: boost::string_ref string_literal(const char* v) { return boost::string_ref(v); } This won't require code change (no need to change `v.push_back(string_literal("Meow"));`) and uses already reviewed and accepted string_ref. You may also provide your own typedef for string_literal: template <class Char> struct only_literals_trait: char_traits<Char>{} typedef boost::basic_string_ref<char, only_literals_trait<char> > string_literal; typedef boost::basic_string_ref<wchar_t, only_literals_trait<wchar_t> > wstring_literal; In theory (I did not check) this wont allow mixing string_refs to literals and string_refs to std::strings. 3. type_info_wrapper class.
<...>
Nice thing, but I'd rather wait for review of more functional https://github.com/apolukhin/type_index library and include that library into utility.
Interesting, although I'd like type_index and template_index to be separated. My type_info_wrapper is close to your type_index, so if I had to port to your library I wouldn't want any additional unneeded code to be included.
Good point! I'll separate them.
One question: you're using strcmp to compare type_info names on some compilers to solve problems with cross-library comparisons. If symbol visibility is configured properly, cross-library comparisons should work properly, shouldn't they? Or are you aware of platforms that do not support cross-library comparisons at all? I'm asking because the list of such faulty platforms in your code looks overly long. E.g. on Linux type_info can be compared just fine, as long as the type info symbols have default visibility, and the comparison is faster than strcmp.
GCC's Standard library and some other libraries had an issue. They were assuming that if two dynamic libraries have same strings, then dynamic linker will load those symbols to the same address. That's why they were comparing types by pointers to type names instead of comparing type names. Unfortunately assumption is not right for some platforms (ARM) and linkers.
Also, when the review is planned?
I'm in search for a review manager. I was planning to search for a review manager more enthusiastically in the second half of September. If you're willing to become the review manager, I'll be pleased. -- Best regards, Antony Polukhin
On Wed, Aug 28, 2013 at 2:01 PM, Antony Polukhin <antoshkka@gmail.com>wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
On Wed, Aug 28, 2013 at 12:56 PM, Antony Polukhin <antoshkka@gmail.com
wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
2. basic_string_literal< CharT, CharTraitsT > class template.
<...>
Could you please describe the difference between this class and string_ref in more details? Why can't you use string_ref instead of basic_string_literal?
I use basic_string_literal when I need to be sure the string is a literal. This gives the guarantee that the string won't disappear or change all of a sudden, which allows to safely store string_literals in containers, etc. In Boost.Log string_literals are used to maintain the list of scopes (which have scope names and source file names as literals), and the mentioned guarantee allows to avoid copying the strings in most cases.
May be I'm missing something, but this could be resolved in a following way. Instead of a separate basic_string_literal container:
v.push_back(string_literal("Meow"));
it is possible to make it a function:
boost::string_ref string_literal(const char* v) { return boost::string_ref(v); }
The thing is that this function can be called with non-literal strings: char temp[10] = { 'a', 'b', 'c' }; v.push_back(string_literal(temp)); std::string str = "abc"; v.push_back(string_literal(str.c_str())); This also doesn't protect from user manually creating a string_ref from a non-literal string. v.push_back(boost::string_ref(temp)); My string_literal tries to prohibit these use cases. v.push_back(boost::string_literal(temp)); // compile error: cannot construct string_literal v.push_back(boost::string_literal("abc")); // ok, deduces length at compile-time It's not 100% bullet-proof, but it does protect in most cases and for the rest it is at least obvious that a string literal is expected and nothing else. This won't require code change (no need to change
`v.push_back(string_literal("Meow"));`) and uses already reviewed and accepted string_ref. You may also provide your own typedef for string_literal:
template <class Char> struct only_literals_trait: char_traits<Char>{}
typedef boost::basic_string_ref<char, only_literals_trait<char> > string_literal; typedef boost::basic_string_ref<wchar_t, only_literals_trait<wchar_t> > wstring_literal;
In theory (I did not check) this wont allow mixing string_refs to literals and string_refs to std::strings.
A special char_trait type does change the type of string_ref, but it doesn't actually prohibit creating a string_ref from a non-literal string.
One question: you're using strcmp to compare type_info names on some compilers to solve problems with cross-library comparisons. If symbol visibility is configured properly, cross-library comparisons should work properly, shouldn't they? Or are you aware of platforms that do not support cross-library comparisons at all? I'm asking because the list of such faulty platforms in your code looks overly long. E.g. on Linux type_info can be compared just fine, as long as the type info symbols have default visibility, and the comparison is faster than strcmp.
GCC's Standard library and some other libraries had an issue. They were assuming that if two dynamic libraries have same strings, then dynamic linker will load those symbols to the same address. That's why they were comparing types by pointers to type names instead of comparing type names. Unfortunately assumption is not right for some platforms (ARM) and linkers.
Can these particular configurations be detected at compile time? E.g. by testing for ARM target? Comparing symbol addresses works on x86 Linux, and I'd like to use it. Alternatively, maybe you could enable the strcmp workaround only on user's request?
Also, when the review is planned?
I'm in search for a review manager. I was planning to search for a review manager more enthusiastically in the second half of September. If you're willing to become the review manager, I'll be pleased.
I'm not sure I'll have enough time for managing the review. I may have more time later in the autumn/winter though. Let's see if anyone else volunteers.
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
A special char_trait type does change the type of string_ref, but it doesn't actually prohibit creating a string_ref from a non-literal string.
Hmmm, you are right. And how about this: #include <boost/utility/string_ref.hpp> #include <string> template <class Char, class Traits> class basic_string_literal: public boost::basic_string_ref<Char, Traits> { typedef boost::basic_string_ref<Char, Traits> base_t; public: template <std::size_t N> explicit basic_string_literal(const Char (&array)[N]) BOOST_NOEXCEPT : base_t(array, N) {} }; typedef basic_string_literal<char, std::char_traits<char> > string_literal; int main() { string_literal sl1("Hello word"); // string_literal sl2(std::string("Hello word")); // compilation error // string_literal sl3(std::string("Hello word").c_str()); // compilation error sl1.clear(); }
One question: you're using strcmp to compare type_info names on some compilers to solve problems with cross-library comparisons. If symbol visibility is configured properly, cross-library comparisons should work properly, shouldn't they? Or are you aware of platforms that do not support cross-library comparisons at all? I'm asking because the list of such faulty platforms in your code looks overly long. E.g. on Linux type_info can be compared just fine, as long as the type info symbols have default visibility, and the comparison is faster than strcmp.
GCC's Standard library and some other libraries had an issue. They were assuming that if two dynamic libraries have same strings, then dynamic linker will load those symbols to the same address. That's why they were comparing types by pointers to type names instead of comparing type names. Unfortunately assumption is not right for some platforms (ARM) and linkers.
Can these particular configurations be detected at compile time? E.g. by testing for ARM target? Comparing symbol addresses works on x86 Linux, and I'd like to use it.
Oh yes, I'll need to check GCC once more. As I remember that issue was fixed in GCC 4.5 and it is safe to use default std::type_info comparison. -- Best regards, Antony Polukhin
On Wed, Aug 28, 2013 at 4:16 PM, Antony Polukhin <antoshkka@gmail.com>wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
A special char_trait type does change the type of string_ref, but it doesn't actually prohibit creating a string_ref from a non-literal string.
Hmmm, you are right. And how about this:
#include <boost/utility/string_ref.hpp> #include <string>
template <class Char, class Traits> class basic_string_literal: public boost::basic_string_ref<Char, Traits> { typedef boost::basic_string_ref<Char, Traits> base_t; public: template <std::size_t N> explicit basic_string_literal(const Char (&array)[N]) BOOST_NOEXCEPT : base_t(array, N) {} };
typedef basic_string_literal<char, std::char_traits<char> > string_literal;
Basically, that's what basic_string_literal is, except that the protection is a little more elaborate. The notable differences are that, unlike basic_string_ref, basic_string_literal doesn't allow to remove_prefix/suffix or substr and is always zero-terminated.
Antony Polukhin <antoshkka@gmail.com> writes:
GCC's Standard library and some other libraries had an issue. They were assuming that if two dynamic libraries have same strings, then dynamic linker will load those symbols to the same address. That's why they were comparing types by pointers to type names instead of comparing type names. Unfortunately assumption is not right for some platforms (ARM) and linkers.
Oh yes, I'll need to check GCC once more. As I remember that issue was fixed in GCC 4.5 and it is safe to use default std::type_info comparison.
If the following would reproduce the problem, then it has been fixed on ARM by now: [petr@trimslice-f18-v7hl tmp]$ cat lib.cc #include <typeinfo> class xx {}; std::type_info const &get_ti () { return typeid (xx); } [petr@trimslice-f18-v7hl tmp]$ g++ lib.cc -Wall -fpic -shared -o lib.so [petr@trimslice-f18-v7hl tmp]$ cat app.cc #include <cassert> #include <typeinfo> class xx {}; std::type_info const &get_ti (); int main (void) { std::type_info const &ti1 = get_ti (); std::type_info const &ti2 = typeid (xx); assert (&ti1 == &ti2); assert (ti1.name () == ti2.name ()); } [petr@trimslice-f18-v7hl tmp]$ g++ app.cc lib.so -Wall [petr@trimslice-f18-v7hl tmp]$ LD_LIBRARY_PATH=. ./a.out [petr@trimslice-f18-v7hl tmp]$ echo $? 0 [petr@trimslice-f18-v7hl tmp]$ rpm -q gcc glibc gcc-4.7.2-8.fc18.armv7hl However that might also be influenced by the dynamic linker, as that's where vague linkage is resolved. I don't know about this particular problem, so I don't know what it was caused by. Thanks, PM
On Wed, Aug 28, 2013 at 4:16 PM, Antony Polukhin <antoshkka@gmail.com>wrote:
2013/8/28 Andrey Semashev <andrey.semashev@gmail.com>
One question: you're using strcmp to compare type_info names on some compilers to solve problems with cross-library comparisons. If symbol visibility is configured properly, cross-library comparisons should work properly, shouldn't they? Or are you aware of platforms that do not support cross-library comparisons at all? I'm asking because the list of such faulty platforms in your code looks overly long. E.g. on Linux type_info can be compared just fine, as long as the type info symbols have default visibility, and the comparison is faster than strcmp.
GCC's Standard library and some other libraries had an issue. They were assuming that if two dynamic libraries have same strings, then dynamic linker will load those symbols to the same address. That's why they were comparing types by pointers to type names instead of comparing type names. Unfortunately assumption is not right for some platforms (ARM) and linkers.
Can these particular configurations be detected at compile time? E.g. by testing for ARM target? Comparing symbol addresses works on x86 Linux, and I'd like to use it.
Oh yes, I'll need to check GCC once more. As I remember that issue was fixed in GCC 4.5 and it is safe to use default std::type_info comparison.
I have a suggestion about type_info_wrapper. How about I extract it from Boost.Log and then you add no-RTTI and strcmp workarounds to it? This way it will become equivalent to your type_index.
Date: Wed, 28 Aug 2013 12:14:51 +0400 From: andrey.semashev@gmail.com
I'd like to extract some general-purpose components from Boost.Log to Boost.Utility so that they could be used in other libraries and user's code that is not related to logging. The components are mostly standalone already and are not tied to Boost.Log specifics.
1. BOOST_EXPLICIT_OPERATOR_BOOL() macro for defining explicit operator bool() for a class. For C++03 compilers the macro implements safe bool idiom or just a regular operator bool(), if the compiler doesn't handle safe bool well. The generated operator is implemented in terms of operator!(), which should be defined for the class:
template< typename T> class my_ptr { T* m_p;
public: BOOST_EXPLICIT_OPERATOR_BOOL() bool operator! () const { return !m_p; } };
Various different implementations of explicit/safe bool operator are present in different libraries. It would be good to unify them.
Implementation: boost/log/utility/explicit_operator_bool.hpp [TRUNCATE]
I could add this to my touch-ups of Boost.Rational, but I see two minor problems using it: a. It isn't marked "constexpr." I can add that myself before using "BOOST_EXPLICIT_OPERATOR_BOOL" in "boost/rational.hpp," but what happens if someone corrects it in "explicit_operator_bool.hpp"? Will the (temporary) double definition cause an error? b. Its #included Boost.Log-specific configuration header file flags an error if RTTI isn't active. And it's unconditional, which would make Boost.Rational unusable for small environments that skip RTTI. But I don't know if Boost.Rational is already unsuitable for such systems, since it uses exceptions and such. Maybe I'll just make a custom solution for now.... Daryle W.
On Sunday 01 September 2013 03:27:06 Daryle Walker wrote:
Date: Wed, 28 Aug 2013 12:14:51 +0400 From: andrey.semashev@gmail.com
1. BOOST_EXPLICIT_OPERATOR_BOOL() macro for defining explicit operator bool() for a class.
I could add this to my touch-ups of Boost.Rational, but I see two minor problems using it:
a. It isn't marked "constexpr." I can add that myself before using "BOOST_EXPLICIT_OPERATOR_BOOL" in "boost/rational.hpp," but what happens if someone corrects it in "explicit_operator_bool.hpp"? Will the (temporary) double definition cause an error?
Well, since there isn't a constexpr_if or something, I don't see a way to add it to BOOST_EXPLICIT_OPERATOR_BOOL. I could add BOOST_EXPLICIT_OPERATOR_BOOL_PP(pre, post), which would add pre() and post() before and after the operator signature. For example: BOOST_EXPLICIT_OPERATOR_BOOL_PP(\ BOOST_PP_IDENTITY(BOOST_CONSTEXPR), BOOST_PP_EMPTY()) would expand to something like constexpr explicit operator bool() /* nothing */ { ... } Although I'm not sure the expansion will be correct if BOOST_CONSTEXPR expands to nothing. PP gurus? A simpler alternative would be just BOOST_CONSTEXPR_EXPLICIT_OPERATOR_BOOL macro.
b. Its #included Boost.Log-specific configuration header file flags an error if RTTI isn't active. And it's unconditional, which would make Boost.Rational unusable for small environments that skip RTTI. But I don't know if Boost.Rational is already unsuitable for such systems, since it uses exceptions and such.
It won't include these headers after extraction and it won't require RTTI.
participants (4)
-
Andrey Semashev -
Antony Polukhin -
Daryle Walker -
Petr Machata