compilerplugins/clang/test/typeidcomparison.cxx | 41 ++++++++++
compilerplugins/clang/typeidcomparison.cxx | 90 ++++++++++++++++++++++++
solenv/CompilerTest_compilerplugins_clang.mk | 1
sw/inc/istype.hxx | 21 +++++
sw/source/core/access/accmap.cxx | 3
sw/source/core/doc/doclay.cxx | 3
sw/source/uibase/app/docsh2.cxx | 7 +
7 files changed, 161 insertions(+), 5 deletions(-)
New commits:
commit d2ba5d98ec67e684c819ef80421eb496723a8d06
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Sat Feb 12 17:10:43 2022 +0100
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Sat Feb 12 19:37:43 2022 +0100
Fix some tautological std::type_info (in-)equality comparisons
...which could never succeed.
I became aware of this when Clang 15 trunk -std=c++2b, implementing
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1328r1.html>
"Making
std::type_info::operator== constexpr", pointed at two of the broken
comparisons
with
> sw/source/uibase/app/docsh2.cxx:478:25: error: code will never be
executed [-Werror,-Wunreachable-code]
> pTmpFrame->GetFrame().Appear();
> ^~~~~~~~~
> sw/source/uibase/app/docsh2.cxx:475:33: error: code will never be
executed [-Werror,-Wunreachable-code]
> bOnly = false;
> ^~~~~
(It didn't emit warnings pointing at any of the other broken comparisons,
though.)
All of these broken comparisons were regressions introduced with
89d39bc100aabf5dccbe77c0b5c0c85736e85b39 "tdf#94559: 4th step to remove
rtti.hxx", replacing uses of the IS_TYPE macro (from include/tools/rtti.hxx,
meanwhile removed with d64e535fe9a00b671cf1be3eb5632c0d5f4b8bea "Remove
unused
rtti.hxx").
I now added loplugin:typeidcomparison to also find the other broken
comparisons
introduced by that commit. (The remaining cases where that commit replaced
uses
of TYPE_INFO with typeid comparisons were correct and/or have meanwhile been
replaced with code not using typeid, see
553ee72041d6f66e26156eb1ad0d9e3c13457f7a "simplify some use of typeid" and
d656da9bc4f2df0bb99c65a288847e3fdd43a37c "~SwModify: do not silently
tolerate
clients registered past death".)
The original IS_TYPE macro made sure not to dereference null pointers,
> #define IS_TYPE(T,pObj) \
> ( pObj && (pObj)->Type() == TYPE(T) )
I don't know if any of the pointers now dereferenced in those typeid
expressions
can legitimately be null. But to be on the safe side, I replicated that
check
in the newly introduced isType (sw/inc/istype.hxx).
(It is interesting to note that none of the static analysis that we
routinely
employ seems to have noticed these broken comparisons.)
Change-Id: I65baffdd27bac1abf744283ff98c2dc864fa63b4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129865
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/compilerplugins/clang/test/typeidcomparison.cxx
b/compilerplugins/clang/test/typeidcomparison.cxx
new file mode 100644
index 000000000000..31ab749a2496
--- /dev/null
+++ b/compilerplugins/clang/test/typeidcomparison.cxx
@@ -0,0 +1,41 @@
+/* -*- 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 <typeinfo>
+
+struct Base
+{
+ virtual ~Base();
+};
+
+struct Derived : Base
+{
+};
+
+void good(Base* p)
+{
+ (void)(typeid(*p) == typeid(Derived));
+ (void)(typeid(Derived) == typeid(*p));
+ (void)(typeid(*p) != typeid(Derived));
+ (void)(typeid(Derived) != typeid(*p));
+}
+
+void bad(Base* p)
+{
+ // expected-error@+1 {{comparison of type info of mixed pointer and
non-pointer types 'Base *' and 'Derived' can never succeed
[loplugin:typeidcomparison]}}
+ (void)(typeid(p) == typeid(Derived));
+ // expected-error@+1 {{comparison of type info of mixed pointer and
non-pointer types 'Derived' and 'Base *' can never succeed
[loplugin:typeidcomparison]}}
+ (void)(typeid(Derived) == typeid(p));
+ // expected-error@+1 {{comparison of type info of mixed pointer and
non-pointer types 'Base *' and 'Derived' can never succeed
[loplugin:typeidcomparison]}}
+ (void)(typeid(p) != typeid(Derived));
+ // expected-error@+1 {{comparison of type info of mixed pointer and
non-pointer types 'Derived' and 'Base *' can never succeed
[loplugin:typeidcomparison]}}
+ (void)(typeid(Derived) != typeid(p));
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/compilerplugins/clang/typeidcomparison.cxx
b/compilerplugins/clang/typeidcomparison.cxx
new file mode 100644
index 000000000000..359231f262e6
--- /dev/null
+++ b/compilerplugins/clang/typeidcomparison.cxx
@@ -0,0 +1,90 @@
+/* -*- 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/.
+ */
+
+// Find (in-)equality comparisons between typeid expressions that can never
succeed. For now, just
+// detects cases where the two involved types are structurally different, one
a pointer type and the
+// other a non-pointer type.
+
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+#include "plugin.hxx"
+
+namespace
+{
+class TypeidComparison final : public
loplugin::FilteringPlugin<TypeidComparison>
+{
+public:
+ explicit TypeidComparison(loplugin::InstantiationData const& data)
+ : FilteringPlugin(data)
+ {
+ }
+
+ // For CXXRewrittenBinaryOperator `typeid(...) != typeid(...)`:
+ bool shouldVisitImplicitCode() const { return true; }
+
+ bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
+
+ void run() override
+ {
+ if (preRun())
+ {
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ }
+ }
+
+ bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* expr)
+ {
+ if (ignoreLocation(expr))
+ {
+ return true;
+ }
+ auto const op = expr->getOperator();
+ if (op != OO_EqualEqual && op != OO_ExclaimEqual)
+ {
+ return true;
+ }
+ assert(expr->getNumArgs() == 2);
+ auto const e1 =
dyn_cast<CXXTypeidExpr>(expr->getArg(0)->IgnoreParenImpCasts());
+ if (e1 == nullptr)
+ {
+ return true;
+ }
+ auto const e2 =
dyn_cast<CXXTypeidExpr>(expr->getArg(1)->IgnoreParenImpCasts());
+ if (e2 == nullptr)
+ {
+ return true;
+ }
+ auto const t1 = getOperandType(e1);
+ auto const t2 = getOperandType(e2);
+ if (t1->isPointerType() == t2->isPointerType())
+ {
+ return true;
+ }
+ report(DiagnosticsEngine::Warning,
+ "comparison of type info of mixed pointer and non-pointer types
%0 and %1 can never "
+ "succeed",
+ expr->getExprLoc())
+ << t1 << t2 << expr->getSourceRange();
+ return true;
+ }
+
+private:
+ QualType getOperandType(CXXTypeidExpr const* expr)
+ {
+ return expr->isTypeOperand() ?
expr->getTypeOperand(compiler.getASTContext())
+ : expr->getExprOperand()->getType();
+ }
+};
+
+static loplugin::Plugin::Registration<TypeidComparison>
typeidcomparison("typeidcomparison");
+}
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk
b/solenv/CompilerTest_compilerplugins_clang.mk
index 93ebf8468228..66b0579dd69b 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -95,6 +95,7 @@ $(eval $(call
gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/stringview \
compilerplugins/clang/test/stringviewparam \
compilerplugins/clang/test/typedefparam \
+ compilerplugins/clang/test/typeidcomparison \
compilerplugins/clang/test/unnecessarycatchthrow \
compilerplugins/clang/test/unnecessaryoverride \
compilerplugins/clang/test/unnecessaryoverride-dtor \
diff --git a/sw/inc/istype.hxx b/sw/inc/istype.hxx
new file mode 100644
index 000000000000..ec7dac428d61
--- /dev/null
+++ b/sw/inc/istype.hxx
@@ -0,0 +1,21 @@
+/* -*- 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/.
+ */
+
+#pragma once
+
+#include <sal/config.h>
+
+#include <typeinfo>
+
+template <typename T1, typename T2> bool isType(T2* p)
+{
+ return p != nullptr && typeid(*p) == typeid(T1);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx
index 7704b16f3e52..e8a106aa5892 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -42,6 +42,7 @@
#include "acccell.hxx"
#include "acctable.hxx"
#include <fesh.hxx>
+#include <istype.hxx>
#include <rootfrm.hxx>
#include <txtfrm.hxx>
#include <hffrm.hxx>
@@ -198,7 +199,7 @@ void SwDrawModellListener_Impl::Notify( SfxBroadcaster&
/*rBC*/,
if (pSdrHint->GetObject() &&
( dynamic_cast< const SwFlyDrawObj* >(pSdrHint->GetObject()) !=
nullptr ||
dynamic_cast< const SwVirtFlyDrawObj* >(pSdrHint->GetObject())
!= nullptr ||
- typeid(SdrObject) == typeid(pSdrHint->GetObject()) ) )
+ isType<SdrObject>(pSdrHint->GetObject()) ) )
{
return;
}
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 7d3ad61e46b4..34bf4fabbcac 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -30,6 +30,7 @@
#include <osl/diagnose.h>
#include <svx/svdouno.hxx>
#include <editeng/frmdiritem.hxx>
+#include <istype.hxx>
#include <swmodule.hxx>
#include <modcfg.hxx>
#include <com/sun/star/beans/XPropertySet.hpp>
@@ -139,7 +140,7 @@ SdrObject* SwDoc::CloneSdrObj( const SdrObject& rObj, bool
bMoveWithinDoc,
SdrLayerID nLayerIdForClone = rObj.GetLayer();
if ( dynamic_cast<const SwFlyDrawObj*>( pObj) == nullptr &&
dynamic_cast<const SwVirtFlyDrawObj*>( pObj) == nullptr &&
- typeid(SdrObject) != typeid(pObj) )
+ !isType<SdrObject>(pObj) )
{
if ( getIDocumentDrawModelAccess().IsVisibleLayerId( nLayerIdForClone
) )
{
diff --git a/sw/source/uibase/app/docsh2.cxx b/sw/source/uibase/app/docsh2.cxx
index 9e606f0b4095..208848bf9a05 100644
--- a/sw/source/uibase/app/docsh2.cxx
+++ b/sw/source/uibase/app/docsh2.cxx
@@ -66,6 +66,7 @@
#include <basic/basmgr.hxx>
#include <comphelper/classids.hxx>
#include <fmtcol.hxx>
+#include <istype.hxx>
#include <view.hxx>
#include <docsh.hxx>
#include <docary.hxx>
@@ -467,13 +468,13 @@ void SwDocShell::Execute(SfxRequest& rReq)
SfxViewFrame *pTmpFrame = SfxViewFrame::GetFirst(this);
SfxViewShell* pViewShell = SfxViewShell::Current();
SwView* pCurrView = dynamic_cast< SwView *> ( pViewShell );
- bool bCurrent = typeid(SwPagePreview) == typeid( pViewShell );
+ bool bCurrent = isType<SwPagePreview>( pViewShell );
while( pTmpFrame ) // search Preview
{
- if( typeid(SwView) == typeid( pTmpFrame->GetViewShell()) )
+ if( isType<SwView>( pTmpFrame->GetViewShell()) )
bOnly = false;
- else if( typeid(SwPagePreview) == typeid(
pTmpFrame->GetViewShell()))
+ else if( isType<SwPagePreview>( pTmpFrame->GetViewShell()))
{
pTmpFrame->GetFrame().Appear();
bFound = true;
