Re: [boost] [utility] Proposal to extract some components from Boost.Log

On Wed, Aug 28, 2013 at 2:01 PM, Antony Polukhin wrote:

2013/8/28 Andrey Semashev

...

On Wed, Aug 28, 2013 at 12:56 PM, Antony Polukhin

...

wrote:

...

2013/8/28 Andrey Semashev

...

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 string_literal; typedef boost::basic_string_ref > 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.