Currently, the following assertion: char const test_string[] = "a test string"; assert( test_string == boost::as_literal( test_string ) ); fails while the following one: assert( test_string == boost::as_literal( test_string ).advance_end( 1 ) ); passes because the left-hand side parameter of operator== is implicitly converted to a boost::iterator_range<> object which _includes_ the trailing zero (because it is treated as any other array) while the boost::iterator_range<> returned by boost::as_literal<>() _excludes_ the trailing zero... I 'know' that there is no 'happy' solution for this because the status quo is 'consistent' across all types but it, on the other hand, creates problems (or unexpected results), like the one outlined above, when strings, char pointers and literals are used...For this reason I would prefer more the other 'less than happy' solution that would treat strings in a special way (by never including the trailing zero)... Another problem is that the conversion, implict or otherwise, from a char container/range whose iterators are not plain char pointers (e.g. std::strings in 'secure STL' implementations) does not work (does not compile). My last post in the [program_options] thread offers a 'fix'. Yet another problem is that the as_literal<>() template function does not use the fact that it was passed an array to implicitly/directly deduce the length but it still calls std::strlen(). The make_range() function it calls for that: make_range( T* const r, bool ) { return iterator_range<T*>( r, r + length(r) ); } is also duplicated by: inline const char* str_end( const char* s, const char* ) { return s + strlen( s ); } ... Yes, some will object that the 'desired' direct deduction of length would be broken for 'typical' local buffers that are not completely filled (if there is no clear way of distinguishing them from literals): void foo() { char buf[ 256 ] = "a small string"; as_literal( buf ).size() == std::strlen( buf ); // in status quo as_literal( buf ).size() == _countof( buf ) - 1; // as proposed } but that could then also be said for the current implict conversion from such a "char buf[ 256 ]" into a boost::iterator_range<char const*>... I'd vote that this be handled with a runtime assertion that if a plain char array is passed it must be 'completely filled' (i.e. strlen( buf ) == _countof( buf ) ) otherwise the range must be explicitly constructed (iterator_range<char const *>( buf, buf + actual length )... Additionaly the above mentioned implementations of make_range() and str_end() add two extra instructions (substraction and addition) because they use strlen() that works with 'indexes'/size which then have to be converted to pointers... Additionaly#2 the boost::range_detail::is_char_ptr() function in as_literal.hpp is a runtime function when it seems it could be a metafunction... -- "What Huxley teaches is that in the age of advanced technology, spiritual devastation is more likely to come from an enemy with a smiling face than from one whose countenance exudes suspicion and hate." Neil Postman
Domagoj Saric wrote:
Currently, the following assertion: char const test_string[] = "a test string"; assert( test_string == boost::as_literal( test_string ) ); fails while the following one: assert( test_string == boost::as_literal( test_string ).advance_end( 1 ) ); passes because the left-hand side parameter of operator== is implicitly converted to a boost::iterator_range<> object which _includes_ the trailing zero (because it is treated as any other array) while the boost::iterator_range<> returned by boost::as_literal<>() _excludes_ the trailing zero...
I 'know' that there is no 'happy' solution for this because the status quo is 'consistent' across all types but it, on the other hand, creates problems (or unexpected results)
I personally do not find that unexpected. An array of size N is a range of its N elements.
like the one outlined above, when strings, char pointers and literals are used...For this reason I would prefer more the other 'less than happy' solution that would treat strings in a special way (by never including the trailing zero)...
I believe this used to be the default, but was changed. Also, consider char arrays that have zeros in the middle on purpose, and/or that are not strings.
Another problem is that the conversion, implict or otherwise
AFAIK there is actually no conversion going on. It just calls template<typename Range> bool operator==(Range&, const iterator_range<const char*>&);
from a char container/range whose iterators are not plain char pointers (e.g. std::strings in 'secure STL' implementations) does not work (does not compile). My last post in the [program_options] thread offers a 'fix'.
as_literal is not meant to be used on anything else but char (or wide chars) arrays.
Yet another problem is that the as_literal<>() template function does not use the fact that it was passed an array to implicitly/directly deduce the length but it still calls std::strlen().
Indeed, but that's the expected and documented behaviour.
Additionaly the above mentioned implementations of make_range() and str_end() add two extra instructions (substraction and addition) because they use strlen() that works with 'indexes'/size which then have to be converted to pointers...
I suspect that strlen on a literal is not really a performance concern. Nevertheless an adaptor to simply remove the last element of the array range might be useful indeed.
"Mathias Gaunard" <mathias.gaunard@ens-lyon.org> wrote in message news:hn2s9r$fpu$1@dough.gmane.org...
I personally do not find that unexpected. An array of size N is a range of its N elements.
But in the mentioned case it is not IMO apriori obvious we are dealing with arrays...Consider a use case like this: bool is_a_recognized_string( char const * const string_to_test ) { boost::iterator_range<char const *> const test_string( boost::as_literal( string_to_test ); if ( ( test_string == "valid_string1" ) || ( test_string == "valid_string2" ) || ( test_string == "valid_string3" ) || ( test_string == "valid_string4" ) ) return true; else return false; } will always return false (unless the advance_end( 1 ) call is added)...to me this was rather unexpected...
Also, consider char arrays that have zeros in the middle on purpose, and/or that are not strings.
I am/was...that's why I said that there is no 'happy' solution (or so it seems to me)... ...because if as_literal() would be broken if it would use the size of the array then, it seems to me, the implicit iterator_range<> constructor is also broken currently/as it is now...As ilustrated in the above example, or why would the following two results be different: typedef iterator_range<char const *> string_chunk_t; char buf[ 256 ] = "a small string"; string_chunk_t string_chunk1( buf ); string_chunk_t string_chunk2( as_literal( buf ) ); Doesn't the very name as_literal() imply that its argument will be treated as 'literal' (a 'full null terminated static array of chars') instead of just as a plain pointer..?
Another problem is that the conversion, implict or otherwise
AFAIK there is actually no conversion going on. It just calls template<typename Range> bool operator==(Range&, const iterator_range<const char*>&);
Sorry, wasn't clear enough, I meant conversion to an iterator_range<> (not using as_literal())...
from a char container/range whose iterators are not plain char pointers (e.g. std::strings in 'secure STL' implementations) does not work (does not compile). My last post in the [program_options] thread offers a 'fix'.
as_literal is not meant to be used on anything else but char (or wide chars) arrays.
As above...wasn't refering to as_literal(). Actually this issue is not specific to strings but for all containers that do not have plain pointers for iterators... For example this does not work in Dinkumware STL implementations: std::vector<unsigned char> storage; boost::iterator_range<unsigned char const *> storage_chunk( storage );
Yet another problem is that the as_literal<>() template function does not use the fact that it was passed an array to implicitly/directly deduce the length but it still calls std::strlen().
Indeed, but that's the expected and documented behaviour.
True for the documented part, sorry, didn't look look at the documentation for quite some time... But still, consider, the argument from above as to what does the name as_literal() imply...
Additionaly the above mentioned implementations of make_range() and str_end() add two extra instructions (substraction and addition) because they use strlen() that works with 'indexes'/size which then have to be converted to pointers...
I suspect that strlen on a literal is not really a performance concern. Nevertheless an adaptor to simply remove the last element of the array range might be useful indeed.
The strlen is not a problem in the sense of a string length computation because that is what is needed but the usage of C std::strlen() that works with indices/integer length..: range works with pointers so std::strlen() requires the recalculation of the pointer value...maybe 'insignificant' but still avoidable (with a find( '\0') like operation that returns a pointer/iterator instead of an index)... -- "What Huxley teaches is that in the age of advanced technology, spiritual devastation is more likely to come from an enemy with a smiling face than from one whose countenance exudes suspicion and hate." Neil Postman
Domagoj Saric wrote:
But in the mentioned case it is not IMO apriori obvious we are dealing with arrays...
Well, the type of the variable in question was an array. If you don't know the types of your variables, you're going to have trouble understanding what overload get selected by generic libraries.
Consider a use case like this:
bool is_a_recognized_string( char const * const string_to_test ) { boost::iterator_range<char const *> const test_string( boost::as_literal( string_to_test );
Now it's a pointer to a const char, a vastly different thing. Does that code even compile? I would have expected as_literal to not work then.
For example this does not work in Dinkumware STL implementations: std::vector<unsigned char> storage; boost::iterator_range<unsigned char const *> storage_chunk( storage );
It's not supposed to work; if you do that, your code is broken.
"Mathias Gaunard" <mathias.gaunard@ens-lyon.org> wrote in message news:hn3e4e$nv4$1@dough.gmane.org...
Domagoj Saric wrote:
But in the mentioned case it is not IMO apriori obvious we are dealing with arrays...
Well, the type of the variable in question was an array. If you don't know the types of your variables, you're going to have trouble understanding what overload get selected by generic libraries.
I was thinking of "apriori obviousness"/"intuitivness"/readability of code...the idea of 'self documenting code' or what you think it does before reading the documentation....and IMO it is more intuitive to expect that the following assumption holds: assert( iterator_range<char const *>( "my test string" ) == as_literal( "my test string" ) ); ...but, more importantly, it is also useful... Likewise, that a function called "as_literal" does _not_ treat its argument as a (string) literal (but as a plain pointer) is not quite obvious to me...If I'm not mistaken, section 2.13.4 of the standard pretty clearly says that a string literal is a null terminatated array of const chars...
Consider a use case like this:
bool is_a_recognized_string( char const * const string_to_test ) { boost::iterator_range<char const *> const test_string( boost::as_literal( string_to_test );
Now it's a pointer to a const char, a vastly different thing. Does that code even compile?
Yes...(the above snippet actually does not because it misses a parenthesis...just an omission on my part while I was simplifying a real world piece of code)...
I would have expected as_literal to not work then.
The documentation specifies its return as "[s,s + std::char_traits<X>::length(s)) if s is a Char* or an array of Char [boost::begin(x),boost::end(x)) otherwise" so it was obviously designed to work that way...
For example this does not work in Dinkumware STL implementations: std::vector<unsigned char> storage; boost::iterator_range<unsigned char const *> storage_chunk( storage );
It's not supposed to work; if you do that, your code is broken.
Why is it not supposed to work? If the container in question has random access iterators and is contiguous in memory (as std::vector is, explicitly, and also std::string, implicitly) why would such code be broken? In any case, what would be your proposal/fix for the following 'solution' http://lists.boost.org/Archives/boost/2010/03/162762.php? IMO the example presented there fits perfectly into the 'story' of Boost.Range introduction/motivation..: if all I need as input is a string then a boost::iterator_range<tchar const *> would be the perfect abstraction (ABI stable and efficient extraction of the strlen information) if it provides automatic conversion from a char const *, a string literal, a std::string, a std::vector<char>, an MFC CString, an ATL CString or just about any other form of a contiguous string of characters in memory... -- "What Huxley teaches is that in the age of advanced technology, spiritual devastation is more likely to come from an enemy with a smiling face than from one whose countenance exudes suspicion and hate." Neil Postman
participants (2)
-
Domagoj Saric -
Mathias Gaunard