From 729eef98f9074e2237c21c775044ecbbe361f668 Mon Sep 17 00:00:00 2001 From: "scc%netscape.com" Date: Sat, 13 May 2000 20:17:35 +0000 Subject: [PATCH] getting ready to turn this stuff on, changes related to that: empty strings specified with null pointer, a couple of other fixes. git-svn-id: svn://10.0.0.236/trunk@69573 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/string/public/nsAReadableString.h | 74 +++++++++++++------ mozilla/string/public/nsAWritableString.h | 53 +++++++++---- mozilla/string/public/nsCharTraits.h | 40 ++++++++-- mozilla/xpcom/ds/nsAReadableString.h | 74 +++++++++++++------ mozilla/xpcom/ds/nsAWritableString.h | 53 +++++++++---- mozilla/xpcom/ds/nsCharTraits.h | 40 ++++++++-- .../xpcom/string/public/nsAReadableString.h | 74 +++++++++++++------ .../xpcom/string/public/nsAWritableString.h | 53 +++++++++---- mozilla/xpcom/string/public/nsCharTraits.h | 40 ++++++++-- 9 files changed, 366 insertions(+), 135 deletions(-) diff --git a/mozilla/string/public/nsAReadableString.h b/mozilla/string/public/nsAReadableString.h index 007950a6b45..a1f4da5f996 100644 --- a/mozilla/string/public/nsAReadableString.h +++ b/mozilla/string/public/nsAReadableString.h @@ -263,6 +263,24 @@ class basic_nsAReadableString PRUint32 CountChar( CharT ) const; + + /* + |Left|, |Mid|, and |Right| are annoying signatures that seem better almost + any _other_ way than they are now. Consider these alternatives + + aWritable = aReadable.Left(17); // ...a member function that returns a |Substring| + aWritable = Left(aReadable, 17); // ...a global function that returns a |Substring| + Left(aReadable, 17, aWritable); // ...a global function that does the assignment + + as opposed to the current signature + + aReadable.Left(aWritable, 17); // ...a member function that does the assignment + + or maybe just stamping them out in favor of |Substring|, they are just duplicate functionality + + aWritable = Substring(aReadable, 0, 17); + */ + PRUint32 Left( basic_nsAWritableString&, PRUint32 ) const; PRUint32 Mid( basic_nsAWritableString&, PRUint32, PRUint32 ) const; PRUint32 Right( basic_nsAWritableString&, PRUint32 ) const; @@ -337,6 +355,7 @@ class basic_nsAReadableString virtual const void* Implementation() const; virtual const CharT* GetReadableFragment( nsReadableFragment&, nsFragmentRequest, PRUint32 = 0 ) const = 0; virtual PRBool Promises( const basic_nsAReadableString& aString ) const { return &aString == this; } +// virtual PRBool PromisesExactly( const basic_nsAReadableString& aString ) const { return false; } private: // NOT TO BE IMPLEMENTED @@ -531,27 +550,24 @@ basic_nsAReadableString::CountChar( CharT c ) const } - /* - Note: |Left()|, |Mid()|, and |Right()| could be modified to notice when they degenerate into copying the - entire string, and call |Assign()| instead. This would be a win when the underlying implementation of - both strings could do buffer sharing. This is _definitely_ something that should be measured before - being implemented. - */ - template PRUint32 -basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, 0, aLengthToCopy); + // If we're just assigning our entire self, give |aResult| the opportunity to share + if ( aStartPos == 0 && aLengthToCopy >= Length() ) + aResult = *this; + else + aResult = Substring(*this, aStartPos, aLengthToCopy); + return aResult.Length(); } template PRUint32 -basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, aStartPos, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, 0, aLengthToCopy); } template @@ -560,8 +576,7 @@ basic_nsAReadableString::Right( basic_nsAWritableString& aResult, { PRUint32 myLength = Length(); aLengthToCopy = NS_MIN(myLength, aLengthToCopy); - aResult = Substring(*this, myLength-aLengthToCopy, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, myLength-aLengthToCopy, aLengthToCopy); } @@ -612,7 +627,7 @@ class basic_nsLiteralString // Note: _not_ explicit basic_nsLiteralString( const CharT* aLiteral ) : mStart(aLiteral), - mEnd(mStart + nsCharTraits::length(mStart)) + mEnd(mStart ? (mStart + nsCharTraits::length(mStart)) : mStart) { // nothing else to do here } @@ -624,7 +639,10 @@ class basic_nsLiteralString // This is an annoying hack. Callers should be fixed to use the other // constructor if they don't really know the length. if ( aLength == PRUint32(-1) ) - mEnd = mStart + nsCharTraits::length(mStart); + { + NS_WARNING("Tell scc: Caller constructing a string doesn't know the real length. Please use the other constructor."); + mEnd = mStart + nsCharTraits::length(mStart); + } } virtual PRUint32 Length() const; @@ -818,6 +836,7 @@ class nsPromiseConcatenation virtual PRUint32 Length() const; virtual PRBool Promises( const basic_nsAReadableString& ) const; +// virtual PRBool PromisesExactly( const basic_nsAReadableString& ) const; nsPromiseConcatenation operator+( const basic_nsAReadableString& rhs ) const; @@ -847,6 +866,15 @@ nsPromiseConcatenation::Promises( const basic_nsAReadableString& a return mStrings[0]->Promises(aString) || mStrings[1]->Promises(aString); } +#if 0 +PRBool +nsPromiseConcatenation::PromisesExactly( const basic_nsAReadableString& aString ) const + { + // Not really like this, test for the empty string, etc + return mStrings[0] == &aString && !mStrings[1] || !mStrings[0] && mStrings[1] == &aString; + } +#endif + template const CharT* nsPromiseConcatenation::GetReadableFragment( nsReadableFragment& aFragment, nsFragmentRequest aRequest, PRUint32 aPosition ) const @@ -992,7 +1020,7 @@ nsPromiseSubstring::GetReadableFragment( nsReadableFragment& aFrag const CharT* position_ptr = mString.GetReadableFragment(aFragment, aRequest, aPosition); - // if there's more physical data in the returned fragment than I logically have left + // if there's more physical data in the returned fragment than I logically have left... size_t logical_size_forward = mLength - aPosition; if ( aFragment.mEnd - position_ptr > logical_size_forward ) aFragment.mEnd = position_ptr + logical_size_forward; @@ -1027,7 +1055,7 @@ copy_string( InputIterator first, InputIterator last, OutputIterator result ) while ( first != last ) { - PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_size(first, last))); + PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_distance(first, last))); NS_ASSERTION(count_copied > 0, "|copy_string| will never terminate"); first += count_copied; } @@ -1060,11 +1088,6 @@ template nsPromiseSubstring Substring( const basic_nsAReadableString& aString, PRUint32 aStartPos, PRUint32 aSubstringLength ) { -#if 0 - // signatures don't work, but consider this twist to help in assignments - if ( aSubstringLength == aString.Length() && aStartPos == 0 ) - return aString; -#endif return nsPromiseSubstring(aString, aStartPos, aSubstringLength); } @@ -1145,6 +1168,11 @@ Compare( const CharT* lhs, const basic_nsAReadableString& rhs ) to implement the virtual functions of readables. */ + /* + I probably need to add |operator+()| for appropriate |CharT| and |CharT*| comparisons, since + automatic conversion to a literal string will not happen. + */ + template inline nsPromiseConcatenation diff --git a/mozilla/string/public/nsAWritableString.h b/mozilla/string/public/nsAWritableString.h index fd22d3a8c50..c1ecf414948 100644 --- a/mozilla/string/public/nsAWritableString.h +++ b/mozilla/string/public/nsAWritableString.h @@ -270,39 +270,39 @@ class basic_nsAWritableString // - // |operator=()|, |Assign()| + // |Assign()|, |operator=()| // - basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } - basic_nsAWritableString& operator=( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator=( CharT aChar ) { do_AssignFromElement(aChar); return *this; } - void Assign( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); } void Assign( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AssignFromPromise(aReadable) : do_AssignFromReadable(aReadable); } // void Assign( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AssignFromIterators(aStart, aEnd); } - void Assign( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); } + void Assign( const CharT* aPtr ) { aPtr ? do_AssignFromElementPtr(aPtr) : SetLength(0); } void Assign( const CharT* aPtr, PRUint32 aLength ) { do_AssignFromElementPtrLength(aPtr, aLength); } void Assign( CharT aChar ) { do_AssignFromElement(aChar); } + basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const CharT* aPtr ) { Assign(aPtr); return *this; } + basic_nsAWritableString& operator=( CharT aChar ) { Assign(aChar); return *this; } + // - // |operator+=()|, |Append()| + // |Append()|, |operator+=()| // - basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } - basic_nsAWritableString& operator+=( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator+=( CharT aChar ) { do_AppendFromElement(aChar); return *this; } - void Append( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); } void Append( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AppendFromPromise(aReadable) : do_AppendFromReadable(aReadable); } // void Append( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AppendFromIterators(aStart, aEnd); } - void Append( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); } + void Append( const CharT* aPtr ) { if (aPtr) do_AppendFromElementPtr(aPtr); } void Append( const CharT* aPtr, PRUint32 aLength ) { do_AppendFromElementPtrLength(aPtr, aLength); } void Append( CharT aChar ) { do_AppendFromElement(aChar); } + basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const CharT* aPtr ) { Append(aPtr); return *this; } + basic_nsAWritableString& operator+=( CharT aChar ) { Append(aChar); return *this; } + // @@ -313,7 +313,7 @@ class basic_nsAWritableString void Insert( const basic_nsAReadableString& aReadable, PRUint32 atPosition ) { do_InsertFromReadable(aReadable, atPosition); } void Insert( const nsPromiseReadable& aReadable, PRUint32 atPosition ) { aReadable.Promises(*this) ? InsertFromPromise(aReadable, atPosition) : do_InsertFromReadable(aReadable, atPosition); } // void Insert( const nsReadingIterator& aStart, const nsReadingIterator& aEnd, PRUint32 atPosition ) { do_InsertFromIterators(aStart, aEnd, atPosition); } - void Insert( const CharT* aPtr, PRUint32 atPosition ) { do_InsertFromElementPtr(aPtr, atPosition); } + void Insert( const CharT* aPtr, PRUint32 atPosition ) { if (aPtr) do_InsertFromElementPtr(aPtr, atPosition); } void Insert( const CharT* aPtr, PRUint32 atPosition, PRUint32 aLength ) { do_InsertFromElementPtrLength(aPtr, atPosition, aLength); } void Insert( CharT aChar, PRUint32 atPosition ) { do_InsertFromElement(aChar, atPosition); } @@ -433,14 +433,37 @@ basic_nsAWritableString::do_AssignFromReadable( const basic_nsAReadableSt template void basic_nsAWritableString::AssignFromPromise( const nsPromiseReadable& aReadable ) + /* + ...this function is only called when a promise that somehow references |this| is assigned _into_ |this|. + E.g., + + ... writable& w ... + ... readable& r ... + + w = r + w; + + In this example, you can see that unless the characters promised by |w| in |r+w| are resolved before + anything starts getting copied into |w|, there will be trouble. They will be overritten by the contents + of |r| before being retrieved to be appended. + + We could have a really tricky solution where we tell the promise to resolve _just_ the data promised + by |this|, but this should be a rare case, since clients with more local knowledge will know that, e.g., + in the case above, |Insert| could have special behavior with significantly better performance. Since + it's a rare case anyway, we should just do the simplest thing that could possibly work, resolve the + entire promise. If we measure and this turns out to show up on performance radar, we then have the + option to fix either the callers or this mechanism. + */ { PRUint32 length = aReadable.Length(); if ( CharT* buffer = new CharT[length] ) { + // Note: not exception safe. We need something to manage temporary buffers like this + copy_string(aReadable.BeginReading(), aReadable.EndReading(), buffer); do_AssignFromElementPtrLength(buffer, length); delete buffer; } + // else assert? } #if 0 diff --git a/mozilla/string/public/nsCharTraits.h b/mozilla/string/public/nsCharTraits.h index 6d7389b5428..f734180ff53 100644 --- a/mozilla/string/public/nsCharTraits.h +++ b/mozilla/string/public/nsCharTraits.h @@ -463,20 +463,28 @@ struct nsCharTraits #endif - template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const InputIterator& iter ) + distance( const InputIterator& first, const InputIterator& last ) + { + // ... + } +#endif + + static + PRUint32 + readable_distance( const InputIterator& iter ) { return iter.size_forward(); } static PRUint32 - readable_size( const InputIterator& first, const InputIterator& last ) + readable_distance( const InputIterator& first, const InputIterator& last ) { return PRUint32(SameFragment(first, last) ? last.operator->()-first.operator->() : first.size_forward()); } @@ -494,9 +502,18 @@ struct nsCharSourceTraits template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( CharT* s ) + distance( CharT* first, CharT* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( CharT* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -504,7 +521,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( CharT* first, CharT* last ) + readable_distance( CharT* first, CharT* last ) { return PRUint32(last-first); } @@ -522,9 +539,18 @@ struct nsCharSourceTraits NS_SPECIALIZE_TEMPLATE struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const char* s ) + distance( const char* first, const char* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( const char* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -532,7 +558,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( const char* first, const char* last ) + readable_distance( const char* first, const char* last ) { return PRUint32(last-first); } diff --git a/mozilla/xpcom/ds/nsAReadableString.h b/mozilla/xpcom/ds/nsAReadableString.h index 007950a6b45..a1f4da5f996 100644 --- a/mozilla/xpcom/ds/nsAReadableString.h +++ b/mozilla/xpcom/ds/nsAReadableString.h @@ -263,6 +263,24 @@ class basic_nsAReadableString PRUint32 CountChar( CharT ) const; + + /* + |Left|, |Mid|, and |Right| are annoying signatures that seem better almost + any _other_ way than they are now. Consider these alternatives + + aWritable = aReadable.Left(17); // ...a member function that returns a |Substring| + aWritable = Left(aReadable, 17); // ...a global function that returns a |Substring| + Left(aReadable, 17, aWritable); // ...a global function that does the assignment + + as opposed to the current signature + + aReadable.Left(aWritable, 17); // ...a member function that does the assignment + + or maybe just stamping them out in favor of |Substring|, they are just duplicate functionality + + aWritable = Substring(aReadable, 0, 17); + */ + PRUint32 Left( basic_nsAWritableString&, PRUint32 ) const; PRUint32 Mid( basic_nsAWritableString&, PRUint32, PRUint32 ) const; PRUint32 Right( basic_nsAWritableString&, PRUint32 ) const; @@ -337,6 +355,7 @@ class basic_nsAReadableString virtual const void* Implementation() const; virtual const CharT* GetReadableFragment( nsReadableFragment&, nsFragmentRequest, PRUint32 = 0 ) const = 0; virtual PRBool Promises( const basic_nsAReadableString& aString ) const { return &aString == this; } +// virtual PRBool PromisesExactly( const basic_nsAReadableString& aString ) const { return false; } private: // NOT TO BE IMPLEMENTED @@ -531,27 +550,24 @@ basic_nsAReadableString::CountChar( CharT c ) const } - /* - Note: |Left()|, |Mid()|, and |Right()| could be modified to notice when they degenerate into copying the - entire string, and call |Assign()| instead. This would be a win when the underlying implementation of - both strings could do buffer sharing. This is _definitely_ something that should be measured before - being implemented. - */ - template PRUint32 -basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, 0, aLengthToCopy); + // If we're just assigning our entire self, give |aResult| the opportunity to share + if ( aStartPos == 0 && aLengthToCopy >= Length() ) + aResult = *this; + else + aResult = Substring(*this, aStartPos, aLengthToCopy); + return aResult.Length(); } template PRUint32 -basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, aStartPos, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, 0, aLengthToCopy); } template @@ -560,8 +576,7 @@ basic_nsAReadableString::Right( basic_nsAWritableString& aResult, { PRUint32 myLength = Length(); aLengthToCopy = NS_MIN(myLength, aLengthToCopy); - aResult = Substring(*this, myLength-aLengthToCopy, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, myLength-aLengthToCopy, aLengthToCopy); } @@ -612,7 +627,7 @@ class basic_nsLiteralString // Note: _not_ explicit basic_nsLiteralString( const CharT* aLiteral ) : mStart(aLiteral), - mEnd(mStart + nsCharTraits::length(mStart)) + mEnd(mStart ? (mStart + nsCharTraits::length(mStart)) : mStart) { // nothing else to do here } @@ -624,7 +639,10 @@ class basic_nsLiteralString // This is an annoying hack. Callers should be fixed to use the other // constructor if they don't really know the length. if ( aLength == PRUint32(-1) ) - mEnd = mStart + nsCharTraits::length(mStart); + { + NS_WARNING("Tell scc: Caller constructing a string doesn't know the real length. Please use the other constructor."); + mEnd = mStart + nsCharTraits::length(mStart); + } } virtual PRUint32 Length() const; @@ -818,6 +836,7 @@ class nsPromiseConcatenation virtual PRUint32 Length() const; virtual PRBool Promises( const basic_nsAReadableString& ) const; +// virtual PRBool PromisesExactly( const basic_nsAReadableString& ) const; nsPromiseConcatenation operator+( const basic_nsAReadableString& rhs ) const; @@ -847,6 +866,15 @@ nsPromiseConcatenation::Promises( const basic_nsAReadableString& a return mStrings[0]->Promises(aString) || mStrings[1]->Promises(aString); } +#if 0 +PRBool +nsPromiseConcatenation::PromisesExactly( const basic_nsAReadableString& aString ) const + { + // Not really like this, test for the empty string, etc + return mStrings[0] == &aString && !mStrings[1] || !mStrings[0] && mStrings[1] == &aString; + } +#endif + template const CharT* nsPromiseConcatenation::GetReadableFragment( nsReadableFragment& aFragment, nsFragmentRequest aRequest, PRUint32 aPosition ) const @@ -992,7 +1020,7 @@ nsPromiseSubstring::GetReadableFragment( nsReadableFragment& aFrag const CharT* position_ptr = mString.GetReadableFragment(aFragment, aRequest, aPosition); - // if there's more physical data in the returned fragment than I logically have left + // if there's more physical data in the returned fragment than I logically have left... size_t logical_size_forward = mLength - aPosition; if ( aFragment.mEnd - position_ptr > logical_size_forward ) aFragment.mEnd = position_ptr + logical_size_forward; @@ -1027,7 +1055,7 @@ copy_string( InputIterator first, InputIterator last, OutputIterator result ) while ( first != last ) { - PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_size(first, last))); + PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_distance(first, last))); NS_ASSERTION(count_copied > 0, "|copy_string| will never terminate"); first += count_copied; } @@ -1060,11 +1088,6 @@ template nsPromiseSubstring Substring( const basic_nsAReadableString& aString, PRUint32 aStartPos, PRUint32 aSubstringLength ) { -#if 0 - // signatures don't work, but consider this twist to help in assignments - if ( aSubstringLength == aString.Length() && aStartPos == 0 ) - return aString; -#endif return nsPromiseSubstring(aString, aStartPos, aSubstringLength); } @@ -1145,6 +1168,11 @@ Compare( const CharT* lhs, const basic_nsAReadableString& rhs ) to implement the virtual functions of readables. */ + /* + I probably need to add |operator+()| for appropriate |CharT| and |CharT*| comparisons, since + automatic conversion to a literal string will not happen. + */ + template inline nsPromiseConcatenation diff --git a/mozilla/xpcom/ds/nsAWritableString.h b/mozilla/xpcom/ds/nsAWritableString.h index fd22d3a8c50..c1ecf414948 100644 --- a/mozilla/xpcom/ds/nsAWritableString.h +++ b/mozilla/xpcom/ds/nsAWritableString.h @@ -270,39 +270,39 @@ class basic_nsAWritableString // - // |operator=()|, |Assign()| + // |Assign()|, |operator=()| // - basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } - basic_nsAWritableString& operator=( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator=( CharT aChar ) { do_AssignFromElement(aChar); return *this; } - void Assign( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); } void Assign( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AssignFromPromise(aReadable) : do_AssignFromReadable(aReadable); } // void Assign( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AssignFromIterators(aStart, aEnd); } - void Assign( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); } + void Assign( const CharT* aPtr ) { aPtr ? do_AssignFromElementPtr(aPtr) : SetLength(0); } void Assign( const CharT* aPtr, PRUint32 aLength ) { do_AssignFromElementPtrLength(aPtr, aLength); } void Assign( CharT aChar ) { do_AssignFromElement(aChar); } + basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const CharT* aPtr ) { Assign(aPtr); return *this; } + basic_nsAWritableString& operator=( CharT aChar ) { Assign(aChar); return *this; } + // - // |operator+=()|, |Append()| + // |Append()|, |operator+=()| // - basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } - basic_nsAWritableString& operator+=( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator+=( CharT aChar ) { do_AppendFromElement(aChar); return *this; } - void Append( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); } void Append( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AppendFromPromise(aReadable) : do_AppendFromReadable(aReadable); } // void Append( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AppendFromIterators(aStart, aEnd); } - void Append( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); } + void Append( const CharT* aPtr ) { if (aPtr) do_AppendFromElementPtr(aPtr); } void Append( const CharT* aPtr, PRUint32 aLength ) { do_AppendFromElementPtrLength(aPtr, aLength); } void Append( CharT aChar ) { do_AppendFromElement(aChar); } + basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const CharT* aPtr ) { Append(aPtr); return *this; } + basic_nsAWritableString& operator+=( CharT aChar ) { Append(aChar); return *this; } + // @@ -313,7 +313,7 @@ class basic_nsAWritableString void Insert( const basic_nsAReadableString& aReadable, PRUint32 atPosition ) { do_InsertFromReadable(aReadable, atPosition); } void Insert( const nsPromiseReadable& aReadable, PRUint32 atPosition ) { aReadable.Promises(*this) ? InsertFromPromise(aReadable, atPosition) : do_InsertFromReadable(aReadable, atPosition); } // void Insert( const nsReadingIterator& aStart, const nsReadingIterator& aEnd, PRUint32 atPosition ) { do_InsertFromIterators(aStart, aEnd, atPosition); } - void Insert( const CharT* aPtr, PRUint32 atPosition ) { do_InsertFromElementPtr(aPtr, atPosition); } + void Insert( const CharT* aPtr, PRUint32 atPosition ) { if (aPtr) do_InsertFromElementPtr(aPtr, atPosition); } void Insert( const CharT* aPtr, PRUint32 atPosition, PRUint32 aLength ) { do_InsertFromElementPtrLength(aPtr, atPosition, aLength); } void Insert( CharT aChar, PRUint32 atPosition ) { do_InsertFromElement(aChar, atPosition); } @@ -433,14 +433,37 @@ basic_nsAWritableString::do_AssignFromReadable( const basic_nsAReadableSt template void basic_nsAWritableString::AssignFromPromise( const nsPromiseReadable& aReadable ) + /* + ...this function is only called when a promise that somehow references |this| is assigned _into_ |this|. + E.g., + + ... writable& w ... + ... readable& r ... + + w = r + w; + + In this example, you can see that unless the characters promised by |w| in |r+w| are resolved before + anything starts getting copied into |w|, there will be trouble. They will be overritten by the contents + of |r| before being retrieved to be appended. + + We could have a really tricky solution where we tell the promise to resolve _just_ the data promised + by |this|, but this should be a rare case, since clients with more local knowledge will know that, e.g., + in the case above, |Insert| could have special behavior with significantly better performance. Since + it's a rare case anyway, we should just do the simplest thing that could possibly work, resolve the + entire promise. If we measure and this turns out to show up on performance radar, we then have the + option to fix either the callers or this mechanism. + */ { PRUint32 length = aReadable.Length(); if ( CharT* buffer = new CharT[length] ) { + // Note: not exception safe. We need something to manage temporary buffers like this + copy_string(aReadable.BeginReading(), aReadable.EndReading(), buffer); do_AssignFromElementPtrLength(buffer, length); delete buffer; } + // else assert? } #if 0 diff --git a/mozilla/xpcom/ds/nsCharTraits.h b/mozilla/xpcom/ds/nsCharTraits.h index 6d7389b5428..f734180ff53 100644 --- a/mozilla/xpcom/ds/nsCharTraits.h +++ b/mozilla/xpcom/ds/nsCharTraits.h @@ -463,20 +463,28 @@ struct nsCharTraits #endif - template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const InputIterator& iter ) + distance( const InputIterator& first, const InputIterator& last ) + { + // ... + } +#endif + + static + PRUint32 + readable_distance( const InputIterator& iter ) { return iter.size_forward(); } static PRUint32 - readable_size( const InputIterator& first, const InputIterator& last ) + readable_distance( const InputIterator& first, const InputIterator& last ) { return PRUint32(SameFragment(first, last) ? last.operator->()-first.operator->() : first.size_forward()); } @@ -494,9 +502,18 @@ struct nsCharSourceTraits template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( CharT* s ) + distance( CharT* first, CharT* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( CharT* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -504,7 +521,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( CharT* first, CharT* last ) + readable_distance( CharT* first, CharT* last ) { return PRUint32(last-first); } @@ -522,9 +539,18 @@ struct nsCharSourceTraits NS_SPECIALIZE_TEMPLATE struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const char* s ) + distance( const char* first, const char* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( const char* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -532,7 +558,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( const char* first, const char* last ) + readable_distance( const char* first, const char* last ) { return PRUint32(last-first); } diff --git a/mozilla/xpcom/string/public/nsAReadableString.h b/mozilla/xpcom/string/public/nsAReadableString.h index 007950a6b45..a1f4da5f996 100644 --- a/mozilla/xpcom/string/public/nsAReadableString.h +++ b/mozilla/xpcom/string/public/nsAReadableString.h @@ -263,6 +263,24 @@ class basic_nsAReadableString PRUint32 CountChar( CharT ) const; + + /* + |Left|, |Mid|, and |Right| are annoying signatures that seem better almost + any _other_ way than they are now. Consider these alternatives + + aWritable = aReadable.Left(17); // ...a member function that returns a |Substring| + aWritable = Left(aReadable, 17); // ...a global function that returns a |Substring| + Left(aReadable, 17, aWritable); // ...a global function that does the assignment + + as opposed to the current signature + + aReadable.Left(aWritable, 17); // ...a member function that does the assignment + + or maybe just stamping them out in favor of |Substring|, they are just duplicate functionality + + aWritable = Substring(aReadable, 0, 17); + */ + PRUint32 Left( basic_nsAWritableString&, PRUint32 ) const; PRUint32 Mid( basic_nsAWritableString&, PRUint32, PRUint32 ) const; PRUint32 Right( basic_nsAWritableString&, PRUint32 ) const; @@ -337,6 +355,7 @@ class basic_nsAReadableString virtual const void* Implementation() const; virtual const CharT* GetReadableFragment( nsReadableFragment&, nsFragmentRequest, PRUint32 = 0 ) const = 0; virtual PRBool Promises( const basic_nsAReadableString& aString ) const { return &aString == this; } +// virtual PRBool PromisesExactly( const basic_nsAReadableString& aString ) const { return false; } private: // NOT TO BE IMPLEMENTED @@ -531,27 +550,24 @@ basic_nsAReadableString::CountChar( CharT c ) const } - /* - Note: |Left()|, |Mid()|, and |Right()| could be modified to notice when they degenerate into copying the - entire string, and call |Assign()| instead. This would be a win when the underlying implementation of - both strings could do buffer sharing. This is _definitely_ something that should be measured before - being implemented. - */ - template PRUint32 -basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, 0, aLengthToCopy); + // If we're just assigning our entire self, give |aResult| the opportunity to share + if ( aStartPos == 0 && aLengthToCopy >= Length() ) + aResult = *this; + else + aResult = Substring(*this, aStartPos, aLengthToCopy); + return aResult.Length(); } template PRUint32 -basic_nsAReadableString::Mid( basic_nsAWritableString& aResult, PRUint32 aStartPos, PRUint32 aLengthToCopy ) const +basic_nsAReadableString::Left( basic_nsAWritableString& aResult, PRUint32 aLengthToCopy ) const { - aResult = Substring(*this, aStartPos, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, 0, aLengthToCopy); } template @@ -560,8 +576,7 @@ basic_nsAReadableString::Right( basic_nsAWritableString& aResult, { PRUint32 myLength = Length(); aLengthToCopy = NS_MIN(myLength, aLengthToCopy); - aResult = Substring(*this, myLength-aLengthToCopy, aLengthToCopy); - return aResult.Length(); + return Mid(aResult, myLength-aLengthToCopy, aLengthToCopy); } @@ -612,7 +627,7 @@ class basic_nsLiteralString // Note: _not_ explicit basic_nsLiteralString( const CharT* aLiteral ) : mStart(aLiteral), - mEnd(mStart + nsCharTraits::length(mStart)) + mEnd(mStart ? (mStart + nsCharTraits::length(mStart)) : mStart) { // nothing else to do here } @@ -624,7 +639,10 @@ class basic_nsLiteralString // This is an annoying hack. Callers should be fixed to use the other // constructor if they don't really know the length. if ( aLength == PRUint32(-1) ) - mEnd = mStart + nsCharTraits::length(mStart); + { + NS_WARNING("Tell scc: Caller constructing a string doesn't know the real length. Please use the other constructor."); + mEnd = mStart + nsCharTraits::length(mStart); + } } virtual PRUint32 Length() const; @@ -818,6 +836,7 @@ class nsPromiseConcatenation virtual PRUint32 Length() const; virtual PRBool Promises( const basic_nsAReadableString& ) const; +// virtual PRBool PromisesExactly( const basic_nsAReadableString& ) const; nsPromiseConcatenation operator+( const basic_nsAReadableString& rhs ) const; @@ -847,6 +866,15 @@ nsPromiseConcatenation::Promises( const basic_nsAReadableString& a return mStrings[0]->Promises(aString) || mStrings[1]->Promises(aString); } +#if 0 +PRBool +nsPromiseConcatenation::PromisesExactly( const basic_nsAReadableString& aString ) const + { + // Not really like this, test for the empty string, etc + return mStrings[0] == &aString && !mStrings[1] || !mStrings[0] && mStrings[1] == &aString; + } +#endif + template const CharT* nsPromiseConcatenation::GetReadableFragment( nsReadableFragment& aFragment, nsFragmentRequest aRequest, PRUint32 aPosition ) const @@ -992,7 +1020,7 @@ nsPromiseSubstring::GetReadableFragment( nsReadableFragment& aFrag const CharT* position_ptr = mString.GetReadableFragment(aFragment, aRequest, aPosition); - // if there's more physical data in the returned fragment than I logically have left + // if there's more physical data in the returned fragment than I logically have left... size_t logical_size_forward = mLength - aPosition; if ( aFragment.mEnd - position_ptr > logical_size_forward ) aFragment.mEnd = position_ptr + logical_size_forward; @@ -1027,7 +1055,7 @@ copy_string( InputIterator first, InputIterator last, OutputIterator result ) while ( first != last ) { - PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_size(first, last))); + PRInt32 count_copied = PRInt32(sink_traits::write(result, source_traits::read(first), source_traits::readable_distance(first, last))); NS_ASSERTION(count_copied > 0, "|copy_string| will never terminate"); first += count_copied; } @@ -1060,11 +1088,6 @@ template nsPromiseSubstring Substring( const basic_nsAReadableString& aString, PRUint32 aStartPos, PRUint32 aSubstringLength ) { -#if 0 - // signatures don't work, but consider this twist to help in assignments - if ( aSubstringLength == aString.Length() && aStartPos == 0 ) - return aString; -#endif return nsPromiseSubstring(aString, aStartPos, aSubstringLength); } @@ -1145,6 +1168,11 @@ Compare( const CharT* lhs, const basic_nsAReadableString& rhs ) to implement the virtual functions of readables. */ + /* + I probably need to add |operator+()| for appropriate |CharT| and |CharT*| comparisons, since + automatic conversion to a literal string will not happen. + */ + template inline nsPromiseConcatenation diff --git a/mozilla/xpcom/string/public/nsAWritableString.h b/mozilla/xpcom/string/public/nsAWritableString.h index fd22d3a8c50..c1ecf414948 100644 --- a/mozilla/xpcom/string/public/nsAWritableString.h +++ b/mozilla/xpcom/string/public/nsAWritableString.h @@ -270,39 +270,39 @@ class basic_nsAWritableString // - // |operator=()|, |Assign()| + // |Assign()|, |operator=()| // - basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } - basic_nsAWritableString& operator=( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator=( CharT aChar ) { do_AssignFromElement(aChar); return *this; } - void Assign( const basic_nsAReadableString& aReadable ) { do_AssignFromReadable(aReadable); } void Assign( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AssignFromPromise(aReadable) : do_AssignFromReadable(aReadable); } // void Assign( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AssignFromIterators(aStart, aEnd); } - void Assign( const CharT* aPtr ) { do_AssignFromElementPtr(aPtr); } + void Assign( const CharT* aPtr ) { aPtr ? do_AssignFromElementPtr(aPtr) : SetLength(0); } void Assign( const CharT* aPtr, PRUint32 aLength ) { do_AssignFromElementPtrLength(aPtr, aLength); } void Assign( CharT aChar ) { do_AssignFromElement(aChar); } + basic_nsAWritableString& operator=( const basic_nsAReadableString& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const nsPromiseReadable& aReadable ) { Assign(aReadable); return *this; } + basic_nsAWritableString& operator=( const CharT* aPtr ) { Assign(aPtr); return *this; } + basic_nsAWritableString& operator=( CharT aChar ) { Assign(aChar); return *this; } + // - // |operator+=()|, |Append()| + // |Append()|, |operator+=()| // - basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); return *this; } - basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } - basic_nsAWritableString& operator+=( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); return *this; } - basic_nsAWritableString& operator+=( CharT aChar ) { do_AppendFromElement(aChar); return *this; } - void Append( const basic_nsAReadableString& aReadable ) { do_AppendFromReadable(aReadable); } void Append( const nsPromiseReadable& aReadable ) { aReadable.Promises(*this) ? AppendFromPromise(aReadable) : do_AppendFromReadable(aReadable); } // void Append( const nsReadingIterator& aStart, const nsReadingIterator& aEnd ) { do_AppendFromIterators(aStart, aEnd); } - void Append( const CharT* aPtr ) { do_AppendFromElementPtr(aPtr); } + void Append( const CharT* aPtr ) { if (aPtr) do_AppendFromElementPtr(aPtr); } void Append( const CharT* aPtr, PRUint32 aLength ) { do_AppendFromElementPtrLength(aPtr, aLength); } void Append( CharT aChar ) { do_AppendFromElement(aChar); } + basic_nsAWritableString& operator+=( const basic_nsAReadableString& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const nsPromiseReadable& aReadable ) { Append(aReadable); return *this; } + basic_nsAWritableString& operator+=( const CharT* aPtr ) { Append(aPtr); return *this; } + basic_nsAWritableString& operator+=( CharT aChar ) { Append(aChar); return *this; } + // @@ -313,7 +313,7 @@ class basic_nsAWritableString void Insert( const basic_nsAReadableString& aReadable, PRUint32 atPosition ) { do_InsertFromReadable(aReadable, atPosition); } void Insert( const nsPromiseReadable& aReadable, PRUint32 atPosition ) { aReadable.Promises(*this) ? InsertFromPromise(aReadable, atPosition) : do_InsertFromReadable(aReadable, atPosition); } // void Insert( const nsReadingIterator& aStart, const nsReadingIterator& aEnd, PRUint32 atPosition ) { do_InsertFromIterators(aStart, aEnd, atPosition); } - void Insert( const CharT* aPtr, PRUint32 atPosition ) { do_InsertFromElementPtr(aPtr, atPosition); } + void Insert( const CharT* aPtr, PRUint32 atPosition ) { if (aPtr) do_InsertFromElementPtr(aPtr, atPosition); } void Insert( const CharT* aPtr, PRUint32 atPosition, PRUint32 aLength ) { do_InsertFromElementPtrLength(aPtr, atPosition, aLength); } void Insert( CharT aChar, PRUint32 atPosition ) { do_InsertFromElement(aChar, atPosition); } @@ -433,14 +433,37 @@ basic_nsAWritableString::do_AssignFromReadable( const basic_nsAReadableSt template void basic_nsAWritableString::AssignFromPromise( const nsPromiseReadable& aReadable ) + /* + ...this function is only called when a promise that somehow references |this| is assigned _into_ |this|. + E.g., + + ... writable& w ... + ... readable& r ... + + w = r + w; + + In this example, you can see that unless the characters promised by |w| in |r+w| are resolved before + anything starts getting copied into |w|, there will be trouble. They will be overritten by the contents + of |r| before being retrieved to be appended. + + We could have a really tricky solution where we tell the promise to resolve _just_ the data promised + by |this|, but this should be a rare case, since clients with more local knowledge will know that, e.g., + in the case above, |Insert| could have special behavior with significantly better performance. Since + it's a rare case anyway, we should just do the simplest thing that could possibly work, resolve the + entire promise. If we measure and this turns out to show up on performance radar, we then have the + option to fix either the callers or this mechanism. + */ { PRUint32 length = aReadable.Length(); if ( CharT* buffer = new CharT[length] ) { + // Note: not exception safe. We need something to manage temporary buffers like this + copy_string(aReadable.BeginReading(), aReadable.EndReading(), buffer); do_AssignFromElementPtrLength(buffer, length); delete buffer; } + // else assert? } #if 0 diff --git a/mozilla/xpcom/string/public/nsCharTraits.h b/mozilla/xpcom/string/public/nsCharTraits.h index 6d7389b5428..f734180ff53 100644 --- a/mozilla/xpcom/string/public/nsCharTraits.h +++ b/mozilla/xpcom/string/public/nsCharTraits.h @@ -463,20 +463,28 @@ struct nsCharTraits #endif - template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const InputIterator& iter ) + distance( const InputIterator& first, const InputIterator& last ) + { + // ... + } +#endif + + static + PRUint32 + readable_distance( const InputIterator& iter ) { return iter.size_forward(); } static PRUint32 - readable_size( const InputIterator& first, const InputIterator& last ) + readable_distance( const InputIterator& first, const InputIterator& last ) { return PRUint32(SameFragment(first, last) ? last.operator->()-first.operator->() : first.size_forward()); } @@ -494,9 +502,18 @@ struct nsCharSourceTraits template struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( CharT* s ) + distance( CharT* first, CharT* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( CharT* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -504,7 +521,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( CharT* first, CharT* last ) + readable_distance( CharT* first, CharT* last ) { return PRUint32(last-first); } @@ -522,9 +539,18 @@ struct nsCharSourceTraits NS_SPECIALIZE_TEMPLATE struct nsCharSourceTraits { +#if 0 static PRUint32 - readable_size( const char* s ) + distance( const char* first, const char* last ) + { + return PRUint32(last-first); + } +#endif + + static + PRUint32 + readable_distance( const char* s ) { return PRUint32(nsCharTraits::length(s)); // return numeric_limits::max(); @@ -532,7 +558,7 @@ struct nsCharSourceTraits static PRUint32 - readable_size( const char* first, const char* last ) + readable_distance( const char* first, const char* last ) { return PRUint32(last-first); }