[Libreoffice-commits] core.git: compilerplugins/clang cui/source

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Tue Oct 22 14:58:32 UTC 2019


 compilerplugins/clang/simplifybool.cxx      |   35 ++++++++++++++++++++++++----
 compilerplugins/clang/test/simplifybool.cxx |   10 ++++----
 cui/source/tabpages/tpline.cxx              |    2 -
 3 files changed, 36 insertions(+), 11 deletions(-)

New commits:
commit 54f292d1d199dae36257a1ceb0ff30f32a7e0824
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Oct 22 12:18:34 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Oct 22 16:57:30 2019 +0200

    Avoid C++20 operator overloading ambiguity
    
    ...which is reported now by Clang 10 trunk with -std=c++2a:
    
    > cui/source/tabpages/tpline.cxx:481:80: error: use of overloaded operator '==' is ambiguous (with operand types 'const XLineEndItem' and 'XLineStartItem')
    >             if( pItem && ( !pOld || !( *static_cast<const XLineEndItem*>(pOld) == *pItem ) ) )
    >                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~
    > include/svx/xlnstit.hxx:43:29: note: candidate function (with reversed parameter order)
    >     virtual bool            operator==(const SfxPoolItem& rItem) const override;
    >                             ^
    > include/svx/xlnedit.hxx:43:29: note: candidate function
    >     virtual bool            operator==(const SfxPoolItem& rItem) const override;
    >                             ^
    
    But the base SfxPoolItem::operator == is virtual anyway, so no need to cast
    pOld to a derived type.
    
    And once the expression is changed to
    
      !( *pOld == *pItem )
    
    loplugin:simplifybool would kick in, but only with old compilers.  So update
    loplugin:simplifybool to also kick in on that with latest Clang trunk with
    -std=c++2a, and simplify the expression accordingly.
    
    Change-Id: I3de9175b30d8645ed7a52f87cfac320144576cc8
    Reviewed-on: https://gerrit.libreoffice.org/81203
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/simplifybool.cxx b/compilerplugins/clang/simplifybool.cxx
index 36c4854e96e8..83cbf2dc56f6 100644
--- a/compilerplugins/clang/simplifybool.cxx
+++ b/compilerplugins/clang/simplifybool.cxx
@@ -241,7 +241,30 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
             << expr->getSourceRange();
         return true;
     }
-    if (auto binaryOp = dyn_cast<BinaryOperator>(expr->getSubExpr()->IgnoreParenImpCasts())) {
+    auto sub = expr->getSubExpr()->IgnoreParenImpCasts();
+    auto reversed = false;
+#if CLANG_VERSION >= 100000
+    if (auto const rewritten = dyn_cast<CXXRewrittenBinaryOperator>(sub)) {
+        if (rewritten->isReversed()) {
+            if (rewritten->getOperator() == BO_EQ) {
+                auto const sem = rewritten->getSemanticForm();
+                bool match;
+                if (auto const op1 = dyn_cast<BinaryOperator>(sem)) {
+                    match = op1->getOpcode() == BO_EQ;
+                } else if (auto const op2 = dyn_cast<CXXOperatorCallExpr>(sem)) {
+                    match = op2->getOperator() == OO_EqualEqual;
+                } else {
+                    match = false;
+                }
+                if (match) {
+                    sub = sem;
+                    reversed = true;
+                }
+            }
+        }
+    }
+#endif
+    if (auto binaryOp = dyn_cast<BinaryOperator>(sub)) {
         // Ignore macros, otherwise
         //    OSL_ENSURE(!b, ...);
         // triggers.
@@ -289,7 +312,7 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
                     << binaryOp->getSourceRange();
         }
     }
-    if (auto binaryOp = dyn_cast<CXXOperatorCallExpr>(expr->getSubExpr()->IgnoreParenImpCasts())) {
+    if (auto binaryOp = dyn_cast<CXXOperatorCallExpr>(sub)) {
         // Ignore macros, otherwise
         //    OSL_ENSURE(!b, ...);
         // triggers.
@@ -301,8 +324,8 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
         if (!(op == OO_EqualEqual || op == OO_ExclaimEqual))
             return true;
         BinaryOperator::Opcode negatedOpcode = BinaryOperator::negateComparisonOp(BinaryOperator::getOverloadedOpcode(op));
-        auto lhs = binaryOp->getArg(0)->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
-        auto rhs = binaryOp->getArg(1)->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
+        auto lhs = binaryOp->getArg(reversed ? 1 : 0)->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
+        auto rhs = binaryOp->getArg(reversed ? 0 : 1)->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
         auto const negOp = findOperator(compiler, negatedOpcode, lhs, rhs);
         if (!negOp)
             return true;
@@ -323,8 +346,10 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
             << expr->getSourceRange();
         if (negOp != ASSUME_OPERATOR_EXISTS)
             report(
-                DiagnosticsEngine::Note, "the presumed corresponding negated operator is declared here",
+                DiagnosticsEngine::Note, "the presumed corresponding negated operator for %0 and %1 is declared here",
                 negOp->getLocation())
+                << binaryOp->getArg(reversed ? 1 : 0)->IgnoreImpCasts()->getType()
+                << binaryOp->getArg(reversed ? 0 : 1)->IgnoreImpCasts()->getType()
                 << negOp->getSourceRange();
     }
     return true;
diff --git a/compilerplugins/clang/test/simplifybool.cxx b/compilerplugins/clang/test/simplifybool.cxx
index 75a26d22aa27..906feabee96f 100644
--- a/compilerplugins/clang/test/simplifybool.cxx
+++ b/compilerplugins/clang/test/simplifybool.cxx
@@ -8,11 +8,11 @@
  */
 
 #include <rtl/ustring.hxx>
-// expected-note at rtl/ustring.hxx:* 2 {{the presumed corresponding negated operator is declared here [loplugin:simplifybool]}}
+// expected-note at rtl/ustring.hxx:* 2 {{the presumed corresponding negated operator for 'rtl::OUString' and 'rtl::OUString' is declared here [loplugin:simplifybool]}}
 #include <rtl/string.hxx>
-// expected-note at rtl/string.hxx:* {{the presumed corresponding negated operator is declared here [loplugin:simplifybool]}}
+// expected-note at rtl/string.hxx:* {{the presumed corresponding negated operator for 'rtl::OString' and 'rtl::OString' is declared here [loplugin:simplifybool]}}
 #include <basegfx/vector/b3dvector.hxx>
-// expected-note at basegfx/tuple/b3dtuple.hxx:* {{the presumed corresponding negated operator is declared here [loplugin:simplifybool]}}
+// expected-note at basegfx/tuple/b3dtuple.hxx:* {{the presumed corresponding negated operator for 'basegfx::B3DVector' and 'basegfx::B3DVector' is declared here [loplugin:simplifybool]}}
 
 #include <map>
 
@@ -82,7 +82,7 @@ struct Record2
 {
     bool operator==(const Record2&) const;
     bool operator!=(const Record2&) const;
-    // expected-note at -1 {{the presumed corresponding negated operator is declared here [loplugin:simplifybool]}}
+    // expected-note at -1 {{the presumed corresponding negated operator for 'group3::Record2' and 'group3::Record2' is declared here [loplugin:simplifybool]}}
 };
 
 struct Record3
@@ -91,7 +91,7 @@ struct Record3
 
 bool operator==(const Record3&, const Record3&);
 bool operator!=(const Record3&, const Record3&);
-// expected-note at -1 {{the presumed corresponding negated operator is declared here [loplugin:simplifybool]}}
+// expected-note at -1 {{the presumed corresponding negated operator for 'group3::Record3' and 'group3::Record3' is declared here [loplugin:simplifybool]}}
 
 void testRecord()
 {
diff --git a/cui/source/tabpages/tpline.cxx b/cui/source/tabpages/tpline.cxx
index 3cc694a7ebc6..89fc0dc227ef 100644
--- a/cui/source/tabpages/tpline.cxx
+++ b/cui/source/tabpages/tpline.cxx
@@ -478,7 +478,7 @@ bool SvxLineTabPage::FillItemSet( SfxItemSet* rAttrs )
             else if( m_pLineEndList->Count() > static_cast<long>( nPos - 1 ) )
                 pItem.reset(new XLineStartItem( m_xLbStartStyle->get_active_text(), m_pLineEndList->GetLineEnd( nPos - 1 )->GetLineEnd() ));
             pOld = GetOldItem( *rAttrs, XATTR_LINESTART );
-            if( pItem && ( !pOld || !( *static_cast<const XLineEndItem*>(pOld) == *pItem ) ) )
+            if( pItem && ( !pOld || *pOld != *pItem ) )
             {
                 rAttrs->Put( *pItem );
                 bModified = true;


More information about the Libreoffice-commits mailing list