compilerplugins/clang/sallogareas.cxx | 107 +++++++++++++++------------
compilerplugins/clang/test/sallogareas.cxx | 58 ++++++++++++++
include/tools/diagnose_ex.h | 24 ++++--
solenv/CompilerTest_compilerplugins_clang.mk | 1
tools/source/debug/debug.cxx | 7 +
xmloff/source/draw/shapeexport.cxx | 6 -
xmloff/source/draw/ximpshap.cxx | 30 +++----
7 files changed, 161 insertions(+), 72 deletions(-)
New commits:
commit e075ee967d0c030a22b7699ee54b5cbd49c07c17
Author: Noel Grandin <noel.gran...@collabora.co.uk>
Date: Thu Mar 29 13:49:19 2018 +0200
give DBG_UNHANDLED_EXCEPTION_WHEN an area parameter
and rename it to DBG_UNHANDLED_EXCEPTION, to make it more like
the SAL_WARN-type macros.
Use some macro magic to deal with different numbers of arguments.
Update the sallogareas plugin to check the area parameter of
DBG_UNHANDLED_EXCEPTION.
Change-Id: Ie790223244c3484f41acb3679c043fb9b438e7c4
Reviewed-on: https://gerrit.libreoffice.org/52073
Tested-by: Jenkins <c...@libreoffice.org>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/compilerplugins/clang/sallogareas.cxx
b/compilerplugins/clang/sallogareas.cxx
index 98cafb596fd5..1095302d9ada 100644
--- a/compilerplugins/clang/sallogareas.cxx
+++ b/compilerplugins/clang/sallogareas.cxx
@@ -49,54 +49,73 @@ bool SalLogAreas::VisitCallExpr( const CallExpr* call )
{
if( ignoreLocation( call ))
return true;
- if( const FunctionDecl* func = call->getDirectCallee())
+ const FunctionDecl* func = call->getDirectCallee();
+ if( !func )
+ return true;
+
+ if( !( func->getNumParams() == 5 && func->getIdentifier() != NULL
+ && ( func->getName() == "sal_detail_log" || func->getName() == "log"
|| func->getName() == "DbgUnhandledException")) )
+ return true;
+
+ auto tc = loplugin::DeclCheck(func);
+ enum class LogCallKind { Sal, DbgUnhandledException};
+ LogCallKind kind;
+ int areaArgIndex;
+ if( tc.Function("sal_detail_log") ||
tc.Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() )
+ {
+ kind = LogCallKind::Sal; // fine
+ areaArgIndex = 1;
+ }
+ else if( tc.Function("DbgUnhandledException").GlobalNamespace() )
+ {
+ kind = LogCallKind::DbgUnhandledException; // ok
+ areaArgIndex = 3;
+ }
+ else
+ return true;
+
+ // The SAL_DETAIL_LOG_STREAM macro expands to two calls to
sal::detail::log(),
+ // so do not warn repeatedly about the same macro (the area->getLocStart()
of all the calls
+ // from the same macro should be the same).
+ if( kind == LogCallKind::Sal )
+ {
+ SourceLocation expansionLocation =
compiler.getSourceManager().getExpansionLoc( call->getLocStart());
+ if( expansionLocation == lastSalDetailLogStreamMacro )
+ return true;
+ lastSalDetailLogStreamMacro = expansionLocation;
+ };
+ if( const clang::StringLiteral* area = dyn_cast< clang::StringLiteral >(
call->getArg( areaArgIndex )->IgnoreParenImpCasts()))
{
- if( func->getNumParams() == 5 && func->getIdentifier() != NULL
- && ( func->getName() == "sal_detail_log" || func->getName() ==
"log" ))
+ if( area->getKind() == clang::StringLiteral::Ascii )
+ checkArea( area->getBytes(), area->getExprLoc());
+ else
+ report( DiagnosticsEngine::Warning, "unsupported string literal
kind (plugin needs fixing?)",
+ area->getLocStart());
+ return true;
+ }
+ if( kind == LogCallKind::DbgUnhandledException ) // below checks don't
apply
+ return true;
+ if(
loplugin::DeclCheck(inFunction).Function("log").Namespace("detail").Namespace("sal").GlobalNamespace()
+ ||
loplugin::DeclCheck(inFunction).Function("sal_detail_logFormat").GlobalNamespace()
)
+ return true; // These functions only forward to sal_detail_log, so ok.
+ if( call->getArg( areaArgIndex )->isNullPointerConstant(
compiler.getASTContext(),
+ Expr::NPC_ValueDependentIsNotNull ) != Expr::NPCK_NotNull )
+ { // If the area argument is a null pointer, that is allowed only for
SAL_DEBUG.
+ const SourceManager& source = compiler.getSourceManager();
+ for( SourceLocation loc = call->getLocStart();
+ loc.isMacroID();
+ loc = source.getImmediateExpansionRange( loc ).first )
{
- auto tc = loplugin::DeclCheck(func);
- if( tc.Function("sal_detail_log") ||
tc.Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() )
- {
- // The SAL_DETAIL_LOG_STREAM macro expands to two calls to
sal::detail::log(),
- // so do not warn repeatedly about the same macro (the
area->getLocStart() of all the calls
- // from the same macro should be the same).
- SourceLocation expansionLocation =
compiler.getSourceManager().getExpansionLoc( call->getLocStart());
- if( expansionLocation == lastSalDetailLogStreamMacro )
- return true;
- lastSalDetailLogStreamMacro = expansionLocation;
- if( const clang::StringLiteral* area = dyn_cast<
clang::StringLiteral >( call->getArg( 1 )->IgnoreParenImpCasts()))
- {
- if( area->getKind() == clang::StringLiteral::Ascii )
- checkArea( area->getBytes(), area->getExprLoc());
- else
- report( DiagnosticsEngine::Warning, "unsupported
string literal kind (plugin needs fixing?)",
- area->getLocStart());
- return true;
- }
- if(
loplugin::DeclCheck(inFunction).Function("log").Namespace("detail").Namespace("sal").GlobalNamespace()
- ||
loplugin::DeclCheck(inFunction).Function("sal_detail_logFormat").GlobalNamespace()
)
- return true; // These functions only forward to
sal_detail_log, so ok.
- if( call->getArg( 1 )->isNullPointerConstant(
compiler.getASTContext(),
- Expr::NPC_ValueDependentIsNotNull ) != Expr::NPCK_NotNull )
- { // If the area argument is a null pointer, that is
allowed only for SAL_DEBUG.
- const SourceManager& source = compiler.getSourceManager();
- for( SourceLocation loc = call->getLocStart();
- loc.isMacroID();
- loc = source.getImmediateExpansionRange( loc ).first )
- {
- StringRef inMacro = Lexer::getImmediateMacroName( loc,
source, compiler.getLangOpts());
- if( inMacro == "SAL_DEBUG" || inMacro ==
"SAL_DEBUG_BACKTRACE" )
- return true; // ok
- }
- report( DiagnosticsEngine::Warning, "missing log area",
- call->getArg( 1
)->IgnoreParenImpCasts()->getLocStart());
- return true;
- }
- report( DiagnosticsEngine::Warning, "cannot analyse log area
argument (plugin needs fixing?)",
- call->getLocStart());
- }
+ StringRef inMacro = Lexer::getImmediateMacroName( loc, source,
compiler.getLangOpts());
+ if( inMacro == "SAL_DEBUG" || inMacro == "SAL_DEBUG_BACKTRACE" )
+ return true; // ok
}
+ report( DiagnosticsEngine::Warning, "missing log area",
+ call->getArg( 1 )->IgnoreParenImpCasts()->getLocStart());
+ return true;
}
+ report( DiagnosticsEngine::Warning, "cannot analyse log area argument
(plugin needs fixing?)",
+ call->getLocStart());
return true;
}
diff --git a/compilerplugins/clang/test/sallogareas.cxx
b/compilerplugins/clang/test/sallogareas.cxx
new file mode 100644
index 000000000000..6a9035a05f30
--- /dev/null
+++ b/compilerplugins/clang/test/sallogareas.cxx
@@ -0,0 +1,58 @@
+/* -*- 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/diagnose_ex.h>
+
+void func1();
+
+int main()
+{
+ SAL_WARN(
+ "bob.none",
+ "message"); // expected-error@-2 {{unknown log area 'bob.none' (check
or extend include/sal/log-areas.dox) [loplugin:sallogareas]}}
+
+ SAL_WARN("xmloff", "message");
+
+ try
+ {
+ func1();
+ }
+ catch (std::exception const&)
+ {
+ DBG_UNHANDLED_EXCEPTION(
+ "bob.none",
+ "message"); // expected-error@-2 {{unknown log area 'bob.none'
(check or extend include/sal/log-areas.dox) [loplugin:sallogareas]}}
+ }
+ try
+ {
+ func1();
+ }
+ catch (std::exception const&)
+ {
+ DBG_UNHANDLED_EXCEPTION("xmloff", "message");
+ }
+ try
+ {
+ func1();
+ }
+ catch (std::exception const&)
+ {
+ DBG_UNHANDLED_EXCEPTION("xmloff");
+ }
+ try
+ {
+ func1();
+ }
+ catch (std::exception const&)
+ {
+ DBG_UNHANDLED_EXCEPTION();
+ }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/include/tools/diagnose_ex.h b/include/tools/diagnose_ex.h
index df71bfa01711..50a65f01a03f 100644
--- a/include/tools/diagnose_ex.h
+++ b/include/tools/diagnose_ex.h
@@ -29,10 +29,9 @@
TOOLS_DLLPUBLIC void DbgUnhandledException(const css::uno::Any&
caughtException,
const char* currentFunction, const char* fileAndLineNo,
- const char* explanatory = nullptr);
+ const char* area = nullptr, const char* explanatory = nullptr);
#if OSL_DEBUG_LEVEL > 0
- #include <com/sun/star/configuration/CorruptedConfigurationException.hpp>
#include <cppuhelper/exc_hlp.hxx>
#include <osl/thread.h>
@@ -40,16 +39,25 @@ TOOLS_DLLPUBLIC void DbgUnhandledException(const
css::uno::Any& caughtException,
Note that whenever you use this, it might be an indicator that your
error
handling is not correct ....
+ This takes two optional parameters: area and explanatory
*/
- #define DBG_UNHANDLED_EXCEPTION() \
- DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC,
SAL_DETAIL_WHERE);
+ #define DBG_UNHANDLED_EXCEPTION_0_ARGS() \
+ DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC,
SAL_DETAIL_WHERE );
+ #define DBG_UNHANDLED_EXCEPTION_1_ARGS(area) \
+ DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC,
SAL_DETAIL_WHERE, area );
+ #define DBG_UNHANDLED_EXCEPTION_2_ARGS(area, explanatory) \
+ DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC,
SAL_DETAIL_WHERE, area, explanatory );
+
+ #define DBG_UNHANDLED_FUNC_CHOOSER(_f1, _f2, _f3, ...) _f3
+ #define DBG_UNHANDLED_FUNC_RECOMPOSER(argsWithParentheses)
DBG_UNHANDLED_FUNC_CHOOSER argsWithParentheses
+ #define DBG_UNHANDLED_CHOOSE_FROM_ARG_COUNT(...)
DBG_UNHANDLED_FUNC_RECOMPOSER((__VA_ARGS__, DBG_UNHANDLED_EXCEPTION_2_ARGS,
DBG_UNHANDLED_EXCEPTION_1_ARGS, DBG_UNHANDLED_EXCEPTION_0_ARGS, ))
+ #define DBG_UNHANDLED_NO_ARG_EXPANDER() ,,DBG_UNHANDLED_EXCEPTION_0_ARGS
+ #define DBG_UNHANDLED_MACRO_CHOOSER(...)
DBG_UNHANDLED_CHOOSE_FROM_ARG_COUNT(DBG_UNHANDLED_NO_ARG_EXPANDER __VA_ARGS__
())
+ #define DBG_UNHANDLED_EXCEPTION(...)
DBG_UNHANDLED_MACRO_CHOOSER(__VA_ARGS__)(__VA_ARGS__)
- #define DBG_UNHANDLED_EXCEPTION_WHEN(explain) \
- DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC,
SAL_DETAIL_WHERE, explain);
#else // OSL_DEBUG_LEVEL
- #define DBG_UNHANDLED_EXCEPTION()
- #define DBG_UNHANDLED_EXCEPTION_WHEN(explain)
+ #define DBG_UNHANDLED_EXCEPTION(...)
#endif // OSL_DEBUG_LEVEL
/** This macro asserts the given condition (in debug mode), and throws
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk
b/solenv/CompilerTest_compilerplugins_clang.mk
index d417702d19fe..5187f60beb2f 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -42,6 +42,7 @@ $(eval $(call
gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/refcounting \
compilerplugins/clang/test/salbool \
compilerplugins/clang/test/salcall \
+ compilerplugins/clang/test/sallogareas \
compilerplugins/clang/test/salunicodeliteral \
compilerplugins/clang/test/simplifybool \
compilerplugins/clang/test/simplifydynamiccast \
diff --git a/tools/source/debug/debug.cxx b/tools/source/debug/debug.cxx
index 6725670f82ce..440c73418ed9 100644
--- a/tools/source/debug/debug.cxx
+++ b/tools/source/debug/debug.cxx
@@ -76,7 +76,7 @@ void DbgTestSolarMutex()
#endif
void DbgUnhandledException(const css::uno::Any & caught, const char*
currentFunction, const char* fileAndLineNo,
- const char* explanatory)
+ const char* area, const char* explanatory)
{
OString sMessage( "DBG_UNHANDLED_EXCEPTION in " );
sMessage += currentFunction;
@@ -120,9 +120,12 @@ void DbgUnhandledException(const css::uno::Any & caught,
const char* currentFunc
}
sMessage += "\n";
+ if (area == nullptr)
+ area = "legacy.osl";
+
SAL_DETAIL_LOG_FORMAT(
SAL_DETAIL_ENABLE_LOG_WARN, SAL_DETAIL_LOG_LEVEL_WARN,
- "legacy.osl", fileAndLineNo, "%s", sMessage.getStr());
+ area, fileAndLineNo, "%s", sMessage.getStr());
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/draw/shapeexport.cxx
b/xmloff/source/draw/shapeexport.cxx
index b0422e8fde8b..fd2ad7ca647c 100644
--- a/xmloff/source/draw/shapeexport.cxx
+++ b/xmloff/source/draw/shapeexport.cxx
@@ -554,7 +554,7 @@ void XMLShapeExport::collectShapeAutoStyles(const
uno::Reference< drawing::XShap
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "collecting auto styles for a
table" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "collecting auto styles for
a table" );
}
break;
}
@@ -746,7 +746,7 @@ void XMLShapeExport::exportShape(const uno::Reference<
drawing::XShape >& xShape
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "exporting layer name for shape"
);
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "exporting layer name for
shape" );
}
}
}
@@ -1867,7 +1867,7 @@ void XMLShapeExport::ImpExportDescription( const
uno::Reference< drawing::XShape
}
catch( uno::Exception& )
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "exporting Title and/or Description for
shape" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "exporting Title and/or Description
for shape" );
}
}
diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx
index b48c92d3c577..edcbcf4af1bc 100644
--- a/xmloff/source/draw/ximpshap.cxx
+++ b/xmloff/source/draw/ximpshap.cxx
@@ -330,7 +330,7 @@ void SdXMLShapeContext::addGluePoint( const uno::Reference<
xml::sax::XAttribute
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "during setting of glue points");
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "during setting of glue
points");
}
}
}
@@ -401,7 +401,7 @@ void SdXMLShapeContext::EndElement()
}
catch(const Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN("while setting hyperlink");
+ DBG_UNHANDLED_EXCEPTION("xmloff", "while setting hyperlink");
}
if( mxLockable.is() )
@@ -443,7 +443,7 @@ void SdXMLShapeContext::AddShape(uno::Reference<
drawing::XShape >& xShape)
}
catch(const Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "while setting visible or printable"
);
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "while setting visible or
printable" );
}
if(!mbTemporaryShape && (!GetImport().HasTextImport()
@@ -678,7 +678,7 @@ void SdXMLShapeContext::SetStyle( bool bSupportsStyle /* =
true */)
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "finding style for shape" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "finding style for
shape" );
}
}
@@ -691,7 +691,7 @@ void SdXMLShapeContext::SetStyle( bool bSupportsStyle /* =
true */)
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting style for shape" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting style for
shape" );
}
}
@@ -986,7 +986,7 @@ void SdXMLRectShapeContext::StartElement(const
uno::Reference< xml::sax::XAttrib
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius");
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner
radius");
}
}
}
@@ -1647,7 +1647,7 @@ void SdXMLTextBoxShapeContext::StartElement(const
uno::Reference< xml::sax::XAtt
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius");
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner
radius");
}
}
}
@@ -1664,7 +1664,7 @@ void SdXMLTextBoxShapeContext::StartElement(const
uno::Reference< xml::sax::XAtt
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting name of next chain
link");
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting name of next
chain link");
}
}
}
@@ -2300,7 +2300,7 @@ void SdXMLCaptionShapeContext::StartElement(const
uno::Reference< xml::sax::XAtt
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius");
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner
radius");
}
}
}
@@ -3352,7 +3352,7 @@ void
SdXMLFrameShapeContext::removeGraphicFromImportContext(const SvXMLImportCon
}
catch( uno::Exception& )
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "Error in cleanup of multiple
graphic object import." );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "Error in cleanup of multiple
graphic object import." );
}
}
}
@@ -3386,7 +3386,7 @@ uno::Reference<graphic::XGraphic>
SdXMLFrameShapeContext::getGraphicFromImportCo
}
catch( uno::Exception& )
{
- DBG_UNHANDLED_EXCEPTION_WHEN("Error in cleanup of multiple graphic
object import.");
+ DBG_UNHANDLED_EXCEPTION("xmloff", "Error in cleanup of multiple
graphic object import.");
}
return xGraphic;
@@ -3410,7 +3410,7 @@ OUString
SdXMLFrameShapeContext::getGraphicPackageURLFromImportContext(const SvX
}
catch( uno::Exception& )
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "Error in cleanup of multiple
graphic object import." );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "Error in cleanup of multiple
graphic object import." );
}
}
@@ -3713,7 +3713,7 @@ void SdXMLCustomShapeContext::StartElement( const
uno::Reference< xml::sax::XAtt
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting enhanced customshape
geometry" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting enhanced customshape
geometry" );
}
SdXMLShapeContext::StartElement(xAttrList);
}
@@ -3796,7 +3796,7 @@ void SdXMLCustomShapeContext::EndElement()
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN( "setting enhanced customshape
geometry" );
+ DBG_UNHANDLED_EXCEPTION( "xmloff", "setting enhanced customshape
geometry" );
}
sal_Int32 nUPD;
@@ -3834,7 +3834,7 @@ void SdXMLCustomShapeContext::EndElement()
}
catch(const uno::Exception&)
{
- DBG_UNHANDLED_EXCEPTION_WHEN("flushing after load");
+ DBG_UNHANDLED_EXCEPTION("xmloff", "flushing after load");
}
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
