[Libreoffice-commits] core.git: compilerplugins/clang dbaccess/source idlc/source sc/source sdext/source sd/source sfx2/source svgio/source svtools/source sw/source vcl/source
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Mon Feb 18 06:16:18 UTC 2019
compilerplugins/clang/simplifybool.cxx | 55 +++++++---
compilerplugins/clang/test/simplifybool.cxx | 30 +++++
dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx | 2
idlc/source/options.cxx | 8 -
sc/source/filter/xml/xmlsubti.cxx | 4
sd/source/core/CustomAnimationEffect.cxx | 2
sd/source/core/stlsheet.cxx | 2
sd/source/ui/dlg/tpoption.cxx | 2
sd/source/ui/slidesorter/controller/SlsCurrentSlideManager.cxx | 2
sd/source/ui/view/DocumentRenderer.cxx | 5
sd/source/ui/view/Outliner.cxx | 2
sd/source/ui/view/ViewShellBase.cxx | 2
sdext/source/presenter/PresenterScrollBar.cxx | 2
sdext/source/presenter/PresenterTextView.cxx | 4
sfx2/source/view/sfxbasecontroller.cxx | 2
svgio/source/svgreader/svgclippathnode.cxx | 2
svgio/source/svgreader/svgmasknode.cxx | 2
svgio/source/svgreader/svgstylenode.cxx | 2
svtools/source/contnr/imivctl1.cxx | 5
sw/source/filter/basflt/shellio.cxx | 4
vcl/source/graphic/GraphicObject2.cxx | 2
21 files changed, 96 insertions(+), 45 deletions(-)
New commits:
commit aa51774e6a309f277e71ca3a3b9d5d5b4b3dbf1a
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Fri Feb 15 12:56:52 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon Feb 18 07:15:57 2019 +0100
loplugin:simplifybool, check for !(!a op !b)
Change-Id: Ic3ee9c05705817580633506498f848aac3ab7ba6
Reviewed-on: https://gerrit.libreoffice.org/67866
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/simplifybool.cxx b/compilerplugins/clang/simplifybool.cxx
index b4752b4108aa..40c5b6fc48ba 100644
--- a/compilerplugins/clang/simplifybool.cxx
+++ b/compilerplugins/clang/simplifybool.cxx
@@ -177,20 +177,47 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
// triggers.
if (compat::getBeginLoc(binaryOp).isMacroID())
return true;
- if (!binaryOp->isComparisonOp())
- return true;
- auto t = binaryOp->getLHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
- if (t->isTemplateTypeParmType() || t->isDependentType() || t->isRecordType())
- return true;
- // for floating point (with NaN) !(x<y) need not be equivalent to x>=y
- if (t->isFloatingType() ||
- binaryOp->getRHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType()->isFloatingType())
- return true;
- report(
- DiagnosticsEngine::Warning,
- ("logical negation of comparison operator, can be simplified by inverting operator"),
- compat::getBeginLoc(expr))
- << expr->getSourceRange();
+ if (binaryOp->isComparisonOp())
+ {
+ auto t = binaryOp->getLHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
+ if (t->isTemplateTypeParmType() || t->isDependentType() || t->isRecordType())
+ return true;
+ // for floating point (with NaN) !(x<y) need not be equivalent to x>=y
+ if (t->isFloatingType() ||
+ binaryOp->getRHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType()->isFloatingType())
+ return true;
+ report(
+ DiagnosticsEngine::Warning,
+ ("logical negation of comparison operator, can be simplified by inverting operator"),
+ compat::getBeginLoc(expr))
+ << expr->getSourceRange();
+ }
+ else if (binaryOp->isLogicalOp())
+ {
+ auto containsNegation = [](Expr const * expr) {
+ expr = ignoreParenImpCastAndComma(expr);
+ if (auto unaryOp = dyn_cast<UnaryOperator>(expr))
+ if (unaryOp->getOpcode() == UO_LNot)
+ return expr;
+ if (auto binaryOp = dyn_cast<BinaryOperator>(expr))
+ if (binaryOp->getOpcode() == BO_NE)
+ return expr;
+ if (auto cxxOpCall = dyn_cast<CXXOperatorCallExpr>(expr))
+ if (cxxOpCall->getOperator() == OO_ExclaimEqual)
+ return expr;
+ return (Expr const*)nullptr;
+ };
+ auto lhs = containsNegation(binaryOp->getLHS());
+ auto rhs = containsNegation(binaryOp->getRHS());
+ if (!lhs || !rhs)
+ return true;
+ if (lhs || rhs)
+ report(
+ DiagnosticsEngine::Warning,
+ ("logical negation of logical op containing negation, can be simplified"),
+ compat::getBeginLoc(binaryOp))
+ << binaryOp->getSourceRange();
+ }
}
if (auto binaryOp = dyn_cast<CXXOperatorCallExpr>(expr->getSubExpr()->IgnoreParenImpCasts())) {
// Ignore macros, otherwise
diff --git a/compilerplugins/clang/test/simplifybool.cxx b/compilerplugins/clang/test/simplifybool.cxx
index 01549f320ab0..64288253c8e2 100644
--- a/compilerplugins/clang/test/simplifybool.cxx
+++ b/compilerplugins/clang/test/simplifybool.cxx
@@ -9,6 +9,8 @@
#include <rtl/ustring.hxx>
+namespace group1
+{
void f1(int a, int b)
{
if (!(a < b))
@@ -25,9 +27,11 @@ void f2(float a, float b)
a = b;
}
};
+};
// Consistently either warn about all or none of the below occurrences of "!!":
-
+namespace group2
+{
enum E1
{
E1_1 = 1
@@ -57,9 +61,11 @@ bool f1(E1 e) { return !!(e & E1_1); }
bool f2(E2 e) { return !!(e & E2_1); }
bool f3(E3 e) { return !!(e & E3::E1); }
+};
// record types
-
+namespace group3
+{
struct Record1
{
bool operator==(const Record1&) const;
@@ -113,5 +119,25 @@ struct Record4
return v;
}
};
+};
+
+namespace group4
+{
+bool foo1(bool a, bool b)
+{
+ return !(!a && !b);
+ // expected-error at -1 {{logical negation of logical op containing negation, can be simplified [loplugin:simplifybool]}}
+}
+bool foo2(int a, bool b)
+{
+ return !(a != 1 && !b);
+ // expected-error at -1 {{logical negation of logical op containing negation, can be simplified [loplugin:simplifybool]}}
+}
+bool foo3(int a, bool b)
+{
+ // no warning expected
+ return !(a != 1 && b);
+}
+};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx b/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
index 070e2ab2814f..766b1dcd2523 100644
--- a/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
+++ b/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
@@ -65,7 +65,7 @@ namespace
{
bool isFieldNameAsterisk(const OUString& _sFieldName )
{
- bool bAsterisk = !(!_sFieldName.isEmpty() && _sFieldName.toChar() != '*');
+ bool bAsterisk = _sFieldName.isEmpty() || _sFieldName.toChar() == '*';
if ( !bAsterisk )
{
sal_Int32 nTokenCount = comphelper::string::getTokenCount(_sFieldName, '.');
diff --git a/idlc/source/options.cxx b/idlc/source/options.cxx
index 04db926dad0d..4f49cd202de4 100644
--- a/idlc/source/options.cxx
+++ b/idlc/source/options.cxx
@@ -227,7 +227,7 @@ bool Options::initOptions(std::vector< std::string > & rArgs)
{
case 'O':
{
- if (!((++first != last) && ((*first)[0] != '-')))
+ if ((++first == last) || ((*first)[0] == '-'))
{
return badOption("invalid", option);
}
@@ -237,7 +237,7 @@ bool Options::initOptions(std::vector< std::string > & rArgs)
}
case 'M':
{
- if (!((++first != last) && ((*first)[0] != '-')))
+ if ((++first == last) || ((*first)[0] == '-'))
{
return badOption("invalid", option);
}
@@ -247,7 +247,7 @@ bool Options::initOptions(std::vector< std::string > & rArgs)
}
case 'I':
{
- if (!((++first != last) && ((*first)[0] != '-')))
+ if ((++first == last) || ((*first)[0] == '-'))
{
return badOption("invalid", option);
}
@@ -283,7 +283,7 @@ bool Options::initOptions(std::vector< std::string > & rArgs)
}
case 'D':
{
- if (!((++first != last) && ((*first)[0] != '-')))
+ if ((++first == last) || ((*first)[0] == '-'))
{
return badOption("invalid", option);
}
diff --git a/sc/source/filter/xml/xmlsubti.cxx b/sc/source/filter/xml/xmlsubti.cxx
index d28a930b2030..1dea576a3e75 100644
--- a/sc/source/filter/xml/xmlsubti.cxx
+++ b/sc/source/filter/xml/xmlsubti.cxx
@@ -243,12 +243,12 @@ uno::Reference< drawing::XShapes > const & ScMyTables::GetCurrentXShapes()
bool ScMyTables::HasDrawPage()
{
- return !((maCurrentCellPos.Tab() != nCurrentDrawPage) || !xDrawPage.is());
+ return (maCurrentCellPos.Tab() == nCurrentDrawPage) && xDrawPage.is();
}
bool ScMyTables::HasXShapes()
{
- return !((maCurrentCellPos.Tab() != nCurrentXShapes) || !xShapes.is());
+ return (maCurrentCellPos.Tab() == nCurrentXShapes) && xShapes.is();
}
void ScMyTables::AddOLE(const uno::Reference <drawing::XShape>& rShape,
diff --git a/sd/source/core/CustomAnimationEffect.cxx b/sd/source/core/CustomAnimationEffect.cxx
index 4bda45524403..418a52b35e05 100644
--- a/sd/source/core/CustomAnimationEffect.cxx
+++ b/sd/source/core/CustomAnimationEffect.cxx
@@ -717,7 +717,7 @@ void CustomAnimationEffect::setTargetSubItem( sal_Int16 nSubItem )
void CustomAnimationEffect::setDuration( double fDuration )
{
- if( !((mfDuration != -1.0) && (mfDuration != fDuration)) )
+ if( (mfDuration == -1.0) || (mfDuration == fDuration) )
return;
try
diff --git a/sd/source/core/stlsheet.cxx b/sd/source/core/stlsheet.cxx
index 83aff4e2fb13..81b465775e8c 100644
--- a/sd/source/core/stlsheet.cxx
+++ b/sd/source/core/stlsheet.cxx
@@ -713,7 +713,7 @@ void SAL_CALL SdStyleSheet::release( ) throw ()
void SAL_CALL SdStyleSheet::dispose( )
{
ClearableMutexGuard aGuard( mrBHelper.rMutex );
- if (!(!mrBHelper.bDisposed && !mrBHelper.bInDispose))
+ if (mrBHelper.bDisposed || mrBHelper.bInDispose)
return;
mrBHelper.bInDispose = true;
diff --git a/sd/source/ui/dlg/tpoption.cxx b/sd/source/ui/dlg/tpoption.cxx
index 84b8563928f0..406c8552d42d 100644
--- a/sd/source/ui/dlg/tpoption.cxx
+++ b/sd/source/ui/dlg/tpoption.cxx
@@ -343,7 +343,7 @@ void SdTpOptionsMisc::ActivatePage( const SfxItemSet& rSet )
SetFieldUnit( *m_pMtrFldOriginalHeight, eFUnit, true );
m_pMtrFldOriginalHeight->SetValue( m_pMtrFldOriginalHeight->Normalize( nVal ), FieldUnit::TWIP );
- if( !(nWidth != 0 && nHeight != 0) )
+ if( nWidth == 0 || nHeight == 0 )
return;
m_pMtrFldInfo1->SetUnit( eFUnit );
diff --git a/sd/source/ui/slidesorter/controller/SlsCurrentSlideManager.cxx b/sd/source/ui/slidesorter/controller/SlsCurrentSlideManager.cxx
index fd926c732638..d3e21aff80bd 100644
--- a/sd/source/ui/slidesorter/controller/SlsCurrentSlideManager.cxx
+++ b/sd/source/ui/slidesorter/controller/SlsCurrentSlideManager.cxx
@@ -120,7 +120,7 @@ void CurrentSlideManager::SwitchCurrentSlide (
const SharedPageDescriptor& rpDescriptor,
const bool bUpdateSelection)
{
- if (!(rpDescriptor.get() != nullptr && mpCurrentSlide!=rpDescriptor))
+ if (rpDescriptor.get() == nullptr || mpCurrentSlide==rpDescriptor)
return;
ReleaseCurrentSlide();
diff --git a/sd/source/ui/view/DocumentRenderer.cxx b/sd/source/ui/view/DocumentRenderer.cxx
index bf23f1a30cdb..149e328e7022 100644
--- a/sd/source/ui/view/DocumentRenderer.cxx
+++ b/sd/source/ui/view/DocumentRenderer.cxx
@@ -1375,9 +1375,8 @@ private:
PrintInfo aInfo (mpPrinter, mpOptions->IsPrintMarkedOnly());
- if (!(aInfo.mpPrinter!=nullptr && pShell!=nullptr))
-return;
-
+ if (aInfo.mpPrinter==nullptr || pShell==nullptr)
+ return;
MapMode aMap (aInfo.mpPrinter->GetMapMode());
aMap.SetMapUnit(MapUnit::Map100thMM);
diff --git a/sd/source/ui/view/Outliner.cxx b/sd/source/ui/view/Outliner.cxx
index 0c98800946b0..e86136bb79a4 100644
--- a/sd/source/ui/view/Outliner.cxx
+++ b/sd/source/ui/view/Outliner.cxx
@@ -1331,7 +1331,7 @@ void SdOutliner::SetViewMode (PageKind ePageKind)
std::shared_ptr<sd::ViewShell> pViewShell (mpWeakViewShell.lock());
std::shared_ptr<sd::DrawViewShell> pDrawViewShell(
std::dynamic_pointer_cast<sd::DrawViewShell>(pViewShell));
- if (!(pDrawViewShell != nullptr && ePageKind != pDrawViewShell->GetPageKind()))
+ if (pDrawViewShell == nullptr || ePageKind == pDrawViewShell->GetPageKind())
return;
// Restore old edit mode.
diff --git a/sd/source/ui/view/ViewShellBase.cxx b/sd/source/ui/view/ViewShellBase.cxx
index 260afc29d8fe..4bc8cc834387 100644
--- a/sd/source/ui/view/ViewShellBase.cxx
+++ b/sd/source/ui/view/ViewShellBase.cxx
@@ -829,7 +829,7 @@ void ViewShellBase::UpdateBorder ( bool bForce /* = false */ )
// We have to check the existence of the window, too.
// The SfxViewFrame accesses the window without checking it.
ViewShell* pMainViewShell = GetMainViewShell().get();
- if (!(pMainViewShell != nullptr && GetWindow()!=nullptr))
+ if (pMainViewShell == nullptr || GetWindow()==nullptr)
return;
SvBorder aCurrentBorder (GetBorderPixel());
diff --git a/sdext/source/presenter/PresenterScrollBar.cxx b/sdext/source/presenter/PresenterScrollBar.cxx
index bd156861c44f..9be3b86776e2 100644
--- a/sdext/source/presenter/PresenterScrollBar.cxx
+++ b/sdext/source/presenter/PresenterScrollBar.cxx
@@ -185,7 +185,7 @@ void PresenterScrollBar::SetThumbPosition (
{
nPosition = ValidateThumbPosition(nPosition);
- if (!(nPosition != mnThumbPosition && ! mbIsNotificationActive))
+ if (nPosition == mnThumbPosition || mbIsNotificationActive)
return;
mnThumbPosition = nPosition;
diff --git a/sdext/source/presenter/PresenterTextView.cxx b/sdext/source/presenter/PresenterTextView.cxx
index 4feb8d92ad21..eb995dd935b3 100644
--- a/sdext/source/presenter/PresenterTextView.cxx
+++ b/sdext/source/presenter/PresenterTextView.cxx
@@ -1096,8 +1096,8 @@ void PresenterTextCaret::SetPosition (
const sal_Int32 nParagraphIndex,
const sal_Int32 nCharacterIndex)
{
- if (!(mnParagraphIndex != nParagraphIndex
- || mnCharacterIndex != nCharacterIndex))
+ if (mnParagraphIndex == nParagraphIndex
+ && mnCharacterIndex == nCharacterIndex)
return;
if (mnParagraphIndex >= 0)
diff --git a/sfx2/source/view/sfxbasecontroller.cxx b/sfx2/source/view/sfxbasecontroller.cxx
index 280529855614..4fdfe8b1a67e 100644
--- a/sfx2/source/view/sfxbasecontroller.cxx
+++ b/sfx2/source/view/sfxbasecontroller.cxx
@@ -1390,7 +1390,7 @@ void SfxBaseController::ShowInfoBars( )
bIsGoogleFile = true;
}
- if ( !(!bCheckedOut && !bIsGoogleFile) )
+ if ( bCheckedOut || bIsGoogleFile )
return;
// Get the Frame and show the InfoBar if not checked out
diff --git a/svgio/source/svgreader/svgclippathnode.cxx b/svgio/source/svgreader/svgclippathnode.cxx
index 090a795da3ac..8d09b51db7ca 100644
--- a/svgio/source/svgreader/svgclippathnode.cxx
+++ b/svgio/source/svgreader/svgclippathnode.cxx
@@ -127,7 +127,7 @@ namespace svgio
drawinglayer::primitive2d::Primitive2DContainer& rContent,
const basegfx::B2DHomMatrix* pTransform) const
{
- if(!(!rContent.empty() && Display_none != getDisplay()))
+ if(rContent.empty() || Display_none == getDisplay())
return;
const drawinglayer::geometry::ViewInformation2D aViewInformation2D;
diff --git a/svgio/source/svgreader/svgmasknode.cxx b/svgio/source/svgreader/svgmasknode.cxx
index f2a919c4582f..286a1088ab60 100644
--- a/svgio/source/svgreader/svgmasknode.cxx
+++ b/svgio/source/svgreader/svgmasknode.cxx
@@ -192,7 +192,7 @@ namespace svgio
drawinglayer::primitive2d::Primitive2DContainer& rTarget,
const basegfx::B2DHomMatrix* pTransform) const
{
- if(!(!rTarget.empty() && Display_none != getDisplay()))
+ if(rTarget.empty() || Display_none == getDisplay())
return;
drawinglayer::primitive2d::Primitive2DContainer aMaskTarget;
diff --git a/svgio/source/svgreader/svgstylenode.cxx b/svgio/source/svgreader/svgstylenode.cxx
index 68a101fb9637..cb81941dbbda 100644
--- a/svgio/source/svgreader/svgstylenode.cxx
+++ b/svgio/source/svgreader/svgstylenode.cxx
@@ -147,7 +147,7 @@ namespace svgio
{
// aSelectors: possible comma-separated list of CssStyle definitions, no spaces at start/end
// aContent: the svg style definitions as string
- if(!(!aSelectors.isEmpty() && !aContent.isEmpty()))
+ if(aSelectors.isEmpty() || aContent.isEmpty())
return;
// create new style and add to local list (for ownership control)
diff --git a/svtools/source/contnr/imivctl1.cxx b/svtools/source/contnr/imivctl1.cxx
index 064f923e425a..c8d689c1a729 100644
--- a/svtools/source/contnr/imivctl1.cxx
+++ b/svtools/source/contnr/imivctl1.cxx
@@ -1979,8 +1979,7 @@ void SvxIconChoiceCtrl_Impl::Command( const CommandEvent& rCEvt )
void SvxIconChoiceCtrl_Impl::ToTop( SvxIconChoiceCtrlEntry* pEntry )
{
- if( !(!maZOrderList.empty()
- && pEntry != maZOrderList.back()))
+ if( maZOrderList.empty() || pEntry == maZOrderList.back())
return;
auto it = std::find(maZOrderList.begin(), maZOrderList.end(), pEntry);
@@ -2698,7 +2697,7 @@ void SvxIconChoiceCtrl_Impl::InitSettings()
pView->SetBackground( rStyleSettings.GetFieldColor());
long nScrBarSize = rStyleSettings.GetScrollBarSize();
- if( !(nScrBarSize != nHorSBarHeight || nScrBarSize != nVerSBarWidth) )
+ if( nScrBarSize == nHorSBarHeight && nScrBarSize == nVerSBarWidth )
return;
nHorSBarHeight = nScrBarSize;
diff --git a/sw/source/filter/basflt/shellio.cxx b/sw/source/filter/basflt/shellio.cxx
index b66cb249ba22..1c0d473cbd4c 100644
--- a/sw/source/filter/basflt/shellio.cxx
+++ b/sw/source/filter/basflt/shellio.cxx
@@ -644,7 +644,7 @@ bool SwReader::HasGlossaries( const Reader& rOptions )
// if a Medium is selected, get its Stream
bool bRet = false;
- if( !( nullptr != (po->m_pMedium = pMedium ) && !po->SetStrmStgPtr() ))
+ if( nullptr == (po->m_pMedium = pMedium ) || po->SetStrmStgPtr() )
bRet = po->HasGlossaries();
return bRet;
}
@@ -660,7 +660,7 @@ bool SwReader::ReadGlossaries( const Reader& rOptions,
// if a Medium is selected, get its Stream
bool bRet = false;
- if( !( nullptr != (po->m_pMedium = pMedium ) && !po->SetStrmStgPtr() ))
+ if( nullptr == (po->m_pMedium = pMedium ) || po->SetStrmStgPtr() )
bRet = po->ReadGlossaries( rBlocks, bSaveRelFiles );
return bRet;
}
diff --git a/vcl/source/graphic/GraphicObject2.cxx b/vcl/source/graphic/GraphicObject2.cxx
index 7148c1098c87..7349e8b25c63 100644
--- a/vcl/source/graphic/GraphicObject2.cxx
+++ b/vcl/source/graphic/GraphicObject2.cxx
@@ -493,7 +493,7 @@ void GraphicObject::ImplTransformBitmap( BitmapEx& rBmpEx,
const Size aSizePixel( rBmpEx.GetSizePixel() );
- if( !(rAttr.GetRotation() != 0 && !IsAnimated()) )
+ if( rAttr.GetRotation() == 0 || IsAnimated() )
return;
if( !(aSizePixel.Width() && aSizePixel.Height() && rDstSize.Width() && rDstSize.Height()) )
More information about the Libreoffice-commits
mailing list