Kaydet (Commit) 78312099 authored tarafından Noel Grandin's avatar Noel Grandin

new loplugin: convertlong

merge the droplong and convertuintptr into one new plugin.
Limit the analysis to looking at var decl's, since that seems to be
safest proposition, even if that too needs some careful analysis.

Change-Id: Id005baaf05cfb157ce44a06a1c81f08559a07d1f
Reviewed-on: https://gerrit.libreoffice.org/46851Tested-by: 's avatarJenkins <ci@libreoffice.org>
Reviewed-by: 's avatarNoel Grandin <noel.grandin@collabora.co.uk>
üst 24158311
......@@ -56,7 +56,7 @@ static OUString toUNOname( char const * p )
while (*p != 'E')
{
// read chars count
long n = (*p++ - '0');
int n = (*p++ - '0');
while ('0' <= *p && '9' >= *p)
{
n *= 10;
......
......@@ -17,21 +17,27 @@
#include "check.hxx"
/**
plugin to help to when converting code from sal_uIntPtr to something more precise.
*/
namespace {
plugin to help to when converting code from
sal_uIntPtr/sal_uLong/sal_Long/long/unsigned long
class ConvertUIntPtr:
public RecursiveASTVisitor<ConvertUIntPtr>, public loplugin::Plugin
to something more precise.
*/
namespace
{
class ConvertLong : public RecursiveASTVisitor<ConvertLong>, public loplugin::Plugin
{
public:
explicit ConvertUIntPtr(loplugin::InstantiationData const & data):
Plugin(data) {}
explicit ConvertLong(loplugin::InstantiationData const& data)
: Plugin(data)
{
}
virtual void run() override
{
std::string fn( compiler.getSourceManager().getFileEntryForID(
compiler.getSourceManager().getMainFileID())->getName() );
std::string fn(compiler.getSourceManager()
.getFileEntryForID(compiler.getSourceManager().getMainFileID())
->getName());
loplugin::normalizeDotDotInFilePath(fn);
// using sal_uIntPtr as in-between type when converting void* to rtl_TextEncoding
if (fn == SRCDIR "/sal/osl/unx/thread.cxx")
......@@ -47,60 +53,90 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
bool VisitImplicitCastExpr(ImplicitCastExpr const *);
bool VisitVarDecl(VarDecl const*);
bool TraverseFunctionDecl(FunctionDecl*);
private:
bool isIntPtr(QualType qt);
bool isInterestingType(QualType qt);
};
bool ConvertUIntPtr::VisitImplicitCastExpr(ImplicitCastExpr const * castExpr)
bool ConvertLong::TraverseFunctionDecl(FunctionDecl* functionDecl)
{
if (ignoreLocation(castExpr))
// ignore template stuff
if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
{
return true;
}
return RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
}
if (castExpr->getCastKind() == CK_LValueToRValue)
bool ConvertLong::VisitVarDecl(VarDecl const* varDecl)
{
if (ignoreLocation(varDecl))
return true;
if (isa<IntegerLiteral>(castExpr->IgnoreCasts()))
StringRef fileName{ compiler.getSourceManager().getFilename(varDecl->getLocation()) };
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
return true;
// ignore literals like "-123"
if (isa<UnaryOperator>(castExpr->IgnoreCasts()))
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/solar.h"))
return true;
bool isSrcIntPtr = isIntPtr(castExpr->getSubExpr()->getType());
bool isDestIntPtr = isIntPtr(castExpr->getType());
if (!isSrcIntPtr && !isDestIntPtr)
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/o3tl/string_view.hxx"))
return true;
// exclude casting between sal_uIntPtr <-> sal_IntPtr
if (isSrcIntPtr && isDestIntPtr)
if (!varDecl->hasInit())
return true;
if (isSrcIntPtr && loplugin::TypeCheck(castExpr->getType()).AnyBoolean())
if (isa<IntegerLiteral>(varDecl->getInit()->IgnoreParenImpCasts()))
return true;
report(
DiagnosticsEngine::Warning,
"cast from %0 to %1",
castExpr->getExprLoc())
<< castExpr->getSubExpr()->getType()
<< castExpr->getType()
<< castExpr->getSourceRange();
// ignore int x = -1;
if (isa<UnaryOperator>(varDecl->getInit()->IgnoreParenImpCasts()))
return true;
auto lhsType = varDecl->getType();
auto rhsType = varDecl->getInit()->IgnoreParenImpCasts()->getType();
if (lhsType.getLocalUnqualifiedType() == rhsType)
return true;
if (!rhsType.getTypePtrOrNull())
return true;
if (isInterestingType(rhsType))
return true;
if (!isInterestingType(lhsType))
return true;
if (rhsType->isFloatingType()) // TODO
return true;
report(DiagnosticsEngine::Warning, "rather replace type of decl %0 with %1",
varDecl->getLocation())
<< lhsType << rhsType << varDecl->getSourceRange();
//lhsType->dump();
//varDecl->dump();
return true;
}
bool ConvertUIntPtr::isIntPtr(QualType qt)
bool ConvertLong::isInterestingType(QualType qt)
{
auto tc = loplugin::TypeCheck(qt);
if (tc.Typedef())
{
TypedefType const* typedefType = qt->getAs<TypedefType>();
auto name = typedefType->getDecl()->getName();
if (name == "sal_uLong")
return true;
// because this is a typedef to long on 64-bit Linux
if (name == "sal_Int64" || name == "sal_uInt64" || name.find("size_t") != StringRef::npos)
return false;
}
if (isa<AutoType>(qt.getTypePtr()))
return false;
auto unqual = qt.getUnqualifiedType();
if (unqual->isSpecificBuiltinType(BuiltinType::Kind::Long)
|| unqual->isSpecificBuiltinType(BuiltinType::Kind::ULong))
{
return true;
}
if (!tc.Typedef())
return false;
TypedefType const * typedefType = qt->getAs<TypedefType>();
TypedefType const* typedefType = qt->getAs<TypedefType>();
auto name = typedefType->getDecl()->getName();
return name == "sal_uIntPtr" || name == "sal_IntPtr";
}
loplugin::Plugin::Registration< ConvertUIntPtr > X("convertuintptr", false);
loplugin::Plugin::Registration<ConvertLong> X("convertlong", false);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
/*
* 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 <memory>
#include <cassert>
#include <string>
#include <iostream>
#include <fstream>
#include <set>
#include "plugin.hxx"
#include "check.hxx"
/**
The types 'long' and 'unsigned long' are different sizes on different platforms, making them wholly unsuitable
for portable code.
And when I mean different sizes, I mean 64bit Linux and 64bit Windows have different sizes.
*/
namespace {
static bool startswith(const std::string& rStr, const char* pSubStr) {
return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
}
class DropLong:
public RecursiveASTVisitor<DropLong>, public loplugin::Plugin
{
public:
explicit DropLong(loplugin::InstantiationData const & data): Plugin(data) {}
virtual void run() override
{
std::string fn( compiler.getSourceManager().getFileEntryForID(
compiler.getSourceManager().getMainFileID())->getName() );
loplugin::normalizeDotDotInFilePath(fn);
if (startswith(fn, SRCDIR "/sal/"))
return;
if (startswith(fn, SRCDIR "/desktop/unx/"))
return;
if (startswith(fn, SRCDIR "/bridges/"))
return;
if (startswith(fn, SRCDIR "/registry/"))
return;
if (startswith(fn, SRCDIR "/tools/source/generic/fract.cxx"))
return;
if (startswith(fn, SRCDIR "/tools/source/generic/bigint.cxx"))
return;
// TODO figure out how to cope with iterators
if (startswith(fn, SRCDIR "/cppu/source/threadpool/jobqueue.cxx"))
return;
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
bool VisitBinAssign(BinaryOperator const *);
bool VisitVarDecl(VarDecl const *);
bool VisitCastExpr(CastExpr const *);
private:
bool isOK(QualType lhs, QualType rhs);
};
bool DropLong::VisitBinAssign(BinaryOperator const * expr)
{
if (ignoreLocation(expr))
return true;
StringRef fileName { compiler.getSourceManager().getFilename(expr->getExprLoc()) };
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
return true;
auto lhsType = expr->getLHS()->getType();
auto rhsType = expr->getRHS()->IgnoreCasts()->getType();
if (!isOK(lhsType, rhsType))
{
report(
DiagnosticsEngine::Warning,
"rather replace %0 with %1",
expr->getExprLoc())
<< lhsType
<< rhsType
<< expr->getSourceRange();
// lhsType->dump();
}
return true;
}
bool DropLong::VisitVarDecl(VarDecl const * varDecl)
{
if (ignoreLocation(varDecl))
return true;
StringRef fileName { compiler.getSourceManager().getFilename(varDecl->getLocation()) };
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
return true;
if (!varDecl->hasInit())
return true;
auto lhsType = varDecl->getType();
auto rhsType = varDecl->getInit()->IgnoreCasts()->getType();
if (!isOK(lhsType, rhsType))
{
report(
DiagnosticsEngine::Warning,
"rather replace %0 with %1",
varDecl->getLocation())
<< lhsType
<< rhsType
<< varDecl->getSourceRange();
// lhsType->dump();
}
return true;
}
bool DropLong::VisitCastExpr(CastExpr const * castExpr)
{
if (ignoreLocation(castExpr))
return true;
StringRef fileName { compiler.getSourceManager().getFilename(castExpr->getExprLoc()) };
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
return true;
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/sal/types.h"))
return true;
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/rtl/math.hxx"))
return true;
// TODO
if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/helpers.hxx"))
return true;
if (isa<ImplicitCastExpr>(castExpr))
return true;
auto type = castExpr->getType();
if (loplugin::TypeCheck(type).Typedef())
{
TypedefType const * typedefType = type->getAs<TypedefType>();
if (typedefType->getDecl()->getName() == "sal_uLong")
report(
DiagnosticsEngine::Warning,
"sal_uLong cast from %0",
castExpr->getExprLoc())
<< castExpr->getSubExpr()->getType()
<< castExpr->getSourceRange();
}
else if (type->isSpecificBuiltinType(BuiltinType::Kind::Long)
|| type->isSpecificBuiltinType(BuiltinType::Kind::ULong))
{
report(
DiagnosticsEngine::Warning,
"long cast from %0",
castExpr->getExprLoc())
<< castExpr->getSubExpr()->getType()
<< castExpr->getSourceRange();
}
return true;
}
bool DropLong::isOK(QualType lhs, QualType rhs)
{
if (loplugin::TypeCheck(lhs).Typedef())
{
TypedefType const * typedefType = lhs->getAs<TypedefType>();
// Lots of stuff in the standard library and in sal/types.h is
// 'long' on Linux, so just ignore all typedefs.
if (typedefType->getDecl()->getName() != "sal_uLong")
return true;
}
else if (lhs->isSpecificBuiltinType(BuiltinType::Kind::Long)
|| lhs->isSpecificBuiltinType(BuiltinType::Kind::ULong))
{
if (rhs->isSpecificBuiltinType(BuiltinType::Kind::Long)
|| rhs->isSpecificBuiltinType(BuiltinType::Kind::ULong))
return true;
}
else
return true;
if (isa<SubstTemplateTypeParmType>(lhs))
return true;
if (isa<AutoType>(lhs))
return true;
return false;
}
loplugin::Plugin::Registration< DropLong > X("droplong", false);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
......@@ -10,18 +10,32 @@
#include <tools/solar.h>
int main()
{
sal_uIntPtr x = 1;
sal_uInt32 y = x;
y = x;
(void)y;
}
void main2()
{
int x = 1;
int y = 1;
long tmp = x + y; // expected-error {{rather replace 'long' with 'int' [loplugin:droplong]}}
long tmp = x + y;
// expected-error@-1 {{rather replace type of decl 'long' with 'int' [loplugin:convertlong]}}
(void)tmp;
tmp = x + y; // expected-error {{rather replace 'long' with 'int' [loplugin:droplong]}}
tmp = x + y;
sal_uLong tmp1 = x + y; // expected-error-re {{rather replace 'sal_uLong' (aka 'unsigned {{.+}}') with 'int' [loplugin:droplong]}}
sal_uLong tmp1 = x + y;
// expected-error-re@-1 {{rather replace type of decl 'sal_uLong' (aka 'unsigned {{.+}}') with 'int' [loplugin:convertlong]}}
(void)tmp1;
int tmp2 = (sal_uLong)1; // expected-error-re {{sal_uLong cast from 'sal_uLong' (aka 'unsigned {{.+}}') [loplugin:droplong]}}
tmp2 = (long)1; // expected-error {{long cast from 'long' [loplugin:droplong]}}
int tmp2 = (sal_uLong)1;
tmp2 = (long)1;
sal_uIntPtr tmp3 = x + y;
// expected-error-re@-1 {{rather replace type of decl 'sal_uIntPtr' (aka 'unsigned {{.+}}') with 'int' [loplugin:convertlong]}}
(void)tmp3;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
/* -*- 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 <tools/solar.h>
int main()
{
sal_uIntPtr x = 1;
sal_uInt32 y = x; // expected-error-re {{cast from 'sal_uIntPtr' (aka 'unsigned {{.+}}') to 'sal_uInt32' (aka 'unsigned {{.+}}') [loplugin:convertuintptr]}}
y = x; // expected-error-re {{cast from 'sal_uIntPtr' (aka 'unsigned {{.+}}') to 'sal_uInt32' (aka 'unsigned {{.+}}') [loplugin:convertuintptr]}}
(void)y;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
......@@ -351,7 +351,7 @@ void Converter::convertMeasure( OUStringBuffer& rBuffer,
case MeasureUnit::MM_10TH:
case MeasureUnit::MM_100TH:
{
long nFac2 = (MeasureUnit::MM_100TH == nSourceUnit) ? 100 : 10;
int nFac2 = (MeasureUnit::MM_100TH == nSourceUnit) ? 100 : 10;
switch( nTargetUnit )
{
case MeasureUnit::MM_100TH:
......
......@@ -15,11 +15,10 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/casttovoid \
compilerplugins/clang/test/commaoperator \
compilerplugins/clang/test/constparams \
$(if $(filter-out INTEL,$(CPU)),compilerplugins/clang/test/convertuintptr) \
compilerplugins/clang/test/convertlong \
compilerplugins/clang/test/cppunitassertequals \
compilerplugins/clang/test/datamembershadow \
compilerplugins/clang/test/dodgyswitch \
compilerplugins/clang/test/droplong \
compilerplugins/clang/test/externvar \
compilerplugins/clang/test/expressionalwayszero \
compilerplugins/clang/test/faileddyncast \
......
......@@ -1840,12 +1840,10 @@ compilerplugins/clang/test/casttovoid.cxx
compilerplugins/clang/test/commaoperator.cxx
compilerplugins/clang/test/constmethod.cxx
compilerplugins/clang/test/constparams.cxx
compilerplugins/clang/test/convertuintptr.cxx
compilerplugins/clang/test/cppunitassertequals.cxx
compilerplugins/clang/test/cppunitassertequals.hxx
compilerplugins/clang/test/datamembershadow.cxx
compilerplugins/clang/test/dodgyswitch.cxx
compilerplugins/clang/test/droplong.cxx
compilerplugins/clang/test/expressionalwayszero.cxx
compilerplugins/clang/test/externvar.cxx
compilerplugins/clang/test/externvar.hxx
......
......@@ -177,9 +177,9 @@ void SotStorageStream::SetSize(sal_uInt64 const nNewSize)
sal_uInt32 SotStorageStream::GetSize() const
{
sal_uLong nPos = Tell();
sal_uInt64 nPos = Tell();
const_cast<SotStorageStream *>(this)->Seek( STREAM_SEEK_TO_END );
sal_uLong nSize = Tell();
sal_uInt64 nSize = Tell();
const_cast<SotStorageStream *>(this)->Seek( nPos );
return nSize;
}
......@@ -199,7 +199,7 @@ void SotStorageStream::CopyTo( SotStorageStream * pDestStm )
if( !pOwnStm || !pDestStm->pOwnStm )
{
// If Ole2 or not only own StorageStreams
sal_uLong nPos = Tell(); // save position
sal_uInt64 nPos = Tell(); // save position
Seek( 0 );
pDestStm->SetSize( 0 ); // empty target stream
......@@ -497,7 +497,7 @@ bool SotStorage::IsStorageFile( SvStream* pStream )
/** code for new storages must come first! **/
if ( pStream )
{
long nPos = pStream->Tell();
sal_uInt64 nPos = pStream->Tell();
bool bRet = UCBStorage::IsStorageFile( pStream );
if ( !bRet )
bRet = Storage::IsStorageFile( pStream );
......
......@@ -825,7 +825,7 @@ void UCBStorageStream_Impl::CopySourceToTemporary()
// current position of the temporary stream is not changed
if( m_bSourceRead )
{
sal_uLong aPos = m_pStream->Tell();
sal_uInt64 aPos = m_pStream->Tell();
m_pStream->Seek( STREAM_SEEK_TO_END );
ReadSourceWriteTemporary();
m_pStream->Seek( aPos );
......@@ -1010,10 +1010,10 @@ sal_uLong UCBStorageStream_Impl::GetSize()
if( !Init() )
return 0;
sal_uLong nPos = m_pStream->Tell();
sal_uInt64 nPos = m_pStream->Tell();
m_pStream->Seek( STREAM_SEEK_TO_END );
ReadSourceWriteTemporary();
sal_uLong nRet = m_pStream->Tell();
sal_uInt64 nRet = m_pStream->Tell();
m_pStream->Seek( nPos );
return nRet;
......@@ -2884,7 +2884,7 @@ bool UCBStorage::IsStorageFile( SvStream* pFile )
if ( !pFile )
return false;
sal_uLong nPos = pFile->Tell();
sal_uInt64 nPos = pFile->Tell();
pFile->Seek( STREAM_SEEK_TO_END );
if ( pFile->Tell() < 4 )
return false;
......@@ -2914,7 +2914,7 @@ bool UCBStorage::IsStorageFile( SvStream* pFile )
OUString UCBStorage::GetLinkedFile( SvStream &rStream )
{
OUString aString;
sal_uLong nPos = rStream.Tell();
sal_uInt64 nPos = rStream.Tell();
rStream.Seek( STREAM_SEEK_TO_END );
if ( !rStream.Tell() )
return aString;
......
......@@ -708,7 +708,7 @@ void SfxItemSet::SetRanges( const sal_uInt16 *pNewRanges )
}
// create new item-array (by iterating through all new ranges)
sal_uLong nSize = Capacity_Impl(pNewRanges);
sal_uInt16 nSize = Capacity_Impl(pNewRanges);
SfxItemArray aNewItems = new const SfxPoolItem* [ nSize ];
sal_uInt16 nNewCount = 0;
if (m_nCount == 0)
......
......@@ -627,7 +627,7 @@ Color* ImpSvNumberformatScan::GetColor(OUString& sStr)
if ( CharClass::isAsciiNumeric( sString ) )
{
long nIndex = sString.toInt32();
sal_Int32 nIndex = sString.toInt32();
if (nIndex > 0 && nIndex <= 64)
{
pResult = pFormatter->GetUserDefColor((sal_uInt16)nIndex-1);
......
......@@ -1041,7 +1041,7 @@ void Polygon::Optimize( PolyOptimizeFlags nOptimizeFlags )
{
tools::Polygon aNewPoly;
const Point& rFirst = mpImplPolygon->mxPointAry[ 0 ];
const long nReduce = ( nOptimizeFlags & PolyOptimizeFlags::REDUCE ) ? 4 : 0;
const int nReduce = ( nOptimizeFlags & PolyOptimizeFlags::REDUCE ) ? 4 : 0;
while( nSize && ( mpImplPolygon->mxPointAry[ nSize - 1 ] == rFirst ) )
nSize--;
......
......@@ -400,7 +400,7 @@ void ZCodec::UpdateCRC ( sal_uInt8 const * pSource, long nDatSize)
bool ZCodec::AttemptDecompression(SvStream& rIStm, SvStream& rOStm)
{
assert(meState == STATE_INIT);
sal_uLong nStreamPos = rIStm.Tell();
sal_uInt64 nStreamPos = rIStm.Tell();
BeginCompression(ZCODEC_DEFAULT_COMPRESSION, false/*updateCrc*/, true/*gzLib*/);
InitDecompress(rIStm);
EndCompression();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment