[Libreoffice-commits] core.git: compilerplugins/clang include/rtl

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Mon Sep 21 20:30:12 UTC 2020


 compilerplugins/clang/unusedmember.cxx |   25 +++++++++++++++++--------
 include/rtl/string.hxx                 |   21 +++++++++++++++++++++
 include/rtl/ustring.hxx                |   20 ++++++++++++++++++++
 3 files changed, 58 insertions(+), 8 deletions(-)

New commits:
commit 52a49f9e480ca03e231cfda82640a928393131c9
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Sep 21 21:01:40 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Sep 21 22:29:31 2020 +0200

    static_assert that O[U]StringLiteral are layout compatible with rtl_[u]String
    
    ...as was suggested by Mike Kaganski in a comment at
    <https://gerrit.libreoffice.org/c/core/+/102222/10#
    message-aa8bcf42f04686766440e2848c7d1f76f474f2f8> "Turn OUStringLiteral into a
    consteval'ed, static-refcound rtl_uString".  Doing so revealed that at least
    lopglugin:unusedmember needs to handle InjectedClassNameType as an alternative
    to RecordType in at least one place.  (More places across compilerplugins might
    benefit from handling InjectedClassNameType too, but which did not lead to
    assertion failures for now and should be addressed in follow-up issues.)
    
    Change-Id: Icdb8b069324b5ce5f3c7c6b92989379ccb67fc8b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103125
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/unusedmember.cxx b/compilerplugins/clang/unusedmember.cxx
index a32fd341c665..1d3017134892 100644
--- a/compilerplugins/clang/unusedmember.cxx
+++ b/compilerplugins/clang/unusedmember.cxx
@@ -240,7 +240,17 @@ public:
         {
             return true;
         }
-        recordRecordDeclAndBases(expr->getTypeSourceInfo()->getType()->castAs<RecordType>());
+        auto const t1 = expr->getTypeSourceInfo()->getType();
+        RecordDecl const* d;
+        if (auto const t2 = t1->getAs<InjectedClassNameType>())
+        {
+            d = t2->getDecl();
+        }
+        else
+        {
+            d = t1->castAs<RecordType>()->getDecl();
+        }
+        recordRecordDeclAndBases(d);
         return true;
     }
 
@@ -276,7 +286,7 @@ public:
         }
         if (auto const t1 = t->getAs<RecordType>())
         {
-            recordRecordDeclAndBases(t1);
+            recordRecordDeclAndBases(t1->getDecl());
         }
         return true;
     }
@@ -406,18 +416,17 @@ private:
         return t.isTrivialType(compiler.getASTContext()) || isWarnUnusedType(t);
     }
 
-    void recordRecordDeclAndBases(RecordType const* type)
+    void recordRecordDeclAndBases(RecordDecl const* decl)
     {
-        auto const d1 = type->getDecl();
-        if (!layout_.insert(d1->getCanonicalDecl()).second)
+        if (!layout_.insert(decl->getCanonicalDecl()).second)
         {
             return;
         }
-        if (auto const d2 = dyn_cast_or_null<CXXRecordDecl>(d1->getDefinition()))
+        if (auto const d2 = dyn_cast_or_null<CXXRecordDecl>(decl->getDefinition()))
         {
             for (auto i = d2->bases_begin(); i != d2->bases_end(); ++i)
             {
-                recordRecordDeclAndBases(i->getType()->castAs<RecordType>());
+                recordRecordDeclAndBases(i->getType()->castAs<RecordType>()->getDecl());
             }
             //TODO: doesn't iterate vbases, but presence of such would run counter to the layout
             // heuristic anyway
@@ -435,7 +444,7 @@ private:
             }
             if (auto const t1 = t->getAs<RecordType>())
             {
-                recordRecordDeclAndBases(t1);
+                recordRecordDeclAndBases(t1->getDecl());
             }
             break;
         }
diff --git a/include/rtl/string.hxx b/include/rtl/string.hxx
index 7086de15c987..fd65e2dfa477 100644
--- a/include/rtl/string.hxx
+++ b/include/rtl/string.hxx
@@ -24,6 +24,7 @@
 
 #include <cassert>
 #include <cstddef>
+#include <cstdlib>
 #include <limits>
 #include <new>
 #include <ostream>
@@ -32,6 +33,7 @@
 
 #if defined LIBO_INTERNAL_ONLY
 #include <string_view>
+#include <type_traits>
 #endif
 
 #include "rtl/textenc.h"
@@ -114,12 +116,31 @@ public:
 
     constexpr char const * getStr() const SAL_RETURNS_NONNULL { return buffer; }
 
+    // offsetof needs a complete type, so do not have these static_asserts as class template
+    // members, but postpone their instantiation to the later non-member static_assert that calls
+    // detail_assertLayout:
+    static constexpr bool detail_assertLayout() {
+        static_assert(offsetof(OStringLiteral, refCount) == offsetof(rtl_String, refCount));
+        static_assert(
+            std::is_same_v<decltype(refCount), decltype(rtl_String::refCount)>);
+        static_assert(offsetof(OStringLiteral, length) == offsetof(rtl_String, length));
+        static_assert(std::is_same_v<decltype(length), decltype(rtl_String::length)>);
+        static_assert(offsetof(OStringLiteral, buffer) == offsetof(rtl_String, buffer));
+        static_assert(
+            std::is_same_v<
+                std::remove_extent_t<decltype(buffer)>,
+                std::remove_extent_t<decltype(rtl_String::buffer)>>);
+        return true;
+    }
+
 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)
 };
+
+static_assert(OStringLiteral<1>::detail_assertLayout());
 #endif
 
 /* ======================================================================= */
diff --git a/include/rtl/ustring.hxx b/include/rtl/ustring.hxx
index 45272b325cc0..f458ac061ea4 100644
--- a/include/rtl/ustring.hxx
+++ b/include/rtl/ustring.hxx
@@ -24,6 +24,7 @@
 
 #include <cassert>
 #include <cstddef>
+#include <cstdlib>
 #include <limits>
 #include <new>
 #include <ostream>
@@ -31,6 +32,7 @@
 
 #if defined LIBO_INTERNAL_ONLY
 #include <string_view>
+#include <type_traits>
 #endif
 
 #include "rtl/ustring.h"
@@ -98,6 +100,22 @@ public:
 
     constexpr operator std::u16string_view() const { return {buffer, sal_uInt32(length)}; }
 
+    // offsetof needs a complete type, so do not have these static_asserts as class template
+    // members, but postpone their instantiation to the later non-member static_assert that calls
+    // detail_assertLayout:
+    static constexpr bool detail_assertLayout() {
+        static_assert(offsetof(OUStringLiteral, refCount) == offsetof(rtl_uString, refCount));
+        static_assert(std::is_same_v<decltype(refCount), decltype(rtl_uString::refCount)>);
+        static_assert(offsetof(OUStringLiteral, length) == offsetof(rtl_uString, length));
+        static_assert(std::is_same_v<decltype(length), decltype(rtl_uString::length)>);
+        static_assert(offsetof(OUStringLiteral, buffer) == offsetof(rtl_uString, buffer));
+        static_assert(
+            std::is_same_v<
+                std::remove_extent_t<decltype(buffer)>,
+                std::remove_extent_t<decltype(rtl_uString::buffer)>>);
+        return true;
+    }
+
 private:
     // Same layout as rtl_uString (include/rtl/ustring.h):
     oslInterlockedCount refCount = 0x40000000; // SAL_STRING_STATIC_FLAG (sal/rtl/strimp.hxx)
@@ -105,6 +123,8 @@ private:
     sal_Unicode buffer[N] = {}; //TODO: drop initialization for C++20 (P1331R2)
 };
 
+static_assert(OUStringLiteral<1>::detail_assertLayout());
+
 #if defined RTL_STRING_UNITTEST
 namespace libreoffice_internal {
 template<std::size_t N> struct ExceptConstCharArrayDetector<OUStringLiteral<N>> {};


More information about the Libreoffice-commits mailing list