r183867 - Introducing -Wheader-guard, a warning that checks header guards actually work
Richard Trieu
rtrieu at google.com
Wed Jun 12 14:20:57 PDT 2013
Author: rtrieu
Date: Wed Jun 12 16:20:57 2013
New Revision: 183867
URL: http://llvm.org/viewvc/llvm-project?rev=183867&view=rev
Log:
Introducing -Wheader-guard, a warning that checks header guards actually work
properly. This warning checks that the #ifndef and #define directives at
the beginning of a header refer to the same macro name. Includes a fix-it
hint to correct the header guard.
Added:
cfe/trunk/test/Lexer/Inputs/
cfe/trunk/test/Lexer/Inputs/bad-header-guard.h
cfe/trunk/test/Lexer/Inputs/different-define.h
cfe/trunk/test/Lexer/Inputs/good-header-guard.h
cfe/trunk/test/Lexer/Inputs/multiple.h
cfe/trunk/test/Lexer/Inputs/no-define.h
cfe/trunk/test/Lexer/Inputs/out-of-order-define.h
cfe/trunk/test/Lexer/Inputs/tokens-between-ifndef-and-define.h
cfe/trunk/test/Lexer/header.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/include/clang/Lex/MultipleIncludeOpt.h
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/PPLexerChange.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Wed Jun 12 16:20:57 2013
@@ -585,5 +585,10 @@ def warn_forgotten_module_header : Warni
InGroup<IncompleteModule>, DefaultIgnore;
def err_expected_id_building_module : Error<
"expected a module name in '__building_module' expression">;
-
+
+def warn_header_guard : Warning<
+ "%0 is used as a header guard here, followed by #define of a different macro">,
+ InGroup<DiagGroup<"header-guard">>;
+def note_header_guard : Note<
+ "%0 is defined here; did you mean %1?">;
}
Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Jun 12 16:20:57 2013
@@ -425,6 +425,11 @@ public:
getFileInfo(File).ControllingMacro = ControllingMacro;
}
+ /// \brief Return true if this is the first time encountering this header.
+ bool FirstTimeLexingFile(const FileEntry *File) {
+ return getFileInfo(File).NumIncludes == 1;
+ }
+
/// \brief Determine whether this file is intended to be safe from
/// multiple inclusions, e.g., it has \#pragma once or a controlling
/// macro.
Modified: cfe/trunk/include/clang/Lex/MultipleIncludeOpt.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MultipleIncludeOpt.h?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/MultipleIncludeOpt.h (original)
+++ cfe/trunk/include/clang/Lex/MultipleIncludeOpt.h Wed Jun 12 16:20:57 2013
@@ -15,6 +15,8 @@
#ifndef LLVM_CLANG_MULTIPLEINCLUDEOPT_H
#define LLVM_CLANG_MULTIPLEINCLUDEOPT_H
+#include "clang/Basic/SourceLocation.h"
+
namespace clang {
class IdentifierInfo;
@@ -32,6 +34,11 @@ class MultipleIncludeOpt {
/// \#endif can be easily detected.
bool ReadAnyTokens;
+ /// ImmediatelyAfterTopLevelIfndef - This is true when the only tokens
+ /// processed in the file so far is an #ifndef and an identifier. Used in
+ /// the detection of header guards in a file.
+ bool ImmediatelyAfterTopLevelIfndef;
+
/// ReadAnyTokens - This is set to false when a file is first opened and true
/// any time a token is returned to the client or a (non-multiple-include)
/// directive is parsed. When the final #endif is parsed this is reset back
@@ -42,11 +49,36 @@ class MultipleIncludeOpt {
/// TheMacro - The controlling macro for a file, if valid.
///
const IdentifierInfo *TheMacro;
+
+ /// DefinedMacro - The macro defined right after TheMacro, if any.
+ const IdentifierInfo *DefinedMacro;
+
+ SourceLocation MacroLoc;
+ SourceLocation DefinedLoc;
public:
MultipleIncludeOpt() {
ReadAnyTokens = false;
+ ImmediatelyAfterTopLevelIfndef = false;
DidMacroExpansion = false;
TheMacro = 0;
+ DefinedMacro = 0;
+ }
+
+ SourceLocation GetMacroLocation() const {
+ return MacroLoc;
+ }
+
+ SourceLocation GetDefinedLocation() const {
+ return DefinedLoc;
+ }
+
+ void resetImmediatelyAfterTopLevelIfndef() {
+ ImmediatelyAfterTopLevelIfndef = false;
+ }
+
+ void SetDefinedMacro(IdentifierInfo *M, SourceLocation Loc) {
+ DefinedMacro = M;
+ DefinedLoc = Loc;
}
/// Invalidate - Permanently mark this file as not being suitable for the
@@ -55,6 +87,8 @@ public:
// If we have read tokens but have no controlling macro, the state-machine
// below can never "accept".
ReadAnyTokens = true;
+ ImmediatelyAfterTopLevelIfndef = false;
+ DefinedMacro = 0;
TheMacro = 0;
}
@@ -63,8 +97,17 @@ public:
/// the "ifndef x" would count as reading tokens.
bool getHasReadAnyTokensVal() const { return ReadAnyTokens; }
+ /// getImmediatelyAfterTopLevelIfndef - returns true if the last directive
+ /// was an #ifndef at the beginning of the file.
+ bool getImmediatelyAfterTopLevelIfndef() const {
+ return ImmediatelyAfterTopLevelIfndef;
+ }
+
// If a token is read, remember that we have seen a side-effect in this file.
- void ReadToken() { ReadAnyTokens = true; }
+ void ReadToken() {
+ ReadAnyTokens = true;
+ ImmediatelyAfterTopLevelIfndef = false;
+ }
/// ExpandedMacro - When a macro is expanded with this lexer as the current
/// buffer, this method is called to disable the MIOpt if needed.
@@ -77,7 +120,7 @@ public:
/// ensures that this is only called if there are no tokens read before the
/// \#ifndef. The caller is required to do this, because reading the \#if
/// line obviously reads in in tokens.
- void EnterTopLevelIFNDEF(const IdentifierInfo *M) {
+ void EnterTopLevelIfndef(const IdentifierInfo *M, SourceLocation Loc) {
// If the macro is already set, this is after the top-level #endif.
if (TheMacro)
return Invalidate();
@@ -91,7 +134,9 @@ public:
// Remember that we're in the #if and that we have the macro.
ReadAnyTokens = true;
+ ImmediatelyAfterTopLevelIfndef = true;
TheMacro = M;
+ MacroLoc = Loc;
}
/// \brief Invoked when a top level conditional (except \#ifndef) is found.
@@ -111,6 +156,7 @@ public:
// At this point, we haven't "read any tokens" but we do have a controlling
// macro.
ReadAnyTokens = false;
+ ImmediatelyAfterTopLevelIfndef = false;
}
/// \brief Once the entire file has been lexed, if there is a controlling
@@ -122,6 +168,12 @@ public:
return TheMacro;
return 0;
}
+
+ /// \brief If the ControllingMacro is followed by a macro definition, return
+ /// the macro that was defined.
+ const IdentifierInfo *GetDefinedMacro() const {
+ return DefinedMacro;
+ }
};
} // end namespace clang
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Wed Jun 12 16:20:57 2013
@@ -1444,7 +1444,7 @@ private:
void HandleMicrosoftImportDirective(Token &Tok);
// Macro handling.
- void HandleDefineDirective(Token &Tok);
+ void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef);
void HandleUndefDirective(Token &Tok);
// Conditional Inclusion.
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Wed Jun 12 16:20:57 2013
@@ -626,6 +626,10 @@ void Preprocessor::HandleDirective(Token
CurPPLexer->ParsingPreprocessorDirective = true;
if (CurLexer) CurLexer->SetKeepWhitespaceMode(false);
+ bool ImmediatelyAfterTopLevelIfndef =
+ CurPPLexer->MIOpt.getImmediatelyAfterTopLevelIfndef();
+ CurPPLexer->MIOpt.resetImmediatelyAfterTopLevelIfndef();
+
++NumDirectives;
// We are about to read a token. For the multiple-include optimization FA to
@@ -713,7 +717,7 @@ void Preprocessor::HandleDirective(Token
// C99 6.10.3 - Macro Replacement.
case tok::pp_define:
- return HandleDefineDirective(Result);
+ return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef);
case tok::pp_undef:
return HandleUndefDirective(Result);
@@ -1760,7 +1764,8 @@ bool Preprocessor::ReadMacroDefinitionAr
/// HandleDefineDirective - Implements \#define. This consumes the entire macro
/// line then lets the caller lex the next real token.
-void Preprocessor::HandleDefineDirective(Token &DefineTok) {
+void Preprocessor::HandleDefineDirective(Token &DefineTok,
+ bool ImmediatelyAfterHeaderGuard) {
++NumDefined;
Token MacroNameTok;
@@ -1786,6 +1791,11 @@ void Preprocessor::HandleDefineDirective
// marking each of the identifiers as being used as macro arguments. Also,
// check other constraints on the first token of the macro body.
if (Tok.is(tok::eod)) {
+ if (ImmediatelyAfterHeaderGuard) {
+ // Save this macro information since it may part of a header guard.
+ CurPPLexer->MIOpt.SetDefinedMacro(MacroNameTok.getIdentifierInfo(),
+ MacroNameTok.getLocation());
+ }
// If there is no body to this macro, we have no special handling here.
} else if (Tok.hasLeadingSpace()) {
// This is a normal token with leading space. Clear the leading space
@@ -2076,7 +2086,7 @@ void Preprocessor::HandleIfdefDirective(
// handle.
if (!ReadAnyTokensBeforeDirective && MI == 0) {
assert(isIfndef && "#ifdef shouldn't reach here");
- CurPPLexer->MIOpt.EnterTopLevelIFNDEF(MII);
+ CurPPLexer->MIOpt.EnterTopLevelIfndef(MII, MacroNameTok.getLocation());
} else
CurPPLexer->MIOpt.EnterTopLevelConditional();
}
@@ -2122,7 +2132,7 @@ void Preprocessor::HandleIfDirective(Tok
// directive seen, handle it for the multiple-include optimization.
if (CurPPLexer->getConditionalStackDepth() == 0) {
if (!ReadAnyTokensBeforeDirective && IfNDefMacro && ConditionalTrue)
- CurPPLexer->MIOpt.EnterTopLevelIFNDEF(IfNDefMacro);
+ CurPPLexer->MIOpt.EnterTopLevelIfndef(IfNDefMacro, ConditionalBegin);
else
CurPPLexer->MIOpt.EnterTopLevelConditional();
}
Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=183867&r1=183866&r2=183867&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
+++ cfe/trunk/lib/Lex/PPLexerChange.cpp Wed Jun 12 16:20:57 2013
@@ -244,8 +244,29 @@ bool Preprocessor::HandleEndOfFile(Token
CurPPLexer->MIOpt.GetControllingMacroAtEndOfFile()) {
// Okay, this has a controlling macro, remember in HeaderFileInfo.
if (const FileEntry *FE =
- SourceMgr.getFileEntryForID(CurPPLexer->getFileID()))
+ SourceMgr.getFileEntryForID(CurPPLexer->getFileID())) {
HeaderInfo.SetFileControllingMacro(FE, ControllingMacro);
+ if (const IdentifierInfo *DefinedMacro =
+ CurPPLexer->MIOpt.GetDefinedMacro()) {
+ if (!ControllingMacro->hasMacroDefinition() &&
+ DefinedMacro != ControllingMacro &&
+ HeaderInfo.FirstTimeLexingFile(FE)) {
+ // Emit a warning for a bad header guard.
+ Diag(CurPPLexer->MIOpt.GetMacroLocation(),
+ diag::warn_header_guard)
+ << CurPPLexer->MIOpt.GetMacroLocation()
+ << ControllingMacro;
+ Diag(CurPPLexer->MIOpt.GetDefinedLocation(),
+ diag::note_header_guard)
+ << CurPPLexer->MIOpt.GetDefinedLocation()
+ << DefinedMacro
+ << ControllingMacro
+ << FixItHint::CreateReplacement(
+ CurPPLexer->MIOpt.GetDefinedLocation(),
+ ControllingMacro->getName());
+ }
+ }
+ }
}
}
Added: cfe/trunk/test/Lexer/Inputs/bad-header-guard.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/bad-header-guard.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/bad-header-guard.h (added)
+++ cfe/trunk/test/Lexer/Inputs/bad-header-guard.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,4 @@
+#ifndef bad_header_guard
+#define bad_guard
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/different-define.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/different-define.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/different-define.h (added)
+++ cfe/trunk/test/Lexer/Inputs/different-define.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,4 @@
+#ifndef different_define
+#define FOO 5
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/good-header-guard.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/good-header-guard.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/good-header-guard.h (added)
+++ cfe/trunk/test/Lexer/Inputs/good-header-guard.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,4 @@
+#ifndef good_header_guard
+#define good_header_guard
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/multiple.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/multiple.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/multiple.h (added)
+++ cfe/trunk/test/Lexer/Inputs/multiple.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,4 @@
+#ifndef multiple
+#define multi
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/no-define.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/no-define.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/no-define.h (added)
+++ cfe/trunk/test/Lexer/Inputs/no-define.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,3 @@
+#ifndef no_define
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/out-of-order-define.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/out-of-order-define.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/out-of-order-define.h (added)
+++ cfe/trunk/test/Lexer/Inputs/out-of-order-define.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,7 @@
+#ifndef out_of_order
+
+#define something_else
+
+#define out_of_order
+
+#endif
Added: cfe/trunk/test/Lexer/Inputs/tokens-between-ifndef-and-define.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/Inputs/tokens-between-ifndef-and-define.h?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/Inputs/tokens-between-ifndef-and-define.h (added)
+++ cfe/trunk/test/Lexer/Inputs/tokens-between-ifndef-and-define.h Wed Jun 12 16:20:57 2013
@@ -0,0 +1,7 @@
+#ifndef tokens_between
+
+const int pi = 3;
+
+#define pi_is_bad
+
+#endif
Added: cfe/trunk/test/Lexer/header.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/header.cpp?rev=183867&view=auto
==============================================================================
--- cfe/trunk/test/Lexer/header.cpp (added)
+++ cfe/trunk/test/Lexer/header.cpp Wed Jun 12 16:20:57 2013
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsyntax-only -Wno-header-guard %s
+// RUN: %clang_cc1 -fsyntax-only -Wheader-guard %s 2>&1 | FileCheck %s
+
+#include "Inputs/good-header-guard.h"
+#include "Inputs/no-define.h"
+#include "Inputs/different-define.h"
+#include "Inputs/out-of-order-define.h"
+#include "Inputs/tokens-between-ifndef-and-define.h"
+
+#include "Inputs/bad-header-guard.h"
+// CHECK: In file included from {{.*}}header.cpp:{{[0-9]*}}:
+// CHECK: {{.*}}bad-header-guard.h:1:9: warning: 'bad_header_guard' is used as a header guard here, followed by #define of a different macro
+// CHECK: {{^}}#ifndef bad_header_guard
+// CHECK: {{^}} ^~~~~~~~~~~~~~~~
+// CHECK: {{.*}}bad-header-guard.h:2:9: note: 'bad_guard' is defined here; did you mean 'bad_header_guard'?
+// CHECK: {{^}}#define bad_guard
+// CHECK: {{^}} ^~~~~~~~~
+// CHECK: {{^}} bad_header_guard
+
+#include "Inputs/multiple.h"
+#include "Inputs/multiple.h"
+#include "Inputs/multiple.h"
+#include "Inputs/multiple.h"
+// CHECK: In file included from {{.*}}header.cpp:{{[0-9]*}}:
+// CHECK: {{.*}}multiple.h:1:9: warning: 'multiple' is used as a header guard here, followed by #define of a different macro
+// CHECK: {{^}}#ifndef multiple
+// CHECK: {{^}} ^~~~~~~~
+// CHECK: {{.*}}multiple.h:2:9: note: 'multi' is defined here; did you mean 'multiple'?
+// CHECK: {{^}}#define multi
+// CHECK: {{^}} ^~~~~
+// CHECK: {{^}} multiple
+
+// CHECK: 2 warnings generated.
More information about the cfe-commits
mailing list