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>

8 files changed, 102 insertions, 42 deletions

diff --git a/sal/CompilerTest_sal_rtl_oustring.mk b/sal/CompilerTest_sal_rtl_oustring.mk
new file mode 100644
index 000000000000..fb77d89048a6
--- /dev/null
+++ b/sal/CompilerTest_sal_rtl_oustring.mk
@@ -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:
diff --git a/sal/Module_sal.mk b/sal/Module_sal.mk
index 4d7a84ee4e61..7611bc950f07 100644
--- a/sal/Module_sal.mk
+++ b/sal/Module_sal.mk
@@ -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
diff --git a/sal/qa/rtl/strings/compile-oustring.cxx b/sal/qa/rtl/strings/compile-oustring.cxx
new file mode 100644
index 000000000000..667a52324e4e
--- /dev/null
+++ b/sal/qa/rtl/strings/compile-oustring.cxx
@@ -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: */
diff --git a/sal/qa/rtl/strings/test_oustring_compare.cxx b/sal/qa/rtl/strings/test_oustring_compare.cxx
index c06d40c4d653..6316f09b1d58 100644
--- a/sal/qa/rtl/strings/test_oustring_compare.cxx
+++ b/sal/qa/rtl/strings/test_oustring_compare.cxx
@@ -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()
diff --git a/sal/qa/rtl/strings/test_oustring_stringliterals.cxx b/sal/qa/rtl/strings/test_oustring_stringliterals.cxx
index 4ba1b04067d4..a1c79f8cc9e5 100644
--- a/sal/qa/rtl/strings/test_oustring_stringliterals.cxx
+++ b/sal/qa/rtl/strings/test_oustring_stringliterals.cxx
@@ -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);
diff --git a/sal/qa/rtl/strings/test_strings_replace.cxx b/sal/qa/rtl/strings/test_strings_replace.cxx
index a27f5f14286b..3be6e176ad41 100644
--- a/sal/qa/rtl/strings/test_strings_replace.cxx
+++ b/sal/qa/rtl/strings/test_strings_replace.cxx
@@ -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() {
diff --git a/sal/rtl/ustring.cxx b/sal/rtl/ustring.cxx
index d3bf1daedc78..cbadddc6a98e 100644
--- a/sal/rtl/ustring.cxx
+++ b/sal/rtl/ustring.cxx
@@ -1548,9 +1548,18 @@ void rtl_uString_newReplaceAllUtf16LUtf16L(
     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) {
diff --git a/sal/util/sal.map b/sal/util/sal.map
index 48d76b1a3802..dbd22c36ee16 100644
--- a/sal/util/sal.map
+++ b/sal/util/sal.map
@@ -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*;