Travis Vitek wrote:
> 
[...]
> Martin Sebor wrote:
>> A few questions/comments on the implementation:
>> http://fisheye6.cenqua.com/browse/stdcxx/trunk/tests/src/braceexp.cpp?r=628817
>>
>> The _rw_is_lower() and _rw_is_upper() helper functions should either
>> be replaced with the C equivalents or rewritten to handle EBCDIC. If
>> the latter, I suggest making them inline.
>>
> The C equivalents are documented to be locale dependent. I don't believe
> that we want the input string to be localized, so I've written my own
> routines that are equivalent the the C functions for the classic locale.

Don't forget about EBCDIC where letters of the alphabet don't have
consecutive values. See src/ctype.cpp.

> 
> I'm not sure why you suggest making them inline. Is there some other reason
> other than the obvious, and possibly unnecessary, optimization benefit?

Just optimization.

> 
> 
> Martin Sebor wrote:
>> In _rw_brace_graph, the member functions don't need to be privatized
>> (with the _C_ prefix). Also, unless the class is intended to be copy
>> constructible and assignable I suggest to explicitly declare the two
>> members private.
>>
> Actually, none of this needs to be privatized because it is already private
> to the implementation. Yay.
> 
> 
> Martin Sebor wrote:
>> Finally, please check your braces :) E.g., these:
>> http://fisheye6.cenqua.com/browse/stdcxx/trunk/tests/src/braceexp.cpp?r=628817#l29
>> http://fisheye6.cenqua.com/browse/stdcxx/trunk/tests/src/braceexp.cpp?r=628817#l54
>> http://fisheye6.cenqua.com/browse/stdcxx/trunk/tests/src/braceexp.cpp?r=628817#l87
>>
> Have I mentioned that our brace convention is wonky? ;-)

Hey, at least it's consistent (for the most part ;-)

Martin