[Libreoffice-commits] core.git: 2 commits - compilerplugins/clang include/svx include/tools sd/source
Noel Grandin
noel.grandin at collabora.co.uk
Sat Feb 10 05:54:41 UTC 2018
compilerplugins/clang/flatten.cxx | 35 -----------
compilerplugins/clang/plugin.cxx | 17 ++++-
compilerplugins/clang/plugin.hxx | 1
compilerplugins/clang/pluginhandler.cxx | 17 ++++-
compilerplugins/clang/pluginhandler.hxx | 7 +-
compilerplugins/clang/salcall.cxx | 25 -------
include/svx/unoshape.hxx | 4 +
include/tools/gen.hxx | 47 ++++++++++++---
sd/source/ui/accessibility/AccessibleSlideSorterView.cxx | 6 -
sd/source/ui/slidesorter/inc/view/SlideSorterView.hxx | 2
sd/source/ui/slidesorter/view/SlideSorterView.cxx | 2
11 files changed, 81 insertions(+), 82 deletions(-)
New commits:
commit 94ab8e4360a2a7a932656e99f718244321d0f923
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Fri Feb 9 15:28:41 2018 +0200
improve loplugin rewriter double source modification detection
because my new rewriter easily generates overlapping rewriting.
Move the code from flatten and salcall up into the pluginhandler, and
drop the simpler detection logic.
Change-Id: I3da51ac510954a5d4276cee0924cc5dc1fc9a734
Reviewed-on: https://gerrit.libreoffice.org/49493
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/flatten.cxx b/compilerplugins/clang/flatten.cxx
index 88156201c3ae..dd116d7a4ea5 100644
--- a/compilerplugins/clang/flatten.cxx
+++ b/compilerplugins/clang/flatten.cxx
@@ -49,13 +49,11 @@ private:
SourceRange extendOverComments(SourceRange range);
std::string getSourceAsString(SourceRange range);
std::string invertCondition(Expr const * condExpr, SourceRange conditionRange);
- bool checkOverlap(SourceRange);
bool isLargeCompoundStmt(Stmt const *);
Stmt const * lastStmtInCompoundStmt = nullptr;
FunctionDecl const * functionDecl = nullptr;
CompoundStmt const * functionDeclBody = nullptr;
- std::vector<std::pair<char const *, char const *>> mvModifiedRanges;
Stmt const * mElseBranch = nullptr;
};
@@ -283,11 +281,6 @@ bool Flatten::rewrite1(IfStmt const * ifStmt)
if (!rewriter)
return false;
- // If we overlap with a previous area we modified, we cannot perform this change
- // without corrupting the source
- if (!checkOverlap(ifStmt->getSourceRange()))
- return false;
-
auto conditionRange = ignoreMacroExpansions(ifStmt->getCond()->getSourceRange());
if (!conditionRange.isValid()) {
return false;
@@ -341,11 +334,6 @@ bool Flatten::rewrite2(IfStmt const * ifStmt)
if (!rewriter)
return false;
- // If we overlap with a previous area we modified, we cannot perform this change
- // without corrupting the source
- if (!checkOverlap(ifStmt->getSourceRange()))
- return false;
-
auto conditionRange = ignoreMacroExpansions(ifStmt->getCond()->getSourceRange());
if (!conditionRange.isValid()) {
return false;
@@ -388,11 +376,6 @@ bool Flatten::rewriteLargeIf(IfStmt const * ifStmt)
if (!rewriter)
return false;
- // If we overlap with a previous area we modified, we cannot perform this change
- // without corrupting the source
- if (!checkOverlap(ifStmt->getSourceRange()))
- return false;
-
auto conditionRange = ignoreMacroExpansions(ifStmt->getCond()->getSourceRange());
if (!conditionRange.isValid()) {
return false;
@@ -428,24 +411,6 @@ bool Flatten::rewriteLargeIf(IfStmt const * ifStmt)
return true;
}
-// If we overlap with a previous area we modified, we cannot perform this change
-// without corrupting the source
-bool Flatten::checkOverlap(SourceRange range)
-{
- SourceManager& SM = compiler.getSourceManager();
- char const *p1 = SM.getCharacterData( range.getBegin() );
- char const *p2 = SM.getCharacterData( range.getEnd() );
- for (std::pair<char const *, char const *> const & rPair : mvModifiedRanges)
- {
- if (rPair.first <= p1 && p1 <= rPair.second)
- return false;
- if (p1 <= rPair.second && rPair.first <= p2)
- return false;
- }
- mvModifiedRanges.emplace_back(p1, p2);
- return true;
-}
-
std::string Flatten::invertCondition(Expr const * condExpr, SourceRange conditionRange)
{
std::string s = getSourceAsString(conditionRange);
diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 2e9e390b6714..ef2b7667de85 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -399,6 +399,7 @@ bool RewritePlugin::insertText( SourceLocation Loc, StringRef Str, bool InsertAf
return false;
if( rewriter->InsertText( Loc, Str, InsertAfter, indentNewLines ))
return reportEditFailure( Loc );
+ handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
return true;
}
@@ -409,6 +410,7 @@ bool RewritePlugin::insertTextAfter( SourceLocation Loc, StringRef Str )
return false;
if( rewriter->InsertTextAfter( Loc, Str ))
return reportEditFailure( Loc );
+ handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
return true;
}
@@ -419,6 +421,7 @@ bool RewritePlugin::insertTextAfterToken( SourceLocation Loc, StringRef Str )
return false;
if( rewriter->InsertTextAfterToken( Loc, Str ))
return reportEditFailure( Loc );
+ handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
return true;
}
@@ -429,6 +432,7 @@ bool RewritePlugin::insertTextBefore( SourceLocation Loc, StringRef Str )
return false;
if( rewriter->InsertTextBefore( Loc, Str ))
return reportEditFailure( Loc );
+ handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
return true;
}
@@ -450,7 +454,7 @@ bool RewritePlugin::removeText( CharSourceRange range, RewriteOptions opts )
return false;
if( rewriter->getRangeSize( range, opts ) == -1 )
return reportEditFailure( range.getBegin());
- if( !handler.addRemoval( range.getBegin() ) )
+ if( !handler.checkOverlap( range.getAsRange() ) )
{
report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", range.getBegin());
return true;
@@ -462,6 +466,7 @@ bool RewritePlugin::removeText( CharSourceRange range, RewriteOptions opts )
}
if( rewriter->RemoveText( range, opts ))
return reportEditFailure( range.getBegin());
+ handler.addSourceModification(range.getAsRange());
return true;
}
@@ -511,13 +516,15 @@ bool RewritePlugin::replaceText( SourceLocation Start, unsigned OrigLength, Stri
assert( rewriter );
if (wouldRewriteWorkdir(Start))
return false;
- if( OrigLength != 0 && !handler.addRemoval( Start ) )
+ SourceRange Range(Start, Start.getLocWithOffset(std::max<size_t>(OrigLength, NewStr.size())));
+ if( OrigLength != 0 && !handler.checkOverlap( Range ) )
{
report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", Start );
return true;
}
if( rewriter->ReplaceText( Start, OrigLength, NewStr ))
return reportEditFailure( Start );
+ handler.addSourceModification(Range);
return true;
}
@@ -528,13 +535,14 @@ bool RewritePlugin::replaceText( SourceRange range, StringRef NewStr )
return false;
if( rewriter->getRangeSize( range ) == -1 )
return reportEditFailure( range.getBegin());
- if( !handler.addRemoval( range.getBegin() ) )
+ if( !handler.checkOverlap( range ) )
{
report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
return true;
}
if( rewriter->ReplaceText( range, NewStr ))
return reportEditFailure( range.getBegin());
+ handler.addSourceModification(range);
return true;
}
@@ -545,13 +553,14 @@ bool RewritePlugin::replaceText( SourceRange range, SourceRange replacementRange
return false;
if( rewriter->getRangeSize( range ) == -1 )
return reportEditFailure( range.getBegin());
- if( !handler.addRemoval( range.getBegin() ) )
+ if( !handler.checkOverlap( range ) )
{
report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
return true;
}
if( rewriter->ReplaceText( range, replacementRange ))
return reportEditFailure( range.getBegin());
+ handler.addSourceModification(range);
return true;
}
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index c6f1c7cbd6e8..95983c00060d 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -21,6 +21,7 @@
#include <clang/Frontend/CompilerInstance.h>
#include <clang/Lex/Preprocessor.h>
#include <unordered_map>
+#include <vector>
#include <clang/Rewrite/Core/Rewriter.h>
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index 150bc4d1ef4d..3e78030993be 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -236,9 +236,22 @@ bool PluginHandler::checkIgnoreLocation(SourceLocation loc)
return true;
}
-bool PluginHandler::addRemoval( SourceLocation loc )
+// If we overlap with a previous area we modified, we cannot perform this change
+// without corrupting the source
+bool PluginHandler::checkOverlap(SourceRange range)
{
- return removals.insert( loc ).second;
+ SourceManager& SM = compiler.getSourceManager();
+ char const *p1 = SM.getCharacterData( range.getBegin() );
+ char const *p2 = SM.getCharacterData( range.getEnd() );
+ for (std::pair<char const *, char const *> const & rPair : mvModifiedRanges)
+ {
+ if (rPair.first <= p1 && p1 <= rPair.second)
+ return false;
+ if (p1 <= rPair.second && rPair.first <= p2)
+ return false;
+ }
+ mvModifiedRanges.emplace_back(p1, p2);
+ return true;
}
void PluginHandler::HandleTranslationUnit( ASTContext& context )
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index cb75f9443bb5..05e8ce3502c7 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -54,10 +54,13 @@ public:
DiagnosticBuilder report( DiagnosticsEngine::Level level, const char * plugin, StringRef message,
CompilerInstance& compiler, SourceLocation loc = SourceLocation());
bool ignoreLocation(SourceLocation loc);
- bool addRemoval( SourceLocation loc );
bool isDebugMode() const { return debugMode; }
bool isLOOLMode() const { return !loolBasePath.empty(); }
static bool isUnitTestMode();
+ // If we overlap with a previous area we modified, we cannot perform this change
+ // without corrupting the source
+ bool checkOverlap(SourceRange range);
+ bool addSourceModification(SourceRange range);
private:
void handleOption( const std::string& option );
void createPlugins( std::set< std::string > rewriters );
@@ -67,12 +70,12 @@ private:
StringRef const mainFileName;
std::unordered_map<SourceLocation, bool> ignored_;
Rewriter rewriter;
- std::set< SourceLocation > removals;
std::string scope;
std::string warningsOnly;
std::string loolBasePath;
bool warningsAsErrors;
bool debugMode = false;
+ std::vector<std::pair<char const*, char const*>> mvModifiedRanges;
};
/**
diff --git a/compilerplugins/clang/salcall.cxx b/compilerplugins/clang/salcall.cxx
index 2c33008ae276..2f289033851d 100644
--- a/compilerplugins/clang/salcall.cxx
+++ b/compilerplugins/clang/salcall.cxx
@@ -77,7 +77,6 @@ public:
private:
void checkForFunctionDecl(Expr const*, bool bCheckOnly = false);
bool rewrite(SourceLocation);
- bool checkOverlap(SourceRange);
bool isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation* pLoc = nullptr);
std::set<FunctionDecl const*> m_addressOfSet;
@@ -87,7 +86,6 @@ private:
Warning
};
PluginPhase m_phase;
- std::vector<std::pair<char const*, char const*>> mvModifiedRanges;
};
bool SalCall::VisitUnaryAddrOf(UnaryOperator const* op)
@@ -662,35 +660,12 @@ bool SalCall::rewrite(SourceLocation locBegin)
SourceRange range(locBegin, locEnd);
- // If we overlap with a previous area we modified, we cannot perform this change
- // without corrupting the source
- if (!checkOverlap(range))
- return false;
-
if (!replaceText(locBegin, 9, ""))
return false;
return true;
}
-// If we overlap with a previous area we modified, we cannot perform this change
-// without corrupting the source
-bool SalCall::checkOverlap(SourceRange range)
-{
- SourceManager& SM = compiler.getSourceManager();
- char const* p1 = SM.getCharacterData(range.getBegin());
- char const* p2 = SM.getCharacterData(range.getEnd());
- for (std::pair<char const*, char const*> const& rPair : mvModifiedRanges)
- {
- if (rPair.first <= p1 && p1 <= rPair.second)
- return false;
- if (p1 <= rPair.second && rPair.first <= p2)
- return false;
- }
- mvModifiedRanges.emplace_back(p1, p2);
- return true;
-}
-
static loplugin::Plugin::Registration<SalCall> reg("salcall", true);
}
commit 5853b0b25d439caa619cac2edd9853ac76f84217
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Fri Feb 9 10:49:54 2018 +0200
make Pair protected base in Pair/Size/Selection
as part of cleaning up the use of the non-const-ref returning methods.
i.e. methods like
long& X()
And make the classes final.
Change-Id: Ice0aa1932124e77f5ed672b527c2a092ec80c481
Reviewed-on: https://gerrit.libreoffice.org/49475
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/include/svx/unoshape.hxx b/include/svx/unoshape.hxx
index eec731fbe971..11334b4d4940 100644
--- a/include/svx/unoshape.hxx
+++ b/include/svx/unoshape.hxx
@@ -141,7 +141,11 @@ protected:
SdrModel* mpModel;
// translations for writer, which works in TWIPS
void ForceMetricToItemPoolMetric(Pair& rPoint) const throw();
+ void ForceMetricToItemPoolMetric(Point& rPoint) const throw() { ForceMetricToItemPoolMetric(rPoint.toPair()); }
+ void ForceMetricToItemPoolMetric(Size& rPoint) const throw() { ForceMetricToItemPoolMetric(rPoint.toPair()); }
void ForceMetricTo100th_mm(Pair& rPoint) const throw();
+ void ForceMetricTo100th_mm(Point& rPoint) const throw() { ForceMetricTo100th_mm(rPoint.toPair()); }
+ void ForceMetricTo100th_mm(Size& rPoint) const throw() { ForceMetricTo100th_mm(rPoint.toPair()); }
// Dimension arrows change size/position on save/reload (#i59051#)
void ForceMetricToItemPoolMetric(basegfx::B2DPolyPolygon& rPolyPolygon) const throw();
void ForceMetricTo100th_mm(basegfx::B2DPolyPolygon& rPolyPolygon) const throw();
diff --git a/include/tools/gen.hxx b/include/tools/gen.hxx
index 1fb1a283a60e..87e1e54b3eed 100644
--- a/include/tools/gen.hxx
+++ b/include/tools/gen.hxx
@@ -69,7 +69,7 @@ inline bool equal(Pair const & p1, Pair const & p2)
// Point
-class SAL_WARN_UNUSED SAL_DLLPUBLIC_EXPORT Point : public Pair
+class SAL_WARN_UNUSED SAL_DLLPUBLIC_EXPORT Point final : protected Pair
{
public:
Point() {}
@@ -100,6 +100,11 @@ public:
long getY() const { return Y(); }
void setX(long nX) { X() = nX; }
void setY(long nY) { Y() = nY; }
+
+ Pair const & toPair() const { return *this; }
+ Pair & toPair() { return *this; }
+
+ using Pair::toString;
};
inline void Point::Move( long nHorzMove, long nVertMove )
@@ -158,7 +163,7 @@ inline Point operator/( const Point &rVal1, const long nVal2 )
inline bool operator ==(Point const & p1, Point const & p2)
{
- return tools::detail::equal(p1, p2);
+ return tools::detail::equal(p1.toPair(), p2.toPair());
}
inline bool operator !=(Point const & p1, Point const & p2)
@@ -175,7 +180,7 @@ inline std::basic_ostream<charT, traits> & operator <<(
// Size
-class SAL_WARN_UNUSED Size : public Pair
+class SAL_WARN_UNUSED Size final : protected Pair
{
public:
Size() {}
@@ -191,11 +196,16 @@ public:
long getHeight() const { return Height(); }
void setWidth(long nWidth) { Width() = nWidth; }
void setHeight(long nHeight) { Height() = nHeight; }
+
+ Pair const & toPair() const { return *this; }
+ Pair & toPair() { return *this; }
+
+ using Pair::toString;
};
inline bool operator ==(Size const & s1, Size const & s2)
{
- return tools::detail::equal(s1, s2);
+ return tools::detail::equal(s1.toPair(), s2.toPair());
}
inline bool operator !=(Size const & s1, Size const & s2)
@@ -214,7 +224,7 @@ inline std::basic_ostream<charT, traits> & operator <<(
#define RANGE_MAX LONG_MAX
-class SAL_WARN_UNUSED Range : public Pair
+class SAL_WARN_UNUSED Range final : protected Pair
{
public:
Range() {}
@@ -230,6 +240,11 @@ public:
bool IsInside( long nIs ) const;
void Justify();
+
+ Pair const & toPair() const { return *this; }
+ Pair & toPair() { return *this; }
+
+ using Pair::toString;
};
inline bool Range::IsInside( long nIs ) const
@@ -249,7 +264,7 @@ inline void Range::Justify()
inline bool operator ==(Range const & r1, Range const & r2)
{
- return tools::detail::equal(r1, r2);
+ return tools::detail::equal(r1.toPair(), r2.toPair());
}
inline bool operator !=(Range const & r1, Range const & r2)
@@ -269,7 +284,7 @@ inline std::basic_ostream<charT, traits> & operator <<(
#define SELECTION_MIN LONG_MIN
#define SELECTION_MAX LONG_MAX
-class SAL_WARN_UNUSED Selection : public Pair
+class SAL_WARN_UNUSED Selection final : protected Pair
{
public:
Selection() {}
@@ -292,6 +307,11 @@ public:
long getMin() const { return Min(); }
void setMin(long nMin) { Min() = nMin; }
void setMax(long nMax) { Max() = nMax; }
+
+ Pair const & toPair() const { return *this; }
+ Pair & toPair() { return *this; }
+
+ using Pair::toString;
};
inline bool Selection::IsInside( long nIs ) const
@@ -311,7 +331,7 @@ inline void Selection::Justify()
inline bool operator ==(Selection const & s1, Selection const & s2)
{
- return tools::detail::equal(s1, s2);
+ return tools::detail::equal(s1.toPair(), s2.toPair());
}
inline bool operator !=(Selection const & s1, Selection const & s2)
@@ -341,7 +361,7 @@ inline std::basic_ostream<charT, traits> & operator <<(
/// Ok, now is the time for despair.
namespace tools
{
-class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Rectangle
+class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Rectangle final
{
static constexpr short RECT_EMPTY = -32767;
public:
@@ -714,6 +734,15 @@ inline std::basic_ostream<charT, traits> & operator <<(
<< "@(" << rectangle.getX() << ',' << rectangle.getY() << ")";
}
+inline SvStream& ReadPair( SvStream& rIStream, Point& v ) { return ReadPair(rIStream, v.toPair()); }
+inline SvStream& WritePair( SvStream& rOStream, const Point& v ) { return WritePair(rOStream, v.toPair()); }
+inline SvStream& ReadPair( SvStream& rIStream, Size& v ) { return ReadPair(rIStream, v.toPair()); }
+inline SvStream& WritePair( SvStream& rOStream, const Size& v ) { return WritePair(rOStream, v.toPair()); }
+inline SvStream& ReadPair( SvStream& rIStream, Range& v ) { return ReadPair(rIStream, v.toPair()); }
+inline SvStream& WritePair( SvStream& rOStream, const Range& v ) { return WritePair(rOStream, v.toPair()); }
+inline SvStream& ReadPair( SvStream& rIStream, Selection& v ) { return ReadPair(rIStream, v.toPair()); }
+inline SvStream& WritePair( SvStream& rOStream, const Selection& v ) { return WritePair(rOStream, v.toPair()); }
+
#endif
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sd/source/ui/accessibility/AccessibleSlideSorterView.cxx b/sd/source/ui/accessibility/AccessibleSlideSorterView.cxx
index 85c65293d485..d070bcd14a6d 100644
--- a/sd/source/ui/accessibility/AccessibleSlideSorterView.cxx
+++ b/sd/source/ui/accessibility/AccessibleSlideSorterView.cxx
@@ -672,9 +672,9 @@ void AccessibleSlideSorterView::Implementation::UpdateChildren()
return;
}
- const Pair aRange (mrSlideSorter.GetView().GetVisiblePageRange());
- mnFirstVisibleChild = aRange.A();
- mnLastVisibleChild = aRange.B();
+ const Range aRange (mrSlideSorter.GetView().GetVisiblePageRange());
+ mnFirstVisibleChild = aRange.Min();
+ mnLastVisibleChild = aRange.Max();
// Release all children.
Clear();
diff --git a/sd/source/ui/slidesorter/inc/view/SlideSorterView.hxx b/sd/source/ui/slidesorter/inc/view/SlideSorterView.hxx
index a8349967e801..8da2748ef70d 100644
--- a/sd/source/ui/slidesorter/inc/view/SlideSorterView.hxx
+++ b/sd/source/ui/slidesorter/inc/view/SlideSorterView.hxx
@@ -152,7 +152,7 @@ public:
The returned pair of page object indices is empty when the
second index is lower than the first.
*/
- Pair const & GetVisiblePageRange();
+ Range const & GetVisiblePageRange();
/** Add a shape to the page. Typically used from inside
PostModelChange().
diff --git a/sd/source/ui/slidesorter/view/SlideSorterView.cxx b/sd/source/ui/slidesorter/view/SlideSorterView.cxx
index 0a4998df6f22..529c51413a8a 100644
--- a/sd/source/ui/slidesorter/view/SlideSorterView.cxx
+++ b/sd/source/ui/slidesorter/view/SlideSorterView.cxx
@@ -703,7 +703,7 @@ std::shared_ptr<cache::PageCache> const & SlideSorterView::GetPreviewCache()
return mpPreviewCache;
}
-Pair const & SlideSorterView::GetVisiblePageRange()
+Range const & SlideSorterView::GetVisiblePageRange()
{
if ( ! mbPageObjectVisibilitiesValid)
DeterminePageObjectVisibilities();
More information about the Libreoffice-commits
mailing list