Check for #include in extern and namespace blocks.

llvm-svn: 190950
This commit is contained in:
John Thompson 2013-09-18 18:19:43 +00:00
parent 0742ce9cf7
commit 740839260b
8 changed files with 278 additions and 24 deletions

View File

@ -75,6 +75,19 @@
// ^
// Macro defined here.
//
// Checks will also be performed for '#include' directives that are
// nested inside 'extern "C/C++" {}' or 'namespace (name) {}' blocks,
// and can produce error message like the following:
//
// IncludeInExtern.h:2:3
// #include "Empty.h"
// ^
// error: Include directive within extern "C" {}.
// IncludeInExtern.h:1:1
// extern "C" {
// ^
// The "extern "C" {}" block is here.
//
// See PreprocessorTracker.cpp for additional details.
//
// Future directions:
@ -84,18 +97,15 @@
//
// Some ideas:
//
// 1. Check for and warn about "#include" directives inside 'extern "C/C++" {}'
// and "namespace (name) {}" blocks.
// 1. Omit duplicate "not always provided" messages
//
// 2. Omit duplicate "not always provided" messages
//
// 3. Add options to disable any of the checks, in case
// 2. Add options to disable any of the checks, in case
// there is some problem with them, or the messages get too verbose.
//
// 4. Try to figure out the preprocessor conditional directives that
// 3. Try to figure out the preprocessor conditional directives that
// contribute to problems and tie them to the inconsistent definitions.
//
// 5. There are some legitimate uses of preprocessor macros that
// 4. There are some legitimate uses of preprocessor macros that
// modularize will flag as errors, such as repeatedly #include'ing
// a file and using interleaving defined/undefined macros
// to change declarations in the included file. Is there a way
@ -103,7 +113,7 @@
// to ignore. Otherwise you can just exclude the file, after checking
// for legitimate errors.
//
// 6. What else?
// 5. What else?
//
// General clean-up and refactoring:
//
@ -453,8 +463,11 @@ private:
class CollectEntitiesVisitor
: public RecursiveASTVisitor<CollectEntitiesVisitor> {
public:
CollectEntitiesVisitor(SourceManager &SM, EntityMap &Entities)
: SM(SM), Entities(Entities) {}
CollectEntitiesVisitor(SourceManager &SM, EntityMap &Entities,
Preprocessor &PP, PreprocessorTracker &PPTracker,
int &HadErrors)
: SM(SM), Entities(Entities), PP(PP), PPTracker(PPTracker),
HadErrors(HadErrors) {}
bool TraverseStmt(Stmt *S) { return true; }
bool TraverseType(QualType T) { return true; }
@ -478,6 +491,42 @@ public:
bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { return true; }
bool TraverseLambdaCapture(LambdaExpr::Capture C) { return true; }
// Check 'extern "*" {}' block for #include directives.
bool VisitLinkageSpecDecl(LinkageSpecDecl *D) {
// Bail if not a block.
if (!D->hasBraces())
return true;
SourceRange BlockRange = D->getSourceRange();
const char *LinkageLabel;
switch (D->getLanguage()) {
case LinkageSpecDecl::lang_c:
LinkageLabel = "extern \"C\" {}";
break;
case LinkageSpecDecl::lang_cxx:
LinkageLabel = "extern \"C++\" {}";
break;
default:
LinkageLabel = "extern \"\" {}";
}
if (!PPTracker.checkForIncludesInBlock(PP, BlockRange, LinkageLabel,
errs()))
HadErrors = 1;
return true;
}
// Check 'namespace (name) {}' block for #include directives.
bool VisitNamespaceDecl(const NamespaceDecl *D) {
SourceRange BlockRange = D->getSourceRange();
std::string Label("namespace ");
Label += D->getName();
Label += " {}";
if (!PPTracker.checkForIncludesInBlock(PP, BlockRange, Label.c_str(),
errs()))
HadErrors = 1;
return true;
}
// Collect definition entities.
bool VisitNamedDecl(NamedDecl *ND) {
// We only care about file-context variables.
if (!ND->getDeclContext()->isFileContext())
@ -517,14 +566,18 @@ public:
private:
SourceManager &SM;
EntityMap &Entities;
Preprocessor &PP;
PreprocessorTracker &PPTracker;
int &HadErrors;
};
class CollectEntitiesConsumer : public ASTConsumer {
public:
CollectEntitiesConsumer(EntityMap &Entities,
PreprocessorTracker &preprocessorTracker,
Preprocessor &PP, StringRef InFile)
: Entities(Entities), PPTracker(preprocessorTracker), PP(PP) {
Preprocessor &PP, StringRef InFile, int &HadErrors)
: Entities(Entities), PPTracker(preprocessorTracker), PP(PP),
HadErrors(HadErrors) {
PPTracker.handlePreprocessorEntry(PP, InFile);
}
@ -534,7 +587,7 @@ public:
SourceManager &SM = Ctx.getSourceManager();
// Collect declared entities.
CollectEntitiesVisitor(SM, Entities)
CollectEntitiesVisitor(SM, Entities, PP, PPTracker, HadErrors)
.TraverseDecl(Ctx.getTranslationUnitDecl());
// Collect macro definitions.
@ -556,39 +609,46 @@ private:
EntityMap &Entities;
PreprocessorTracker &PPTracker;
Preprocessor &PP;
int &HadErrors;
};
class CollectEntitiesAction : public SyntaxOnlyAction {
public:
CollectEntitiesAction(EntityMap &Entities,
PreprocessorTracker &preprocessorTracker)
: Entities(Entities), PPTracker(preprocessorTracker) {}
PreprocessorTracker &preprocessorTracker,
int &HadErrors)
: Entities(Entities), PPTracker(preprocessorTracker),
HadErrors(HadErrors) {}
protected:
virtual clang::ASTConsumer *CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) {
return new CollectEntitiesConsumer(Entities, PPTracker,
CI.getPreprocessor(), InFile);
CI.getPreprocessor(), InFile, HadErrors);
}
private:
EntityMap &Entities;
PreprocessorTracker &PPTracker;
int &HadErrors;
};
class ModularizeFrontendActionFactory : public FrontendActionFactory {
public:
ModularizeFrontendActionFactory(EntityMap &Entities,
PreprocessorTracker &preprocessorTracker)
: Entities(Entities), PPTracker(preprocessorTracker) {}
PreprocessorTracker &preprocessorTracker,
int &HadErrors)
: Entities(Entities), PPTracker(preprocessorTracker),
HadErrors(HadErrors) {}
virtual CollectEntitiesAction *create() {
return new CollectEntitiesAction(Entities, PPTracker);
return new CollectEntitiesAction(Entities, PPTracker, HadErrors);
}
private:
EntityMap &Entities;
PreprocessorTracker &PPTracker;
int &HadErrors;
};
int main(int Argc, const char **Argv) {
@ -626,8 +686,9 @@ int main(int Argc, const char **Argv) {
EntityMap Entities;
ClangTool Tool(*Compilations, Headers);
Tool.appendArgumentsAdjuster(new AddDependenciesAdjuster(Dependencies));
int HadErrors =
Tool.run(new ModularizeFrontendActionFactory(Entities, *PPTracker));
int HadErrors = 0;
HadErrors |= Tool.run(
new ModularizeFrontendActionFactory(Entities, *PPTracker, HadErrors));
// Create a place to save duplicate entity locations, separate bins per kind.
typedef SmallVector<Location, 8> LocationArray;

View File

@ -7,7 +7,7 @@
//
//===--------------------------------------------------------------------===//
//
// The Basic Idea
// The Basic Idea (Macro and Conditional Checking)
//
// Basically we install a PPCallbacks-derived object to track preprocessor
// activity, namely when a header file is entered/exited, when a macro
@ -49,7 +49,17 @@
// ^
// Macro defined here.
//
// Design and Implementation Details
// The Basic Idea ('Extern "C/C++" {}' Or 'namespace {}') With Nested
// '#include' Checking)
//
// To check for '#include' directives nested inside 'Extern "C/C++" {}'
// or 'namespace {}' blocks, we keep track of the '#include' directives
// while running the preprocessor, and later during a walk of the AST
// we call a function to check for any '#include' directies inside
// an 'Extern "C/C++" {}' or 'namespace {}' block, given its source
// range.
//
// Design and Implementation Details (Macro and Conditional Checking)
//
// A PreprocessorTrackerImpl class implements the PreprocessorTracker
// interface. It uses a PreprocessorCallbacks class derived from PPCallbacks
@ -205,6 +215,22 @@
// to make clearer the separate reporting phases, I could add an output
// message marking the phases.
//
// Design and Implementation Details ('Extern "C/C++" {}' Or
// 'namespace {}') With Nested '#include' Checking)
//
// We override the InclusionDirective in PPCallbacks to record information
// about each '#include' directive encountered during preprocessing.
// We co-opt the PPItemKey class to store the information about each
// '#include' directive, including the source file name containing the
// directive, the name of the file being included, and the source line
// and column of the directive. We store these object in a vector,
// after first check to see if an entry already exists.
//
// Later, while the AST is being walked for other checks, we provide
// visit handlers for 'extern "C/C++" {}' and 'namespace (name) {}'
// blocks, checking to see if any '#include' directives occurred
// within the blocks, reporting errors if any found.
//
// Future Directions
//
// We probably should add options to disable any of the checks, in case
@ -285,7 +311,7 @@ std::string getSourceString(clang::Preprocessor &PP, clang::SourceRange Range) {
return llvm::StringRef(BeginPtr, Length).trim().str();
}
// Retrieve source line from file image.
// Retrieve source line from file image given a location.
std::string getSourceLine(clang::Preprocessor &PP, clang::SourceLocation Loc) {
const llvm::MemoryBuffer *MemBuffer =
PP.getSourceManager().getBuffer(PP.getSourceManager().getFileID(Loc));
@ -310,6 +336,39 @@ std::string getSourceLine(clang::Preprocessor &PP, clang::SourceLocation Loc) {
return llvm::StringRef(BeginPtr, Length).str();
}
// Retrieve source line from file image given a file ID and line number.
std::string getSourceLine(clang::Preprocessor &PP, clang::FileID FileID,
int Line) {
const llvm::MemoryBuffer *MemBuffer = PP.getSourceManager().getBuffer(FileID);
const char *Buffer = MemBuffer->getBufferStart();
const char *BufferEnd = MemBuffer->getBufferEnd();
const char *BeginPtr = Buffer;
const char *EndPtr = BufferEnd;
int LineCounter = 1;
if (Line == 1)
BeginPtr = Buffer;
else {
while (Buffer < BufferEnd) {
if (*Buffer == '\n') {
if (++LineCounter == Line) {
BeginPtr = Buffer++ + 1;
break;
}
}
Buffer++;
}
}
while (Buffer < BufferEnd) {
if (*Buffer == '\n') {
EndPtr = Buffer;
break;
}
Buffer++;
}
size_t Length = EndPtr - BeginPtr;
return llvm::StringRef(BeginPtr, Length).str();
}
// Get the string for the Unexpanded macro instance.
// The soureRange is expected to end at the last token
// for the macro instance, which in the case of a function-style
@ -765,6 +824,14 @@ public:
~PreprocessorCallbacks() {}
// Overridden handlers.
void InclusionDirective(clang::SourceLocation HashLoc,
const clang::Token &IncludeTok,
llvm::StringRef FileName, bool IsAngled,
clang::CharSourceRange FilenameRange,
const clang::FileEntry *File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
const clang::Module *Imported);
void FileChanged(clang::SourceLocation Loc,
clang::PPCallbacks::FileChangeReason Reason,
clang::SrcMgr::CharacteristicKind FileType,
@ -821,6 +888,70 @@ public:
// Handle exiting a preprocessing session.
void handlePreprocessorExit() { HeaderStack.clear(); }
// Handle include directive.
// This function is called every time an include directive is seen by the
// preprocessor, for the purpose of later checking for 'extern "" {}' or
// "namespace {}" blocks containing #include directives.
void handleIncludeDirective(llvm::StringRef DirectivePath, int DirectiveLine,
int DirectiveColumn, llvm::StringRef TargetPath) {
HeaderHandle CurrentHeaderHandle = findHeaderHandle(DirectivePath);
StringHandle IncludeHeaderHandle = addString(TargetPath);
for (std::vector<PPItemKey>::const_iterator I = IncludeDirectives.begin(),
E = IncludeDirectives.end();
I != E; ++I) {
// If we already have an entry for this directive, return now.
if ((I->File == CurrentHeaderHandle) && (I->Line == DirectiveLine))
return;
}
PPItemKey IncludeDirectiveItem(IncludeHeaderHandle, CurrentHeaderHandle,
DirectiveLine, DirectiveColumn);
IncludeDirectives.push_back(IncludeDirectiveItem);
}
// Check for include directives within the given source line range.
// Report errors if any found. Returns true if no include directives
// found in block.
bool checkForIncludesInBlock(clang::Preprocessor &PP,
clang::SourceRange BlockSourceRange,
const char *BlockIdentifierMessage,
llvm::raw_ostream &OS) {
clang::SourceLocation BlockStartLoc = BlockSourceRange.getBegin();
clang::SourceLocation BlockEndLoc = BlockSourceRange.getEnd();
// Use block location to get FileID of both the include directive
// and block statement.
clang::FileID FileID = PP.getSourceManager().getFileID(BlockStartLoc);
std::string SourcePath = getSourceLocationFile(PP, BlockStartLoc);
HeaderHandle SourceHandle = findHeaderHandle(SourcePath);
int BlockStartLine, BlockStartColumn, BlockEndLine, BlockEndColumn;
bool returnValue = true;
getSourceLocationLineAndColumn(PP, BlockStartLoc, BlockStartLine,
BlockStartColumn);
getSourceLocationLineAndColumn(PP, BlockEndLoc, BlockEndLine,
BlockEndColumn);
for (std::vector<PPItemKey>::const_iterator I = IncludeDirectives.begin(),
E = IncludeDirectives.end();
I != E; ++I) {
// If we find an entry within the block, report an error.
if ((I->File == SourceHandle) && (I->Line >= BlockStartLine) &&
(I->Line < BlockEndLine)) {
returnValue = false;
OS << SourcePath << ":" << I->Line << ":" << I->Column << "\n";
OS << getSourceLine(PP, FileID, I->Line) << "\n";
if (I->Column > 0)
OS << std::string(I->Column - 1, ' ') << "^\n";
OS << "error: Include directive within " << BlockIdentifierMessage
<< ".\n";
OS << SourcePath << ":" << BlockStartLine << ":" << BlockStartColumn
<< "\n";
OS << getSourceLine(PP, BlockStartLoc) << "\n";
if (BlockStartColumn > 0)
OS << std::string(BlockStartColumn - 1, ' ') << "^\n";
OS << "The \"" << BlockIdentifierMessage << "\" block is here.\n";
}
}
return returnValue;
}
// Handle entering a header source file.
void handleHeaderEntry(clang::Preprocessor &PP, llvm::StringRef HeaderPath) {
// Ignore <built-in> and <command-line> to reduce message clutter.
@ -1176,6 +1307,7 @@ private:
std::vector<HeaderInclusionPath> InclusionPaths;
InclusionPathHandle CurrentInclusionPathHandle;
llvm::SmallSet<HeaderHandle, 128> HeadersInThisCompile;
std::vector<PPItemKey> IncludeDirectives;
MacroExpansionMap MacroExpansions;
ConditionalExpansionMap ConditionalExpansions;
bool InNestedHeader;
@ -1193,6 +1325,20 @@ PreprocessorTracker *PreprocessorTracker::create() {
// Preprocessor callbacks for modularize.
// Handle include directive.
void PreprocessorCallbacks::InclusionDirective(
clang::SourceLocation HashLoc, const clang::Token &IncludeTok,
llvm::StringRef FileName, bool IsAngled,
clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
llvm::StringRef SearchPath, llvm::StringRef RelativePath,
const clang::Module *Imported) {
int DirectiveLine, DirectiveColumn;
std::string HeaderPath = getSourceLocationFile(PP, HashLoc);
getSourceLocationLineAndColumn(PP, HashLoc, DirectiveLine, DirectiveColumn);
PPTracker.handleIncludeDirective(HeaderPath, DirectiveLine, DirectiveColumn,
FileName);
}
// Handle file entry/exit.
void PreprocessorCallbacks::FileChanged(
clang::SourceLocation Loc, clang::PPCallbacks::FileChangeReason Reason,

View File

@ -52,6 +52,22 @@ public:
// object is destroyed.)
virtual void handlePreprocessorExit() = 0;
// Handle include directive.
// This function is called every time an include directive is seen by the
// preprocessor, for the purpose of later checking for 'extern "" {}' or
// "namespace {}" blocks containing #include directives.
virtual void handleIncludeDirective(llvm::StringRef DirectivePath,
int DirectiveLine, int DirectiveColumn,
llvm::StringRef TargetPath) = 0;
// Check for include directives within the given source line range.
// Report errors if any found. Returns true if no include directives
// found in block.
virtual bool checkForIncludesInBlock(clang::Preprocessor &PP,
clang::SourceRange BlockSourceRange,
const char *BlockIdentifierMessage,
llvm::raw_ostream &OS) = 0;
// Report on inconsistent macro instances.
// Returns true if any mismatches.
virtual bool reportInconsistentMacros(llvm::raw_ostream &OS) = 0;

View File

@ -0,0 +1 @@
// Empty header for testing #include directives in blocks.

View File

@ -0,0 +1,3 @@
extern "C" {
#include "Empty.h"
}

View File

@ -0,0 +1,3 @@
namespace MyNamespace {
#include "Empty.h"
}

View File

@ -0,0 +1,12 @@
# RUN: not modularize %s -x c++ 2>&1 | FileCheck %s
Inputs/IncludeInExtern.h
# CHECK: {{.*}}{{[/\\]}}Inputs{{[/\\]}}IncludeInExtern.h:2:3
# CHECK-NEXT: #include "Empty.h"
# CHECK-NEXT: ^
# CHECK-NEXT: error: Include directive within extern "C" {}.
# CHECK-NEXT: {{.*}}{{[/\\]}}Inputs{{[/\\]}}IncludeInExtern.h:1:1
# CHECK-NEXT: extern "C" {
# CHECK-NEXT: ^
# CHECK-NEXT: The "extern "C" {}" block is here.

View File

@ -0,0 +1,12 @@
# RUN: not modularize %s -x c++ 2>&1 | FileCheck %s
Inputs/IncludeInNamespace.h
# CHECK: {{.*}}{{[/\\]}}Inputs{{[/\\]}}IncludeInNamespace.h:2:3
# CHECK-NEXT: #include "Empty.h"
# CHECK-NEXT: ^
# CHECK-NEXT: error: Include directive within namespace MyNamespace {}.
# CHECK-NEXT: {{.*}}{{[/\\]}}Inputs{{[/\\]}}IncludeInNamespace.h:1:1
# CHECK-NEXT: namespace MyNamespace {
# CHECK-NEXT: ^
# CHECK-NEXT: The "namespace MyNamespace {}" block is here.