Make many OUString functions take std::u16string_view parameters

...instead of having individual overloads for OUString, OUStringLiteral, and
literal char16_t const[N].  (The variants taking OUString are still needed for
!LIBO_INTERNAL_ONLY, though.  The variants taking ASCII-only literal char
const[N] are also left in place.)

This nicely reduces the number of needed overloads.  std::u16string_view allows
to pass as arguments:

* OUString
* OUStringLiteral
* OUStringChar  (with the necessary conversion added now)
* OUStringNumber
* u"..." char16_t string literals
* u"..."sv std::u16string_view literals
* std::u16string, plain char16_t*, and more

A notable exceptions is OUStringConcat, which now needs to be wrapped in
OUString(...), see the handful of places that needed to be adapted.

One caveat is the treatment of embedded NUL characters, as

  std::u16string_view(u"x\0y")

constructs a view of size 1, while only

  u"x\0y"sv

constructs a view of size 3 (which matches the old behavior of overloads for
literal char16_t const[N] via the ConstCharArrayDetector<>::TypeUtf16
machinery).  See the new checkEmbeddedNul in
sal/qa/rtl/strings/test_oustring_stringliterals.cxx.

The functions that have been changed are generally those that:

* already take a string of determined length, so that using std::u16string_view,
  which is always constructed with a determined length, is no pessimization
  (e.g., there are operator == overloads taking plain pointers, which do not
  need to determine the string length upfront);

* could not benefit from the fact that the passed-in argument is an OUString
  (e.g., the corresponding operator = overload can reuse the passed-in
  OUString's rtl_uString pData member);

* do not run into overload resolution ambiguity issues, like the comparison
  operators would do.

One inconsistency that showed up is that while the original

  replaceAll(OUString const &, OUString const &, sal_Int32 fromIndex = 0)

overload takes an optional third fromIndex argument, the existing replaceAll
overloads taking OUStringLiteral and literal char16_t const[N] arguments did
not.  Fixing that required a new (LIBO_INTERNAL_ONLY)
rtl_uString_newReplaceAllFromIndexUtf16LUtf16L (with test code in
sal/qa/rtl/strings/test_strings_replace.cxx).

Another issue was posed by test code in
sal/qa/rtl/strings/test_oustring_stringliterals.cxx that used the
RTL_STRING_UNITTEST-only OUString(Except*CharArrayDetector) ctors to verify that
certain function calls should not compile (and would compile under
RTL_STRING_UNITTEST by taking those Except*CharArrayDetector converted to
OUString as arguments).  Those problematic "should fail to compile" tests have
been converted into a new CompilerTest_sal_rtl_oustring.

Change-Id: Id72e8c4cc338258cadad00ddc6ea5b9da2e1f780
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102020
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This commit is contained in:
Stephan Bergmann 2020-09-03 21:04:01 +02:00
parent baa84b5b4d
commit 1b43cceaea
20 changed files with 338 additions and 748 deletions

View file

@ -794,7 +794,7 @@ void LCFormatNode::generateCode (const OFileWriter &of) const
if (n100s < 0)
incErrorInt( "Error: Time100SecSeparator not present in FormatCode formatindex=\"%d\".\n",
formatindex);
n100s = aCode.indexOf( pSep->getValue() + "00");
n100s = aCode.indexOf( OUString(pSep->getValue() + "00"));
if (n100s < 0)
incErrorInt( "Error: Time100SecSeparator+00 not present in FormatCode formatindex=\"%d\".\n",
formatindex);

View file

@ -94,6 +94,7 @@ struct SAL_WARN_UNUSED OUStringChar_ {
constexpr OUStringChar_(sal_Unicode theC): c(theC) {}
constexpr OUStringChar_(char theC): c(theC) { assert(c <= 0x7F); }
template<typename T> OUStringChar_(T &&) = delete;
constexpr operator std::u16string_view() const { return {&c, 1}; }
sal_Unicode const c;
};
using OUStringChar = OUStringChar_ const;

View file

@ -2001,6 +2001,41 @@ SAL_DLLPUBLIC void SAL_CALL rtl_uString_newReplaceAllUtf16LUtf16L(
sal_Int32 fromLength, sal_Unicode const * to, sal_Int32 toLength)
SAL_THROW_EXTERN_C();
#if defined LIBO_INTERNAL_ONLY
/** Create a new string by replacing all occurrences of a given substring with
another substring.
Replacing subsequent occurrences picks up only after a given replacement.
That is, replacing from "xa" to "xx" in "xaa" results in "xxa", not "xxx".
@param[in, out] newStr pointer to the new string; must not be null; must
point to null or a valid rtl_uString; upon return, points to the newly
allocated string or to null if there was either an out-of-memory condition
or the resulting number of UTF-16 code units would have been larger than
SAL_MAX_INT32
@param str pointer to the original string; must not be null
@param from pointer to the substring to be replaced; must not be null and
must point to memory of at least \p fromLength UTF-16 code units
@param fromLength the length of the \p from substring; must be non-negative
@param to pointer to the substring to be replaced; must not be null and
must point to memory of at least \p toLength UTF-16 code units
@param toLength the length of the \p to substring; must be non-negative
@param fromIndex the position in the string where we will begin searching
@since LibreOffice 7.1
*/
SAL_DLLPUBLIC void SAL_CALL rtl_uString_newReplaceAllFromIndexUtf16LUtf16L(
rtl_uString ** newStr, rtl_uString * str, sal_Unicode const * from,
sal_Int32 fromLength, sal_Unicode const * to, sal_Int32 toLengt, sal_Int32 fromIndex)
SAL_THROW_EXTERN_C();
#endif
/** Create a new string by converting all ASCII uppercase letters to lowercase
within another string.

File diff suppressed because it is too large Load diff

View file

@ -0,0 +1,16 @@
# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t; fill-column: 100 -*-
#
# This file is part of the LibreOffice project.
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
$(eval $(call gb_CompilerTest_CompilerTest,sal_rtl_oustring))
$(eval $(call gb_CompilerTest_add_exception_objects,sal_rtl_oustring, \
sal/qa/rtl/strings/compile-oustring \
))
# vim: set noet sw=4 ts=4:

View file

@ -30,6 +30,8 @@ $(eval $(call gb_Module_add_check_targets,sal,\
CppunitTest_sal_osl \
CppunitTest_sal_rtl \
CppunitTest_sal_types \
$(if $(COM_IS_CLANG),$(if $(COMPILER_EXTERNAL_TOOL)$(COMPILER_PLUGIN_TOOL),, \
CompilerTest_sal_rtl_oustring)) \
))
endif

View file

@ -0,0 +1,45 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
/*
* This file is part of the LibreOffice project.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
#include <sal/config.h>
#include <rtl/string.h>
#include <rtl/ustring.hxx>
// expected-note@rtl/ustring.hxx:* 1+ {{}}
void checkExtraIntArgument()
{
// This makes sure that using by mistake RTL_CONSTASCII_STRINGPARAM does not trigger a different
// overload, i.e. the second argument to match() in this case is the indexFrom argument,
// but with the macro it would contain the length of the string. Therefore
// match( RTL_CONSTASCII_STRINGPARAM( "bar" )) would be match( "bar", 3 ), which would be
// true when called for OUString( "foobar" ). But this should not happen because of the
// &foo[0] trick in the RTL_CONSTASCII_STRINGPARAM macro.
// expected-error@+1 {{}}
OUString("foobar").match(RTL_CONSTASCII_STRINGPARAM("bar"));
}
void checkNonconstChar()
{
// check that non-const char[] data do not trigger string literal overloads
char test[] = "test";
char bar[] = "bar";
const char consttest[] = "test";
const char constbar[] = "bar";
// expected-error@+1 {{}}
(void)OUString("footest").replaceAll(test, bar);
// expected-error@+1 {{}}
(void)OUString("footest").replaceAll(consttest, bar);
// expected-error@+1 {{}}
(void)OUString("footest").replaceAll(test, constbar);
(void)OUString("footest").replaceAll(consttest, constbar);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

View file

@ -67,12 +67,12 @@ void test::oustring::Compare::compareToIgnoreAsciiCase()
{
CPPUNIT_ASSERT_EQUAL(
sal_Int32(0),
OUString("abc").compareToIgnoreAsciiCase("ABC"));
OUString("abc").compareToIgnoreAsciiCase(u"ABC"));
CPPUNIT_ASSERT(
OUString("ABC").compareToIgnoreAsciiCase("abcdef")
OUString("ABC").compareToIgnoreAsciiCase(u"abcdef")
< 0);
CPPUNIT_ASSERT(
OUString("A").compareToIgnoreAsciiCase("_") > 0);
OUString("A").compareToIgnoreAsciiCase(u"_") > 0);
}
void test::oustring::Compare::compareTo()

View file

@ -12,6 +12,8 @@
#include <sal/config.h>
#include <string>
#include <string_view>
#include <utility>
#include <o3tl/cppunittraitshelper.hxx>
@ -33,12 +35,11 @@ class StringLiterals: public CppUnit::TestFixture
private:
void checkCtors();
void checkUsage();
void checkExtraIntArgument();
void checkNonconstChar();
void checkBuffer();
void checkOUStringLiteral();
void checkOUStringChar();
void checkUtf16();
void checkEmbeddedNul();
void testcall( const char str[] );
@ -48,12 +49,11 @@ private:
CPPUNIT_TEST_SUITE(StringLiterals);
CPPUNIT_TEST(checkCtors);
CPPUNIT_TEST(checkUsage);
CPPUNIT_TEST(checkExtraIntArgument);
CPPUNIT_TEST(checkNonconstChar);
CPPUNIT_TEST(checkBuffer);
CPPUNIT_TEST(checkOUStringLiteral);
CPPUNIT_TEST(checkOUStringChar);
CPPUNIT_TEST(checkUtf16);
CPPUNIT_TEST(checkEmbeddedNul);
CPPUNIT_TEST_SUITE_END();
};
@ -157,40 +157,6 @@ void test::oustring::StringLiterals::checkUsage()
CPPUNIT_ASSERT( !rtl_string_unittest_const_literal );
}
void test::oustring::StringLiterals::checkExtraIntArgument()
{
// This makes sure that using by mistake RTL_CONSTASCII_STRINGPARAM does not trigger a different
// overload, i.e. the second argument to match() in this case is the indexFrom argument,
// but with the macro it would contain the length of the string. Therefore
// match( RTL_CONSTASCII_STRINGPARAM( "bar" )) would be match( "bar", 3 ), which would be
// true when called for OUString( "foobar" ). But this should not happen because of the
// &foo[0] trick in the RTL_CONSTASCII_STRINGPARAM macro.
CPPUNIT_ASSERT( !rtl::OUString("foobar").match( "bar" ));
CPPUNIT_ASSERT( !rtl::OUString("foobar").match( RTL_CONSTASCII_STRINGPARAM( "bar" )));
}
void test::oustring::StringLiterals::checkNonconstChar()
{ // check that non-const char[] data do not trigger string literal overloads
CPPUNIT_ASSERT_EQUAL( rtl::OUString( "foobar" ), rtl::OUString( "footest" ).replaceAll( "test", "bar" ));
char test[] = "test";
char bar[] = "bar";
const char consttest[] = "test";
const char constbar[] = "bar";
CPPUNIT_ASSERT(
!VALID_CONVERSION_CALL(
[&test, &bar]() {
return rtl::OUString("footest").replaceAll(test, bar); }));
CPPUNIT_ASSERT(
!VALID_CONVERSION_CALL(
[&consttest, &bar]() {
return rtl::OUString("footest").replaceAll(consttest, bar); }));
CPPUNIT_ASSERT(
!VALID_CONVERSION_CALL(
[&test, &constbar]() {
return rtl::OUString("footest").replaceAll(test, constbar); }));
CPPUNIT_ASSERT_EQUAL( rtl::OUString( "foobar" ), rtl::OUString( "footest" ).replaceAll( consttest, constbar ));
}
void test::oustring::StringLiterals::checkBuffer()
{
rtl::OUStringBuffer buf;
@ -424,6 +390,20 @@ void test::oustring::StringLiterals::checkUtf16() {
CPPUNIT_ASSERT_EQUAL(sal_Int32(5), b.lastIndexOf(u"ab"));
}
void test::oustring::StringLiterals::checkEmbeddedNul() {
using namespace std::literals;
rtl::OUString const s("foobar");
constexpr char16_t const a[] = u"foo\0hidden";
char16_t const * const p = a;
CPPUNIT_ASSERT(s.startsWith(a));
CPPUNIT_ASSERT(s.startsWith(p));
CPPUNIT_ASSERT(s.startsWith(u"foo\0hidden"));
CPPUNIT_ASSERT(!s.startsWith(u"foo\0hidden"s));
CPPUNIT_ASSERT(!s.startsWith(u"foo\0hidden"sv));
CPPUNIT_ASSERT(!s.startsWith(rtlunittest::OUStringLiteral(a)));
CPPUNIT_ASSERT(!s.startsWith(rtlunittest::OUStringLiteral(u"foo\0hidden")));
}
} // namespace
CPPUNIT_TEST_SUITE_REGISTRATION(test::oustring::StringLiterals);

View file

@ -273,6 +273,9 @@ void Test::ustringReplaceAll() {
CPPUNIT_ASSERT_EQUAL(
OUString("xxa"),
OUString("xaa").replaceAll(s_xa, s_xx));
CPPUNIT_ASSERT_EQUAL(
OUString("foobarbaz"), OUString("foobarfoo").replaceAll(u"foo", u"baz", 1));
}
void Test::ustringReplaceAllAsciiL() {

View file

@ -1547,10 +1547,19 @@ void rtl_uString_newReplaceAllUtf16LUtf16L(
rtl_uString ** newStr, rtl_uString * str, sal_Unicode const * from,
sal_Int32 fromLength, sal_Unicode const * to, sal_Int32 toLength)
SAL_THROW_EXTERN_C()
{
rtl_uString_newReplaceAllFromIndexUtf16LUtf16L(newStr, str, from, fromLength, to, toLength, 0);
}
void rtl_uString_newReplaceAllFromIndexUtf16LUtf16L(
rtl_uString ** newStr, rtl_uString * str, sal_Unicode const * from,
sal_Int32 fromLength, sal_Unicode const * to, sal_Int32 toLength, sal_Int32 fromIndex)
SAL_THROW_EXTERN_C()
{
assert(toLength >= 0);
assert(fromIndex >= 0 && fromIndex <= str->length);
rtl_uString_assign(newStr, str);
for (sal_Int32 i = 0;; i += toLength) {
for (sal_Int32 i = fromIndex;; i += toLength) {
rtl_uString_newReplaceFirstUtf16LUtf16L(
newStr, *newStr, from, fromLength, to, toLength, &i);
if (i == -1 || *newStr == nullptr) {

View file

@ -749,6 +749,11 @@ PRIVATE_1.6 { # LibreOffice 6.4
rtl_ustr_toInt64_WithLength;
} PRIVATE_1.5;
PRIVATE_1.7 { # LibreOffice 7.1
global:
rtl_uString_newReplaceAllFromIndexUtf16LUtf16L;
} PRIVATE_1.5;
PRIVATE_textenc.1 { # LibreOffice 3.6
global:
_ZN3sal6detail7textenc20convertCharToUnicode*;

View file

@ -452,7 +452,7 @@ XclExpHyperlink::XclExpHyperlink( const XclExpRoot& rRoot, const SvxURLField& rU
mnFlags |= EXC_HLINK_MARK;
OUString location = XclXmlUtils::ToOUString(*mxTextMark);
if (!location.isEmpty() && msTarget.endsWith("#" + location))
if (!location.isEmpty() && msTarget.endsWith(OUString("#" + location)))
msTarget = msTarget.copy(0, msTarget.getLength() - location.getLength() - 1);
}

View file

@ -589,7 +589,7 @@ Any SAL_CALL TypeConverter_Impl::convertTo( const Any& rVal, const Type& aDestTy
for ( nPos = reinterpret_cast<typelib_EnumTypeDescription *>(aEnumTD.get())->nEnumValues; nPos--; )
{
if (o3tl::forceAccess<OUString>(rVal)->equalsIgnoreAsciiCase(
reinterpret_cast<typelib_EnumTypeDescription *>(aEnumTD.get())->ppEnumNames[nPos] ))
OUString::unacquired(&reinterpret_cast<typelib_EnumTypeDescription *>(aEnumTD.get())->ppEnumNames[nPos]) ))
break;
}
}

View file

@ -5558,7 +5558,7 @@ OUString SvNumberformat::impTransliterateImpl(const OUString& rStr,
sal_Int32 nField = -1;
do
{
nField = rNum.GetParams().indexOf(rKeywords[nDateKey] + "=", ++nField);
nField = rNum.GetParams().indexOf(OUString(rKeywords[nDateKey] + "="), ++nField);
}
while (nField != -1 && nField != 0 &&
(rNum.GetParams()[nField - 1] != ',' &&

View file

@ -194,7 +194,7 @@ void PlaceEditDialog::InitDetails( )
auto nSize = std::min(aTypesUrlsList.getLength(), aTypesNamesList.getLength());
for ( sal_Int32 i = 0; i < nSize; ++i )
{
OUString sUrl = aTypesUrlsList[i].replaceFirst("<host", "<" + SvtResId(STR_SVT_HOST)).replaceFirst("port>", SvtResId(STR_SVT_PORT) + ">");
OUString sUrl = aTypesUrlsList[i].replaceFirst("<host", OUString("<" + SvtResId(STR_SVT_HOST))).replaceFirst("port>", OUString(SvtResId(STR_SVT_PORT) + ">"));
if ((sUrl == GDRIVE_BASE_URL && bSkipGDrive) ||
(sUrl.startsWith( ALFRESCO_CLOUD_BASE_URL) && bSkipAlfresco) ||

View file

@ -288,7 +288,7 @@ void Gallery::ImplLoadSubDirs( const INetURLObject& rBaseURL, bool& rbDirIsReadO
{
const char *appBundle = [[[NSBundle mainBundle] bundlePath] UTF8String];
OUString path = rBaseURL.GetURLPath();
if( path.startsWith( OUString( appBundle, strlen( appBundle ), RTL_TEXTENCODING_UTF8 ) + "/" ) )
if( path.startsWith( OUString(OUString( appBundle, strlen( appBundle ), RTL_TEXTENCODING_UTF8 ) + "/") ) )
rbDirIsReadOnly = true;
}
#else

View file

@ -98,7 +98,7 @@ namespace
// #120879# - helper method to identify a bookmark name to match the internal TOC bookmark naming convention
bool IsTOCBookmarkName(const OUString& rName)
{
return rName.startsWith("_Toc") || rName.startsWith(IDocumentMarkAccess::GetCrossRefHeadingBookmarkNamePrefix()+"_Toc");
return rName.startsWith("_Toc") || rName.startsWith(OUString(IDocumentMarkAccess::GetCrossRefHeadingBookmarkNamePrefix()+"_Toc"));
}
OUString EnsureTOCBookmarkName(const OUString& rName)

View file

@ -632,7 +632,7 @@ bool SwCustomizeAddressBlockDialog::HasItem(sal_Int32 nUserData)
}
}
//search for this entry in the content
return m_xDragED->GetText().indexOf("<" + sEntry + ">") >= 0;
return m_xDragED->GetText().indexOf(OUString("<" + sEntry + ">")) >= 0;
}
IMPL_LINK_NOARG(SwCustomizeAddressBlockDialog, SelectionChangedIdleHdl, Timer*, void)

View file

@ -314,7 +314,7 @@ void DocumentSignatureHelper::AppendContentTypes(const uno::Reference<embed::XSt
it = std::find_if(rDefaults.begin(), rDefaults.end(), [&](const beans::StringPair& rPair)
{
return rElement.endsWith("." + rPair.First);
return rElement.endsWith(OUString("." + rPair.First));
});
if (it != rDefaults.end())