mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

compiler plugin check for if/while/true bodies with possibly {} missing

Browse Source 
Change-Id: Ia84c70006b0b8a039b6fea27f3c5cde796f25d03
This commit is contained in:
Luboš Luňák
2012-10-09 16:21:33 +02:00
parent 76757ebd98
commit d4aa136e97
5 changed files with 206 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
compilerplugins/Makefile-clang.mk
View File

@@ -9,7 +9,10 @@
# Make sure variables in this Makefile do not conflict with other variables (e.g. from gbuild).
# The list of source files.
CLANGSRC=compileplugin.cxx unusedvariablecheck.cxx
CLANGSRC=compileplugin.cxx \
bodynotinblock.cxx \
unusedvariablecheck.cxx \
# You may occassionally want to override some of these

12
compilerplugins/README
View File

@@ -28,6 +28,18 @@ All warnings and errors are marked '[loplugin]' in the message.
Additional check for unused variables.
==== Body of if/while/for not in {} ====
- statement aligned as second statement in if/while/for body but not in a statement block [loplugin]
Warn about the following construct:
if( a != 0 )
b = 2;
c = 3;
Here either both statements should be inside {} or the second statement in indented wrong.
== Code documentation / howtos ==

147
compilerplugins/clang/bodynotinblock.cxx Normal file
View File

@@ -0,0 +1,147 @@
/*
* This file is part of the LibreOffice project.
*
* Based on LLVM/Clang.
*
* This file is distributed under the University of Illinois Open Source
* License. See LICENSE.TXT for details.
*
*/
#include "bodynotinblock.hxx"
#include <clang/Basic/SourceManager.h>
using namespace clang;
namespace loplugin
{
/*
Check for two statements that are both indented to look like a body of if/while/for
but are not inside a compound statement and thus the second one is unrelated.
*/
BodyNotInBlock::BodyNotInBlock( ASTContext& context )
: context( context )
{
}
DiagnosticBuilder BodyNotInBlock::report( DiagnosticsEngine::Level level, StringRef message, SourceLocation loc )
{
// Do some mappings (e.g. for -Werror) that clang does not do for custom messages for some reason.
DiagnosticsEngine& diag = context.getDiagnostics();
if( level == DiagnosticsEngine::Warning && diag.getWarningsAsErrors())
level = DiagnosticsEngine::Error;
if( level == DiagnosticsEngine::Error && diag.getErrorsAsFatal())
level = DiagnosticsEngine::Fatal;
return diag.Report( loc, diag.getCustomDiagID( level, message ));
}
void BodyNotInBlock::run()
{
TraverseDecl( context.getTranslationUnitDecl());
}
bool BodyNotInBlock::VisitFunctionDecl( FunctionDecl* declaration )
{
// TODO also LO header files? or a subdir?
if( !context.getSourceManager().isFromMainFile( declaration->getLocStart()))
return true;
if( !declaration->doesThisDeclarationHaveABody())
return true;
StmtParents parents;
traverseStatement( declaration->getBody(), parents );
return true;
}
void BodyNotInBlock::traverseStatement( const Stmt* stmt, StmtParents& parents )
{
parents.push_back( stmt );
for( ConstStmtIterator it = stmt->child_begin();
it != stmt->child_end();
++it )
{
if( *it != NULL ) // some children can be apparently NULL
{
traverseStatement( *it, parents ); // substatements first
parents.push_back( *it );
if( const IfStmt* ifstmt = dyn_cast< IfStmt >( *it ))
{
checkBody( ifstmt->getThen(), parents, 0 );
checkBody( ifstmt->getElse(), parents, 0 );
}
else if( const WhileStmt* whilestmt = dyn_cast< WhileStmt >( *it ))
checkBody( whilestmt->getBody(), parents, 1 );
else if( const ForStmt* forstmt = dyn_cast< ForStmt >( *it ))
checkBody( forstmt->getBody(), parents, 2 );
else if( const CXXForRangeStmt* forstmt = dyn_cast< CXXForRangeStmt >( *it ))
checkBody( forstmt->getBody(), parents, 2 );
parents.pop_back();
}
}
assert( parents.back() == stmt );
parents.pop_back();
}
void BodyNotInBlock::checkBody( const Stmt* body, const StmtParents& parents, int stmtType )
{
if( body == NULL || parents.size() < 2 )
return;
if( dyn_cast< CompoundStmt >( body ))
return; // if body is a compound statement, then it is in {}
// Find the next statement (in source position) after 'body'.
for( int parent_pos = parents.size() - 2; // start from grandparent
parent_pos >= 0;
--parent_pos )
{
for( ConstStmtIterator it = parents[ parent_pos ]->child_begin();
it != parents[ parent_pos ]->child_end();
)
{
if( *it == parents[ parent_pos + 1 ] ) // found grand(grand...)parent
{
// get next statement after our (grand...)parent
++it;
while( it != parents[ parent_pos ]->child_end() && *it == NULL )
++it; // skip empty ones (missing 'else' bodies for example)
if( it != parents[ parent_pos ]->child_end())
{
// TODO: If both statements come from macro expansions, they may be
// below evaluated to be in the same place (because they may be
// the result of expansion of the same macro). Analysing this including
// macro expansions would be probably more complicated, so just
// ignore that in order to avoid false positives.
if( body->getLocStart().isMacroID() && (*it)->getLocStart().isMacroID())
return;
bool invalid1, invalid2;
unsigned bodyColumn = context.getSourceManager()
.getPresumedColumnNumber( body->getLocStart(), &invalid1 );
unsigned nextStatementColumn = context.getSourceManager()
.getPresumedColumnNumber( (*it)->getLocStart(), &invalid2 );
if( invalid1 || invalid2 )
return;
if( bodyColumn == nextStatementColumn )
{
report( DiagnosticsEngine::Warning,
"statement aligned as second statement in %select{if|while|for}0 body but not in a statement block [loplugin]",
(*it)->getLocStart()) << stmtType;
report( DiagnosticsEngine::Note,
"%select{if|while|for}0 body statement is here [loplugin]",
body->getLocStart()) << stmtType;
}
return;
}
// else we need to go higher to find the next statement
}
else
++it;
}
// If going up would mean leaving a {} block, stop, because the } should
// make it visible the two statements are not in the same body.
if( dyn_cast< CompoundStmt >( parents[ parent_pos ] ))
return;
}
}
} // namespace

39
compilerplugins/clang/bodynotinblock.hxx Normal file
View File

@@ -0,0 +1,39 @@
/*
* This file is part of the LibreOffice project.
*
* Based on LLVM/Clang.
*
* This file is distributed under the University of Illinois Open Source
* License. See LICENSE.TXT for details.
*
*/
#ifndef BODYNOTINBLOCK_H
#define BODYNOTINBLOCK_H
#include <clang/AST/RecursiveASTVisitor.h>
using namespace clang;
namespace loplugin
{
typedef std::vector< const Stmt* > StmtParents;
class BodyNotInBlock
: public RecursiveASTVisitor< BodyNotInBlock >
{
public:
explicit BodyNotInBlock( ASTContext& context );
void run();
bool VisitFunctionDecl( FunctionDecl* declaration );
private:
void traverseStatement( const Stmt* stmt, StmtParents& parents );
void checkBody( const Stmt* body, const StmtParents& parents, int stmtType );
DiagnosticBuilder report( DiagnosticsEngine::Level level, StringRef message, SourceLocation loc );
ASTContext& context;
};
} // namespace
#endif // BODYNOTINBLOCK_H

4
compilerplugins/clang/compileplugin.cxx
View File

@@ -17,6 +17,7 @@
#include <clang/Frontend/FrontendPluginRegistry.h>
#include <clang/Rewrite/Rewriter.h>
#include "bodynotinblock.hxx"
#include "unusedvariablecheck.hxx"
using namespace clang;
@@ -33,6 +34,7 @@ class PluginHandler
public:
explicit PluginHandler( ASTContext& context )
: rewriter( context.getSourceManager(), context.getLangOpts())
, bodyNotInBlock( context )
, unusedVariableCheck( context )
{
}
@@ -40,6 +42,7 @@ class PluginHandler
{
if( context.getDiagnostics().hasErrorOccurred())
return;
bodyNotInBlock.run();
unusedVariableCheck.run();
// TODO also LO header files? or a subdir?
if( const RewriteBuffer* buf = rewriter.getRewriteBufferFor( context.getSourceManager().getMainFileID()))
@@ -48,6 +51,7 @@ class PluginHandler
}
private:
Rewriter rewriter;
BodyNotInBlock bodyNotInBlock;
UnusedVariableCheck unusedVariableCheck;
};