[Libreoffice-commits] core.git: accessibility/source basctl/source basic/source compilerplugins/clang desktop/source
Noel Grandin
noel.grandin at collabora.co.uk
Tue Nov 8 06:12:15 UTC 2016
accessibility/source/extended/accessiblelistboxentry.cxx | 6
basctl/source/basicide/localizationmgr.cxx | 7
basctl/source/dlged/dlged.cxx | 3
basic/source/classes/sbunoobj.cxx | 7
compilerplugins/clang/store/oncevar.cxx | 141 ++++++++-------
desktop/source/app/app.cxx | 4
6 files changed, 89 insertions(+), 79 deletions(-)
New commits:
commit 8955c3fde60b0621527e4b91576e49778494f926
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Mon Nov 7 15:59:37 2016 +0200
loplugin:oncevar
Change-Id: I44fb6858eeff14fcbd9fdfbbb0aabd1433b6a27d
Reviewed-on: https://gerrit.libreoffice.org/30668
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/accessibility/source/extended/accessiblelistboxentry.cxx b/accessibility/source/extended/accessiblelistboxentry.cxx
index 39ece7a..ab9b216 100644
--- a/accessibility/source/extended/accessiblelistboxentry.cxx
+++ b/accessibility/source/extended/accessiblelistboxentry.cxx
@@ -802,8 +802,6 @@ namespace accessibility
checkActionIndex_Impl( nIndex );
EnsureIsAlive();
- static const char sActionDesc1[] = "Check";
- static const char sActionDesc2[] = "UnCheck";
// sal_Bool bHasButtons = (getListBox()->GetStyle() & WB_HASBUTTONS)!=0;
SvTreeListEntry* pEntry = getListBox()->GetEntryFromPath( m_aEntryPath );
SvButtonState state = getListBox()->GetCheckButtonState( pEntry );
@@ -813,9 +811,9 @@ namespace accessibility
if(getAccessibleRole() == AccessibleRole::CHECK_BOX)
{
if ( state == SvButtonState::Checked )
- return OUString(sActionDesc2);
+ return OUString("UnCheck");
else if (state == SvButtonState::Unchecked)
- return OUString(sActionDesc1);
+ return OUString("Check");
}
else
{
diff --git a/basctl/source/basicide/localizationmgr.cxx b/basctl/source/basicide/localizationmgr.cxx
index e9dbf62..6ee7bdd 100644
--- a/basctl/source/basicide/localizationmgr.cxx
+++ b/basctl/source/basicide/localizationmgr.cxx
@@ -68,7 +68,6 @@ bool LocalizationMgr::isLibraryLocalized ()
void LocalizationMgr::handleTranslationbar ()
{
- static const char aLayoutManagerName[] = "LayoutManager";
static const char aToolBarResName[] = "private:resource/toolbar/translationbar";
Reference< beans::XPropertySet > xFrameProps
@@ -76,7 +75,7 @@ void LocalizationMgr::handleTranslationbar ()
if ( xFrameProps.is() )
{
Reference< css::frame::XLayoutManager > xLayoutManager;
- uno::Any a = xFrameProps->getPropertyValue( aLayoutManagerName );
+ uno::Any a = xFrameProps->getPropertyValue( "LayoutManager" );
a >>= xLayoutManager;
if ( xLayoutManager.is() )
{
@@ -884,8 +883,6 @@ void LocalizationMgr::deleteControlResourceIDsForDeletedEditorObject( DlgEditor*
void LocalizationMgr::setStringResourceAtDialog( const ScriptDocument& rDocument, const OUString& aLibName,
const OUString& aDlgName, const Reference< container::XNameContainer >& xDialogModel )
{
- static const char aResourceResolverPropName[] = "ResourceResolver";
-
// Get library
Reference< container::XNameContainer > xDialogLib( rDocument.getLibrary( E_DIALOGS, aLibName, true ) );
Reference< XStringResourceManager > xStringResourceManager =
@@ -907,7 +904,7 @@ void LocalizationMgr::setStringResourceAtDialog( const ScriptDocument& rDocument
}
Reference< beans::XPropertySet > xDlgPSet( xDialogModel, UNO_QUERY );
- xDlgPSet->setPropertyValue( aResourceResolverPropName, Any(xStringResourceManager) );
+ xDlgPSet->setPropertyValue( "ResourceResolver", Any(xStringResourceManager) );
}
}
diff --git a/basctl/source/dlged/dlged.cxx b/basctl/source/dlged/dlged.cxx
index 09fb336..9d8b0ad 100644
--- a/basctl/source/dlged/dlged.cxx
+++ b/basctl/source/dlged/dlged.cxx
@@ -59,7 +59,6 @@ using namespace ::com::sun::star::io;
static const char aResourceResolverPropName[] = "ResourceResolver";
static const char aDecorationPropName[] = "Decoration";
-static const char aTitlePropName[] = "Title";
// DlgEdHint
@@ -124,7 +123,7 @@ void DlgEditor::ShowDialog()
if( !bDecoration )
{
xNewDlgModPropSet->setPropertyValue( aDecorationPropName, makeAny( true ) );
- xNewDlgModPropSet->setPropertyValue( aTitlePropName, makeAny( OUString() ) );
+ xNewDlgModPropSet->setPropertyValue( "Title", makeAny( OUString() ) );
}
}
catch(const UnknownPropertyException& )
diff --git a/basic/source/classes/sbunoobj.cxx b/basic/source/classes/sbunoobj.cxx
index 2a73adf..d4ccf6d 100644
--- a/basic/source/classes/sbunoobj.cxx
+++ b/basic/source/classes/sbunoobj.cxx
@@ -106,7 +106,6 @@ static char const ID_DBG_PROPERTIES[] = "Dbg_Properties";
static char const ID_DBG_METHODS[] = "Dbg_Methods";
static char const aSeqLevelStr[] = "[]";
-static char const defaultNameSpace[] = "ooo.vba";
// Gets the default property for an uno object. Note: There is some
// redirection built in. The property name specifies the name
@@ -3258,7 +3257,7 @@ void VBAConstantHelper::init()
{
Sequence< TypeClass > types(1);
types[ 0 ] = TypeClass_CONSTANTS;
- Reference< XTypeDescriptionEnumeration > xEnum = getTypeDescriptorEnumeration( defaultNameSpace, types, TypeDescriptionSearchDepth_INFINITE );
+ Reference< XTypeDescriptionEnumeration > xEnum = getTypeDescriptorEnumeration( "ooo.vba", types, TypeDescriptionSearchDepth_INFINITE );
if ( !xEnum.is())
{
@@ -4162,8 +4161,6 @@ void RTL_Impl_CreateUnoValue( StarBASIC* pBasic, SbxArray& rPar, bool bWrite )
(void)pBasic;
(void)bWrite;
- static const char aTypeTypeString[] = "type";
-
// 2 parameters needed
if ( rPar.Count() != 3 )
{
@@ -4175,7 +4172,7 @@ void RTL_Impl_CreateUnoValue( StarBASIC* pBasic, SbxArray& rPar, bool bWrite )
OUString aTypeName = rPar.Get(1)->GetOUString();
SbxVariable* pVal = rPar.Get(2);
- if( aTypeName == aTypeTypeString )
+ if( aTypeName == "type" )
{
SbxDataType eBaseType = pVal->SbxValue::GetType();
OUString aValTypeName;
diff --git a/compilerplugins/clang/store/oncevar.cxx b/compilerplugins/clang/store/oncevar.cxx
index e618922..1947515 100644
--- a/compilerplugins/clang/store/oncevar.cxx
+++ b/compilerplugins/clang/store/oncevar.cxx
@@ -9,13 +9,14 @@
#include <string>
#include <iostream>
-#include <map>
+#include <unordered_map>
#include "plugin.hxx"
+#include "check.hxx"
#include "clang/AST/CXXInheritance.h"
// Idea from tml.
-// Check for OUString variables that are
+// Check for OUString/char[] variables that are
// (1) initialised from a string literal
// (2) only used in one spot
// In which case, we might as well inline it.
@@ -27,21 +28,64 @@ class OnceVar:
public RecursiveASTVisitor<OnceVar>, public loplugin::Plugin
{
public:
- explicit OnceVar(InstantiationData const & data): Plugin(data), mbChecking(false) {}
+ explicit OnceVar(InstantiationData const & data): Plugin(data) {}
virtual void run() override {
+ // ignore some files with problematic macros
+ std::string fn( compiler.getSourceManager().getFileEntryForID(
+ compiler.getSourceManager().getMainFileID())->getName() );
+ normalizeDotDotInFilePath(fn);
+ // TODO not possible here, need to figure out how to ignore cases where we index
+ // into the string
+ if (fn == SRCDIR "/vcl/source/filter/ixpm/xpmread.cxx")
+ return;
+ if (fn == SRCDIR "/sc/source/filter/excel/xiescher.cxx")
+ return;
+ // all the constants are nicely lined up at the top of the file, seems
+ // a pity to just inline a handful.
+ if (fn == SRCDIR "/sc/source/ui/docshell/docsh.cxx")
+ return;
+ if (fn == SRCDIR "/sw/source/core/text/EnhancedPDFExportHelper.cxx")
+ return;
+ if (fn == SRCDIR "/svgio/source/svgreader/svgtoken.cxx")
+ return;
+ // TODO explicit length array
+ if (fn == SRCDIR "/sal/qa/osl/file/osl_File.cxx")
+ return;
+
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+
+ for (auto it = maVarUsesMap.cbegin(); it != maVarUsesMap.cend(); ++it)
+ {
+ if (it->second == 1)
+ {
+ report(DiagnosticsEngine::Warning,
+ "string/char[] var used only once, should be inlined",
+ it->first)
+ << maVarDeclSourceRangeMap[it->first];
+ report(DiagnosticsEngine::Note,
+ "to this spot",
+ maVarUseSourceRangeMap[it->first].getBegin())
+ << maVarUseSourceRangeMap[it->first];
+ }
+ }
}
- bool TraverseFunctionDecl( FunctionDecl* stmt );
bool VisitDeclRefExpr( const DeclRefExpr* );
private:
- bool mbChecking;
- std::map<SourceLocation,int> maVarUsesMap;
- std::map<SourceLocation,SourceRange> maVarDeclSourceRangeMap;
- std::map<SourceLocation,SourceRange> maVarUseSourceRangeMap;
StringRef getFilename(SourceLocation loc);
+
+ struct SourceLocationHash
+ {
+ size_t operator()( SourceLocation const & sl ) const
+ {
+ return sl.getRawEncoding();
+ }
+ };
+ std::unordered_map<SourceLocation, int, SourceLocationHash> maVarUsesMap;
+ std::unordered_map<SourceLocation, SourceRange, SourceLocationHash> maVarDeclSourceRangeMap;
+ std::unordered_map<SourceLocation, SourceRange, SourceLocationHash> maVarUseSourceRangeMap;
};
StringRef OnceVar::getFilename(SourceLocation loc)
@@ -51,69 +95,43 @@ StringRef OnceVar::getFilename(SourceLocation loc)
return name;
}
-bool OnceVar::TraverseFunctionDecl(FunctionDecl * decl)
-{
- if (ignoreLocation(decl)) {
- return true;
- }
- if (!decl->hasBody()) {
- return true;
- }
- if ( decl != decl->getCanonicalDecl() ) {
- return true;
- }
-
- // some places, it makes the code worse
- StringRef aFileName = getFilename(decl->getLocStart());
- if (aFileName.startswith(SRCDIR "/sal/qa/osl/module/osl_Module.cxx")) {
- return true;
- }
-
- maVarUsesMap.clear();
- maVarDeclSourceRangeMap.clear();
- mbChecking = true;
- TraverseStmt(decl->getBody());
- mbChecking = false;
- for (auto it = maVarUsesMap.cbegin(); it != maVarUsesMap.cend(); ++it)
- {
- if (it->second == 1)
- {
- report(DiagnosticsEngine::Warning,
- "OUString var used only once, should be inlined",
- it->first)
- << maVarDeclSourceRangeMap[it->first];
- report(DiagnosticsEngine::Note,
- "to this spot",
- maVarUseSourceRangeMap[it->first].getBegin())
- << maVarUseSourceRangeMap[it->first];
- }
- }
- return true;
-}
-
bool OnceVar::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
{
- if (!mbChecking)
+ if (ignoreLocation(declRefExpr)) {
return true;
+ }
const Decl* decl = declRefExpr->getDecl();
if (!isa<VarDecl>(decl) || isa<ParmVarDecl>(decl)) {
return true;
}
const VarDecl * varDecl = dyn_cast<VarDecl>(decl)->getCanonicalDecl();
- if (!varDecl->hasInit() || varDecl->hasGlobalStorage()) {
- return true;
- }
- if (varDecl->getType().getUnqualifiedType().getAsString().find("OUString") == std::string::npos) {
+ // ignore stuff in header files (which should really not be there, but anyhow)
+ if (!compiler.getSourceManager().isInMainFile(varDecl->getLocation())) {
return true;
}
- const CXXConstructExpr* constructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit());
- if (!constructExpr || constructExpr->getNumArgs() < 1) {
+ if (!varDecl->hasInit()) {
return true;
}
- if (!isa<StringLiteral>(constructExpr->getArg(0))) {
- return true;
+ if (const StringLiteral* stringLit = dyn_cast<StringLiteral>(varDecl->getInit())) {
+ // ignore long literals, helps to make the code more legible
+ if (stringLit->getLength() > 40) {
+ return true;
+ }
+ // ok
+ } else {
+ const CXXConstructExpr* constructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit());
+ if (!constructExpr || constructExpr->getNumArgs() != 1) {
+ return true;
+ }
+ const StringLiteral* stringLit2 = dyn_cast<StringLiteral>(varDecl->getInit());
+ if (!stringLit2) {
+ return true;
+ }
+ // ignore long literals, helps to make the code more legible
+ if (stringLit2->getLength() > 40) {
+ return true;
+ }
}
-
SourceLocation loc = varDecl->getLocation();
// ignore cases like:
@@ -122,8 +140,11 @@ bool OnceVar::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
// and
// foo(&xxx);
// where we cannot inline the declaration.
- if (isa<MemberExpr>(parentStmt(declRefExpr))
- || isa<UnaryOperator>(parentStmt(declRefExpr))) {
+ auto const tc = loplugin::TypeCheck(varDecl->getType());
+ if (tc.Class("OUString").Namespace("rtl").GlobalNamespace()
+ && (isa<MemberExpr>(parentStmt(declRefExpr))
+ || isa<UnaryOperator>(parentStmt(declRefExpr))))
+ {
maVarUsesMap[loc] = 2;
return true;
}
diff --git a/desktop/source/app/app.cxx b/desktop/source/app/app.cxx
index 7f278e1..53365f4 100644
--- a/desktop/source/app/app.cxx
+++ b/desktop/source/app/app.cxx
@@ -1090,12 +1090,10 @@ void handleCrashReport()
void handleSafeMode()
{
- static const char SERVICENAME_SAFEMODE[] = "com.sun.star.comp.svx.SafeModeUI";
-
css::uno::Reference< css::uno::XComponentContext > xContext = ::comphelper::getProcessComponentContext();
Reference< css::frame::XSynchronousDispatch > xSafeModeUI(
- xContext->getServiceManager()->createInstanceWithContext(SERVICENAME_SAFEMODE, xContext),
+ xContext->getServiceManager()->createInstanceWithContext("com.sun.star.comp.svx.SafeModeUI", xContext),
css::uno::UNO_QUERY_THROW);
css::util::URL aURL;
More information about the Libreoffice-commits
mailing list