[Libreoffice-commits] core.git: codemaker/source compilerplugins/clang config_host/config_global.h.in configure.ac i18npool/source include/rtl sal/qa
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Wed Sep 2 06:12:45 UTC 2020
codemaker/source/javamaker/javaoptions.cxx | 2
compilerplugins/clang/compat.hxx | 24 ++++
compilerplugins/clang/stringconcatliterals.cxx | 2
compilerplugins/clang/test/stringconcatliterals.cxx | 2
compilerplugins/clang/unusedmember.cxx | 3
config_host/config_global.h.in | 4
configure.ac | 13 ++
i18npool/source/breakiterator/breakiterator_unicode.cxx | 2
include/rtl/string.hxx | 90 +++++++++-------
sal/qa/rtl/digest/rtl_digest.cxx | 15 +-
sal/qa/rtl/strings/test_ostring_concat.cxx | 10 +
sal/qa/rtl/textenc/rtl_textcvt.cxx | 2
12 files changed, 118 insertions(+), 51 deletions(-)
New commits:
commit 4b9e440c51be3e40326bc90c33ae69885bfb51e4
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Aug 31 21:19:22 2020 +0200
Commit: Stephan Bergmann <sbergman at redhat.com>
CommitDate: Wed Sep 2 08:12:04 2020 +0200
Turn OStringLiteral into a consteval'ed, static-refcound rtl_String
...from which an OString can cheaply be instantiated.
The one downside is that OStringLiteral now needs to be a template abstracting
over the string length. But any uses for which that is a problem (e.g., as the
element type of a containers that would no longer be homogeneous, or in the
signature of a function that shall not be turned into a template for one reason
or another) can be replaced with std::string_view, without loss of efficiency
compared to the original OStringLiteral, and without loss of expressivity (esp.
with the newly introduced OString(std::string_view) ctor).
The new OStringLiteral ctor code would probably not be very efficient if it were
ever executed at runtime, but it is intended to be only executed at compile
time. Where available, C++20 "consteval" is used to statically ensure that.
The intended use of the new OStringLiteral is in all cases where an
object that shall itself not be an OString (e.g., because it shall be a
global static variable for which the OString ctor/dtor would be detrimental at
library load/unload) must be converted to an OString instance in at least one
place. Other string literal abstractions could use std::string_view (or just
plain char const[N]), but interestingly OStringLiteral might be more efficient
than constexpr std::string_view even for such cases, as it should not need any
relocations at library load time. For now, no existing uses of OUStringLiteral
have been changed to some other abstraction (unless technically necessary as
discussed above), and no additional places that would benefit from
OUStringLiteral have been changed to use it.
sal/qa/rtl/strings/test_ostring_concat.cxx documents some workarounds for GCC
bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878> "Failed class template
argument deduction in unevaluated, parenthesized context". Those places, as
well as uses of OStringLiteral in incodemaker/source/javamaker/javaoptions.cxx
and i18npool/source/breakiterator/breakiterator_unicode.cxx, which have been
replaced with OString::Concat (and which is arguably a better choice, anyway),
also caused failures with at least Clang 5.0.2 (but would not have caused
failures with at least recent Clang 12 trunk, so appear to be bugs in Clang that
have meanwhile been fixed).
This change also revealed a bug in at least recent Clang 12 trunk
CastExpr::getSubExprAsWritten (still to be reported to LLVM), triggered at least
in some calls from loplugin code (for which it can be fixed for now in the
existing compat::getSubStringAsWritten).
A similar commit for OUStringLiteral is planned, too.
Change-Id: Ib192f4ed4c44769512a16364cb55c25627bae6f4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101814
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/codemaker/source/javamaker/javaoptions.cxx b/codemaker/source/javamaker/javaoptions.cxx
index 31e43414c68c..c86f35e6cf15 100644
--- a/codemaker/source/javamaker/javaoptions.cxx
+++ b/codemaker/source/javamaker/javaoptions.cxx
@@ -86,7 +86,7 @@ bool JavaOptions::initOptions(int ac, char* av[], bool bCmdFile)
case 'n':
if (av[i][2] != 'D' || av[i][3] != '\0')
{
- OString tmp(OStringLiteral("'-nD', please check your input '") + av[i] + "'");
+ OString tmp(OString::Concat("'-nD', please check your input '") + av[i] + "'");
throw IllegalArgument(tmp);
}
diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index fb8791f978b4..70b462e54e4b 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -215,6 +215,28 @@ inline clang::Expr * getSubExpr(clang::MaterializeTemporaryExpr const * expr) {
// `-MemberExpr 0x2b74e8e656a0 '<bound member function type>' .operator ResString 0x2b74e8dc1f00
// `-DeclRefExpr 0x2b74e8e65648 'struct ErrorResource_Impl' lvalue Var 0x2b74e8e653b0 'aEr' 'struct ErrorResource_Impl'
//
+// Also work around CastExpr::getSubExprAsWritten firing
+//
+// include/llvm/Support/Casting.h:269: typename llvm::cast_retty<X, Y*>::ret_type llvm::cast(Y*)
+// [with X = clang::CXXConstructExpr; Y = clang::Expr;
+// typename llvm::cast_retty<X, Y*>::ret_type = clang::CXXConstructExpr*]: Assertion
+// `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
+//
+// for CastExprs involving ConstantExpr (introduced with
+// <https://github.com/llvm/llvm-project/commit/7c44da279e399d302a685c500e7f802f8adf9762> "Create
+// ConstantExpr class" towards LLVM 8) like
+//
+// CXXFunctionalCastExpr 0xc01c4e8 'class rtl::OStringLiteral<9>':'class rtl::OStringLiteral<9>' functional cast to OStringLiteral <ConstructorConversion>
+// `-ConstantExpr 0xc01c380 'class rtl::OStringLiteral<9>':'class rtl::OStringLiteral<9>'
+// |-value: Struct
+// | |-fields: Int 1073741824, Int 8
+// | `-field: Array size=9
+// | |-elements: Int 46, Int 111, Int 115, Int 108
+// | |-elements: Int 45, Int 116, Int 109, Int 112
+// | `-element: Int 0
+// `-CXXConstructExpr 0xc01c350 'class rtl::OStringLiteral<9>':'class rtl::OStringLiteral<9>' 'void (const char (&)[9])'
+// `-StringLiteral 0xc019ad8 'const char [9]' lvalue ".osl-tmp"
+//
// Copies code from Clang's lib/AST/Expr.cpp:
namespace detail {
inline clang::Expr *skipImplicitTemporary(clang::Expr *expr) {
@@ -240,7 +262,7 @@ inline clang::Expr *getSubExprAsWritten(clang::CastExpr *This) {
// subexpression describing the call; strip it off.
if (E->getCastKind() == clang::CK_ConstructorConversion)
SubExpr =
- detail::skipImplicitTemporary(clang::cast<clang::CXXConstructExpr>(SubExpr)->getArg(0));
+ detail::skipImplicitTemporary(clang::cast<clang::CXXConstructExpr>(SubExpr->IgnoreImplicit())->getArg(0));
else if (E->getCastKind() == clang::CK_UserDefinedConversion) {
assert((clang::isa<clang::CXXMemberCallExpr>(SubExpr) ||
clang::isa<clang::BlockExpr>(SubExpr)) &&
diff --git a/compilerplugins/clang/stringconcatliterals.cxx b/compilerplugins/clang/stringconcatliterals.cxx
index 25872ff6ee32..509a10363c01 100644
--- a/compilerplugins/clang/stringconcatliterals.cxx
+++ b/compilerplugins/clang/stringconcatliterals.cxx
@@ -27,7 +27,7 @@ Expr const * stripCtor(Expr const * expr) {
return expr;
}
auto qt = loplugin::DeclCheck(e2->getConstructor());
- if (qt.MemberFunction().Struct("OStringLiteral").Namespace("rtl").GlobalNamespace()) {
+ if (qt.MemberFunction().Class("OStringLiteral").Namespace("rtl").GlobalNamespace()) {
if (e2->getNumArgs() == 1) {
return e2->getArg(0)->IgnoreParenImpCasts();
}
diff --git a/compilerplugins/clang/test/stringconcatliterals.cxx b/compilerplugins/clang/test/stringconcatliterals.cxx
index 76b82797f77e..348440f0ec4c 100644
--- a/compilerplugins/clang/test/stringconcatliterals.cxx
+++ b/compilerplugins/clang/test/stringconcatliterals.cxx
@@ -19,7 +19,7 @@
void f(std::ostream& s1)
{
- static char const foo[] = "foo";
+ static constexpr char foo[] = "foo";
static char16_t const foou[] = u"foo";
s1 << "foo"
<< "foo";
diff --git a/compilerplugins/clang/unusedmember.cxx b/compilerplugins/clang/unusedmember.cxx
index bcd9a8593a68..a32fd341c665 100644
--- a/compilerplugins/clang/unusedmember.cxx
+++ b/compilerplugins/clang/unusedmember.cxx
@@ -21,6 +21,7 @@
#include "config_clang.h"
#include "check.hxx"
+#include "compat.hxx"
#include "plugin.hxx"
namespace
@@ -289,7 +290,7 @@ public:
return true;
}
auto const t1 = expr->getType();
- auto const t2 = expr->getSubExprAsWritten()->getType();
+ auto const t2 = compat::getSubExprAsWritten(expr)->getType();
if (loplugin::TypeCheck(t1).Pointer().Void())
{
recordCastedRecordDecl(t2);
diff --git a/config_host/config_global.h.in b/config_host/config_global.h.in
index d9a907f4b91f..0df9ac8a47f1 100644
--- a/config_host/config_global.h.in
+++ b/config_host/config_global.h.in
@@ -15,6 +15,10 @@ Any change in this header will cause a rebuild of almost everything.
#define HAVE_GCC_BUILTIN_ATOMIC 0
#define HAVE_SYSLOG_H 0
+// Compiler supports C++20 <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1073r3.html>
+// "Immediate functions":
+#define HAVE_CPP_CONSTEVAL 0
+
// Compiler supports all of C++2a <https://wg21.link/P0202R3> "Add Constexpr Modifiers to Functions
// in <algorithm> and <utility> Headers", <https://wg21.link/P1004R2> "Making std::vector
// constexpr", and <https://wg21.link/P1143R2> "Adding the constinit keyword":
diff --git a/configure.ac b/configure.ac
index d60bfaf425a3..c05f66fd855f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -6780,6 +6780,19 @@ if test "$GCC" = yes; then
fi
AC_SUBST([HAVE_GCC_FNO_SIZED_DEALLOCATION])
+AC_MSG_CHECKING([whether $CXX_BASE supports C++20 consteval])
+AC_LANG_PUSH([C++])
+save_CXXFLAGS=$CXXFLAGS
+CXXFLAGS="$CXXFLAGS $CXXFLAGS_CXX11"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([
+ consteval int f() { return 0; }
+ ])], [
+ AC_DEFINE([HAVE_CPP_CONSTEVAL],[1])
+ AC_MSG_RESULT([yes])
+ ], [AC_MSG_RESULT([no])])
+CXXFLAGS=$save_CXXFLAGS
+AC_LANG_POP([C++])
+
AC_MSG_CHECKING([whether $CXX_BASE supports C++2a constinit sorted vectors])
AC_LANG_PUSH([C++])
save_CXXFLAGS=$CXXFLAGS
diff --git a/i18npool/source/breakiterator/breakiterator_unicode.cxx b/i18npool/source/breakiterator/breakiterator_unicode.cxx
index bb8d7d9862aa..ca34f91e157b 100644
--- a/i18npool/source/breakiterator/breakiterator_unicode.cxx
+++ b/i18npool/source/breakiterator/breakiterator_unicode.cxx
@@ -214,7 +214,7 @@ void BreakIterator_Unicode::loadICUBreakIterator(const css::lang::Locale& rLocal
rbi.reset();
// ;rule (only)
- const OString aBIMapRuleOnlyKey( OStringLiteral(";") + rule);
+ const OString aBIMapRuleOnlyKey( OString::Concat(";") + rule);
aMapIt = theBIMap.find( aBIMapRuleOnlyKey);
bInMap = (aMapIt != theBIMap.end());
if (bInMap)
diff --git a/include/rtl/string.hxx b/include/rtl/string.hxx
index da75e5eee57c..ca1fd99b7fba 100644
--- a/include/rtl/string.hxx
+++ b/include/rtl/string.hxx
@@ -24,6 +24,7 @@
#include <cassert>
#include <cstddef>
+#include <limits>
#include <new>
#include <ostream>
#include <utility>
@@ -38,6 +39,7 @@
#include "rtl/stringutils.hxx"
#ifdef LIBO_INTERNAL_ONLY // "RTL_FAST_STRING"
+#include "config_global.h"
#include "rtl/stringconcat.hxx"
#endif
@@ -70,34 +72,53 @@ namespace rtl
#ifdef LIBO_INTERNAL_ONLY // "RTL_FAST_STRING"
/**
-A simple wrapper around string literal. It is usually not necessary to use, can
-be mostly used to force OString operator+ working with operands that otherwise would
-not trigger it.
+A wrapper dressing a string literal as a static-refcount rtl_String.
This class is not part of public API and is meant to be used only in LibreOffice code.
@since LibreOffice 4.0
*/
-struct SAL_WARN_UNUSED OStringLiteral
-{
- template< int N >
- explicit OStringLiteral( const char (&str)[ N ] ) : size( N - 1 ), data( str ) { assert( strlen( str ) == N - 1 ); }
+template<std::size_t N> class SAL_WARN_UNUSED OStringLiteral {
+ static_assert(N != 0);
+ static_assert(N - 1 <= std::numeric_limits<sal_Int32>::max(), "literal too long");
+
+public:
+#if HAVE_CPP_CONSTEVAL
+ consteval
+#else
+ constexpr
+#endif
+ explicit OStringLiteral(char const (&literal)[N]) {
+ assert(literal[N - 1] == '\0');
+ //TODO: Use C++20 constexpr std::copy_n (P0202R3):
+ for (std::size_t i = 0; i != N; ++i) {
+ buffer[i] = literal[i];
+ }
+ }
+
#if defined __cpp_char8_t
- template< int N >
- explicit OStringLiteral( const char8_t (&str)[ N ] ) : size( N - 1 ), data( reinterpret_cast<char const *>(str) ) { assert( strlen( data ) == N - 1 ); }
+#if HAVE_CPP_CONSTEVAL
+ consteval
+#else
+ constexpr
+#endif
+ explicit OStringLiteral(char8_t const (&literal)[N]) {
+ assert(literal[N - 1] == '\0');
+ //TODO: Use C++20 constexpr std::copy_n (P0202R3):
+ for (std::size_t i = 0; i != N; ++i) {
+ buffer[i] = literal[i];
+ }
+ }
#endif
- int size;
- const char* data;
+ constexpr sal_Int32 getLength() const { return length; }
- /** So we can use this in some places interchangeably with OUString.
- * @since LibreOffice 7.1
- */
- constexpr sal_Int32 getLength() const { return size; }
+ constexpr char const * getStr() const SAL_RETURNS_NONNULL { return buffer; }
- /** So we can use this in some places interchangeably with OString.
- * @since LibreOffice 7.1
- */
- constexpr const char* getStr() const { return data; }
+private:
+ // Same layout as rtl_String (include/rtl/string.h):
+ oslInterlockedCount refCount = 0x40000000; // SAL_STRING_STATIC_FLAG (sal/rtl/strimp.hxx)
+ sal_Int32 length = N - 1;
+ char buffer[N] = {}; //TODO: drop initialization for C++20 (P1331R2)
};
#endif
@@ -274,23 +295,22 @@ public:
/**
New string from an 8-Bit string literal.
- This constructor is similar to the "direct template" one, but can be
- useful in cases where the latter does not work, like in
-
- OString(flag ? "a" : "bb")
-
- written as
-
- OString(flag ? OStringLiteral("a") : OStringLiteral("bb"))
-
@since LibreOffice 7.1
*/
- OString(OStringLiteral literal): pData(NULL) {
- rtl_string_newFromLiteral(&pData, literal.data, literal.size, 0);
- }
+ template<std::size_t N> OString(OStringLiteral<N> const & literal):
+ pData(const_cast<rtl_String *>(reinterpret_cast<rtl_String const *>(&literal))) {}
/// @endcond
#endif
+#if defined LIBO_INTERNAL_ONLY
+ explicit OString(std::string_view sv) {
+ if (sv.size() > sal_uInt32(std::numeric_limits<sal_Int32>::max())) {
+ throw std::bad_alloc();
+ }
+ pData = nullptr;
+ rtl_string_newFromStr_WithLength(&pData, sv.data(), sv.size());
+ }
+#endif
/**
New string from a Unicode character buffer array.
@@ -1946,11 +1966,11 @@ struct ToStringHelper< OString >
/**
@internal
*/
-template<>
-struct ToStringHelper< OStringLiteral >
+template<std::size_t N>
+struct ToStringHelper< OStringLiteral<N> >
{
- static std::size_t length( const OStringLiteral& str ) { return str.size; }
- static char* addData( char* buffer, const OStringLiteral& str ) { return addDataHelper( buffer, str.data, str.size ); }
+ static constexpr std::size_t length( const OStringLiteral<N>& str ) { return str.getLength(); }
+ static char* addData( char* buffer, const OStringLiteral<N>& str ) { return addDataHelper( buffer, str.getStr(), str.getLength() ); }
static const bool allowOStringConcat = true;
static const bool allowOUStringConcat = false;
};
diff --git a/sal/qa/rtl/digest/rtl_digest.cxx b/sal/qa/rtl/digest/rtl_digest.cxx
index e2392eaadcc8..d948deaf6d6c 100644
--- a/sal/qa/rtl/digest/rtl_digest.cxx
+++ b/sal/qa/rtl/digest/rtl_digest.cxx
@@ -24,6 +24,7 @@
#include <cppunit/plugin/TestPlugIn.h>
#include <memory>
+#include <string_view>
#include <rtl/digest.h>
#include <rtl/string.h>
@@ -61,14 +62,14 @@ const sal_uInt32 constDigestAlgorithmLengths[] =
RTL_DIGEST_LENGTH_HMAC_SHA1,
};
-const OStringLiteral constSampleStringSums[] =
+const std::string_view constSampleStringSums[] =
{
- OStringLiteral("647ee6c9d4aa5fdd374ed9d7a156acbf"),
- OStringLiteral("b16b903e6fc0b62ae389013ed93fe531"),
- OStringLiteral("eab2814429b2613301c8a077b806af3680548914"),
- OStringLiteral("2bc5bdb7506a2cdc2fd27fc8b9889343012d5008"),
- OStringLiteral("0b1b0e1a6f2e4420326354b031063605"),
- OStringLiteral("1998c6a556915be76451bfb587fa7c34d849936e")
+ std::string_view("647ee6c9d4aa5fdd374ed9d7a156acbf"),
+ std::string_view("b16b903e6fc0b62ae389013ed93fe531"),
+ std::string_view("eab2814429b2613301c8a077b806af3680548914"),
+ std::string_view("2bc5bdb7506a2cdc2fd27fc8b9889343012d5008"),
+ std::string_view("0b1b0e1a6f2e4420326354b031063605"),
+ std::string_view("1998c6a556915be76451bfb587fa7c34d849936e")
};
// Create hex-value string from the digest value to keep the string size minimal
diff --git a/sal/qa/rtl/strings/test_ostring_concat.cxx b/sal/qa/rtl/strings/test_ostring_concat.cxx
index e5872654b58d..94f4e8d692cf 100644
--- a/sal/qa/rtl/strings/test_ostring_concat.cxx
+++ b/sal/qa/rtl/strings/test_ostring_concat.cxx
@@ -68,9 +68,15 @@ void test::ostring::StringConcat::checkConcat()
CPPUNIT_ASSERT_EQUAL( OString( "foobar" ), OString( OStringBuffer( "foo" ) + "bar" ));
CPPUNIT_ASSERT_EQUAL(( typeid( OStringConcat< OStringBuffer, const char[ 4 ] > )), typeid( OStringBuffer( "foo" ) + "bar" ));
CPPUNIT_ASSERT_EQUAL( OString( "foobar" ), OString( OStringLiteral( "foo" ) + "bar" ));
- CPPUNIT_ASSERT_EQUAL(( typeid( OStringConcat< OStringLiteral, const char[ 4 ] > )), typeid( OStringLiteral( "foo" ) + "bar" ));
+ CPPUNIT_ASSERT_EQUAL(( typeid( OStringConcat< OStringLiteral<4>, const char[ 4 ] > )), typeid( OStringLiteral<4>( "foo" ) + "bar" ));
+ //TODO: the explicit OUStringLiteral<4> template argument in the unevaluated typeid context
+ // is needed by some GCC versions, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878>
+ // "Failed class template argument deduction in unevaluated, parenthesized context"
CPPUNIT_ASSERT_EQUAL( OString( "foobar" ), OString( OStringLiteral( "foo" ) + static_cast<const char*>("bar") ));
- CPPUNIT_ASSERT_EQUAL(( typeid( OStringConcat< OStringLiteral, const char* > )), typeid( OStringLiteral( "foo" ) + static_cast<const char*>("bar") ));
+ CPPUNIT_ASSERT_EQUAL(( typeid( OStringConcat< OStringLiteral<4>, const char* > )), typeid( OStringLiteral<4>( "foo" ) + static_cast<const char*>("bar") ));
+ //TODO: the explicit OUStringLiteral<4> template argument in the unevaluated typeid context
+ // is needed by some GCC versions, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878>
+ // "Failed class template argument deduction in unevaluated, parenthesized context"
const char d1[] = "xyz";
char d2[] = "abc";
const char* d3 = d1;
diff --git a/sal/qa/rtl/textenc/rtl_textcvt.cxx b/sal/qa/rtl/textenc/rtl_textcvt.cxx
index 8e619a23e1ca..6dec034a96ff 100644
--- a/sal/qa/rtl/textenc/rtl_textcvt.cxx
+++ b/sal/qa/rtl/textenc/rtl_textcvt.cxx
@@ -2966,7 +2966,7 @@ void Test::testInvalidUtf8() {
sal_uInt32 info;
sal_Size converted;
auto const size = rtl_convertTextToUnicode(
- converter, nullptr, input.data, input.size, buf,
+ converter, nullptr, input.getStr(), input.getLength(), buf,
TEST_STRING_SIZE,
(RTL_TEXTTOUNICODE_FLAGS_UNDEFINED_ERROR
| RTL_TEXTTOUNICODE_FLAGS_MBUNDEFINED_ERROR
More information about the Libreoffice-commits
mailing list