Hi! The following patch implements the clang -Wheader-guard warning, which warns if a valid multiple inclusion header guard's #ifndef/#if !defined directive is immediately (no other non-line directives nor other (non-comment) tokens in between) followed by #define directive for some different macro, which in get_suggestion rules is close enough to the actual header guard macro (i.e. likely misspelling), the #define is object-like with empty definition (I've followed what clang implements) and the macro isn't defined later on (at least not on the final #endif at the end of a header).
In this case it emits a warning, so that #ifndef STDIO_H #define STDOI_H ... #endif or similar misspellings can be caught. clang enables this warning by default, but I've put it into -Wall instead as it still seems to be a style warning, nothing more severe; if a header doesn't survive multiple inclusion because of the misspelling, users will get different diagnostics. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-09-11 Jakub Jelinek <ja...@redhat.com> PR preprocessor/96842 libcpp/ * include/cpplib.h (struct cpp_options): Add warn_header_guard member. (enum cpp_warning_reason): Add CPP_W_HEADER_GUARD enumerator. * internal.h (struct cpp_reader): Add mi_def_cmacro, mi_loc and mi_def_loc members. (_cpp_defined_macro_p): Constify type pointed by argument type. Formatting fix. * init.cc (cpp_create_reader): Clear CPP_OPTION (pfile, warn_header_guard). * directives.cc (struct if_stack): Add def_loc and mi_def_cmacro members. (DIRECTIVE_TABLE): Add IF_COND flag to define. (do_define): Set ifs->mi_def_cmacro on a define immediately following #ifndef directive for the guard. Clear pfile->mi_valid. Formatting fix. (do_endif): Copy over pfile->mi_def_cmacro and pfile->mi_def_loc if ifs->mi_def_cmacro is set and pfile->mi_cmacro isn't a defined macro. (push_conditional): Clear mi_def_cmacro and mi_def_loc members. * files.cc (_cpp_pop_file_buffer): Emit -Wheader-guard diagnostics. gcc/ * doc/invoke.texi (Wheader-guard): Document. gcc/c-family/ * c.opt (Wheader-guard): New option. * c.opt.urls: Regenerated. * c-ppoutput.cc (init_pp_output): Initialize also cb->get_suggestion. gcc/testsuite/ * c-c++-common/cpp/Wheader-guard-1.c: New test. * c-c++-common/cpp/Wheader-guard-1-1.h: New test. * c-c++-common/cpp/Wheader-guard-1-2.h: New test. * c-c++-common/cpp/Wheader-guard-1-3.h: New test. * c-c++-common/cpp/Wheader-guard-1-4.h: New test. * c-c++-common/cpp/Wheader-guard-1-5.h: New test. * c-c++-common/cpp/Wheader-guard-1-6.h: New test. * c-c++-common/cpp/Wheader-guard-1-7.h: New test. * c-c++-common/cpp/Wheader-guard-1-8.h: New test. * c-c++-common/cpp/Wheader-guard-1-9.h: New test. * c-c++-common/cpp/Wheader-guard-1-10.h: New test. * c-c++-common/cpp/Wheader-guard-1-11.h: New test. * c-c++-common/cpp/Wheader-guard-1-12.h: New test. * c-c++-common/cpp/Wheader-guard-2.c: New test. * c-c++-common/cpp/Wheader-guard-2.h: New test. * c-c++-common/cpp/Wheader-guard-3.c: New test. * c-c++-common/cpp/Wheader-guard-3.h: New test. --- libcpp/include/cpplib.h.jj 2024-09-03 16:47:47.323031836 +0200 +++ libcpp/include/cpplib.h 2024-09-11 16:39:36.373680969 +0200 @@ -435,6 +435,10 @@ struct cpp_options /* Different -Wimplicit-fallthrough= levels. */ unsigned char cpp_warn_implicit_fallthrough; + /* Nonzero means warn about a define of a different macro right after + #ifndef/#if !defined header guard directive. */ + unsigned char warn_header_guard; + /* Nonzero means we should look for header.gcc files that remap file names. */ unsigned char remap; @@ -702,7 +706,8 @@ enum cpp_warning_reason { CPP_W_EXPANSION_TO_DEFINED, CPP_W_BIDIRECTIONAL, CPP_W_INVALID_UTF8, - CPP_W_UNICODE + CPP_W_UNICODE, + CPP_W_HEADER_GUARD }; /* Callback for header lookup for HEADER, which is the name of a --- libcpp/internal.h.jj 2024-09-03 16:47:47.324031823 +0200 +++ libcpp/internal.h 2024-09-11 17:09:26.481097532 +0200 @@ -493,9 +493,11 @@ struct cpp_reader been used. */ bool seen_once_only; - /* Multiple include optimization. */ + /* Multiple include optimization and -Wheader-guard warning. */ const cpp_hashnode *mi_cmacro; const cpp_hashnode *mi_ind_cmacro; + const cpp_hashnode *mi_def_cmacro; + location_t mi_loc, mi_def_loc; bool mi_valid; /* Lexing. */ @@ -676,7 +678,8 @@ _cpp_in_main_source_file (cpp_reader *pf } /* True if NODE is a macro for the purposes of ifdef, defined etc. */ -inline bool _cpp_defined_macro_p (cpp_hashnode *node) +inline bool +_cpp_defined_macro_p (const cpp_hashnode *node) { /* Do not treat conditional macros as being defined. This is due to the powerpc port using conditional macros for 'vector', 'bool', --- libcpp/init.cc.jj 2024-09-03 16:47:47.323031836 +0200 +++ libcpp/init.cc 2024-09-11 20:07:35.533395061 +0200 @@ -228,6 +228,7 @@ cpp_create_reader (enum c_lang lang, cpp CPP_OPTION (pfile, warn_variadic_macros) = 1; CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1; CPP_OPTION (pfile, cpp_warn_implicit_fallthrough) = 0; + CPP_OPTION (pfile, warn_header_guard) = 0; /* By default, track locations of tokens resulting from macro expansion. The '2' means, track the locations with the highest accuracy. Read the comments for struct --- libcpp/directives.cc.jj 2024-09-03 16:47:47.299032147 +0200 +++ libcpp/directives.cc 2024-09-11 18:52:42.000000000 +0200 @@ -31,7 +31,10 @@ struct if_stack { struct if_stack *next; location_t line; /* Line where condition started. */ - const cpp_hashnode *mi_cmacro;/* macro name for #ifndef around entire file */ + location_t def_loc; /* Locus of following #define if any. */ + const cpp_hashnode *mi_cmacro;/* Macro name for #ifndef around entire + file. */ + const cpp_hashnode *mi_def_cmacro; /* Macro name in following #define. */ bool skip_elses; /* Can future #else / #elif be skipped? */ bool was_skipping; /* If were skipping on entry. */ int type; /* Most recent conditional for diagnostics. */ @@ -144,7 +147,7 @@ static void cpp_pop_definition (cpp_read where the extension appears to have come from. */ #define DIRECTIVE_TABLE \ - D(define, T_DEFINE = 0, KANDR, IN_I) \ + D(define, T_DEFINE = 0, KANDR, IN_I | IF_COND) \ D(include, T_INCLUDE, KANDR, INCL | EXPAND) \ D(endif, T_ENDIF, KANDR, COND) \ D(ifdef, T_IFDEF, KANDR, COND | IF_COND) \ @@ -661,8 +664,8 @@ do_define (cpp_reader *pfile) /* If we have been requested to expand comments into macros, then re-enable saving of comments. */ - pfile->state.save_comments = - ! CPP_OPTION (pfile, discard_comments_in_macro_exp); + pfile->state.save_comments + = ! CPP_OPTION (pfile, discard_comments_in_macro_exp); if (pfile->cb.before_define) pfile->cb.before_define (pfile); @@ -672,7 +675,28 @@ do_define (cpp_reader *pfile) pfile->cb.define (pfile, pfile->directive_line, node); node->flags &= ~NODE_USED; + + if (pfile->mi_valid + && !pfile->mi_cmacro + && CPP_OPTION (pfile, warn_header_guard) + && node->type == NT_USER_MACRO + && node->value.macro + && node->value.macro->count == 0 + && !node->value.macro->fun_like) + { + cpp_buffer *buffer = pfile->buffer; + struct if_stack *ifs = buffer->if_stack; + if (ifs + && !ifs->next + && ifs->mi_cmacro + && node != ifs->mi_cmacro) + { + ifs->mi_def_cmacro = node; + ifs->def_loc = pfile->directive_line; + } + } } + pfile->mi_valid = false; } /* Handle #undef. Mark the identifier NT_VOID in the hash table. */ @@ -2251,6 +2275,13 @@ do_endif (cpp_reader *pfile) { pfile->mi_valid = true; pfile->mi_cmacro = ifs->mi_cmacro; + pfile->mi_loc = ifs->line; + pfile->mi_def_cmacro = NULL; + if (ifs->mi_def_cmacro && !_cpp_defined_macro_p (pfile->mi_cmacro)) + { + pfile->mi_def_cmacro = ifs->mi_def_cmacro; + pfile->mi_def_loc = ifs->def_loc; + } } buffer->if_stack = ifs->next; @@ -2272,6 +2303,7 @@ push_conditional (cpp_reader *pfile, int ifs = XOBNEW (&pfile->buffer_ob, struct if_stack); ifs->line = pfile->directive_line; + ifs->def_loc = 0; ifs->next = buffer->if_stack; ifs->skip_elses = pfile->state.skipping || !skip; ifs->was_skipping = pfile->state.skipping; @@ -2281,6 +2313,7 @@ push_conditional (cpp_reader *pfile, int ifs->mi_cmacro = cmacro; else ifs->mi_cmacro = 0; + ifs->mi_def_cmacro = 0; pfile->state.skipping = skip; buffer->if_stack = ifs; --- libcpp/files.cc.jj 2024-09-03 16:47:47.322031849 +0200 +++ libcpp/files.cc 2024-09-11 19:32:43.754868132 +0200 @@ -1664,7 +1664,28 @@ _cpp_pop_file_buffer (cpp_reader *pfile, /* Record the inclusion-preventing macro, which could be NULL meaning no controlling macro. */ if (pfile->mi_valid && file->cmacro == NULL) - file->cmacro = pfile->mi_cmacro; + { + file->cmacro = pfile->mi_cmacro; + if (pfile->mi_cmacro + && pfile->mi_def_cmacro + && pfile->cb.get_suggestion) + { + const char *names[] + = { (const char *) NODE_NAME (pfile->mi_def_cmacro), NULL }; + if (pfile->cb.get_suggestion (pfile, + (const char *) + NODE_NAME (pfile->mi_cmacro), names) + && cpp_warning_with_line (pfile, CPP_W_HEADER_GUARD, + pfile->mi_loc, 0, + "header guard \"%s\" followed by " + "\"#define\" of a different macro", + NODE_NAME (pfile->mi_cmacro))) + cpp_error_at (pfile, CPP_DL_NOTE, pfile->mi_def_loc, + "\"%s\" is defined here; did you mean \"%s\"?", + NODE_NAME (pfile->mi_def_cmacro), + NODE_NAME (pfile->mi_cmacro)); + } + } /* Invalidate control macros in the #including file. */ pfile->mi_valid = false; --- gcc/doc/invoke.texi.jj 2024-09-09 09:27:38.841086571 +0200 +++ gcc/doc/invoke.texi 2024-09-11 20:23:10.033730119 +0200 @@ -371,7 +371,7 @@ Objective-C and Objective-C++ Dialects}. -Wformat-security -Wformat-signedness -Wformat-truncation=@var{n} -Wformat-y2k -Wframe-address -Wframe-larger-than=@var{byte-size} -Wno-free-nonheap-object --Wno-if-not-aligned -Wno-ignored-attributes +-Wheader-guard -Wno-if-not-aligned -Wno-ignored-attributes -Wignored-qualifiers -Wno-incompatible-pointer-types -Whardened -Wimplicit -Wimplicit-fallthrough -Wimplicit-fallthrough=@var{n} -Wno-implicit-function-declaration -Wno-implicit-int @@ -10096,6 +10096,19 @@ Do not warn if certain built-in macros a warnings for redefinition of @code{__TIMESTAMP__}, @code{__TIME__}, @code{__DATE__}, @code{__FILE__}, and @code{__BASE_FILE__}. +@opindex Wheader-guard +@item -Wheader-guard +Warn if a valid preprocessor header multiple inclusion guard has +a @code{#define} directive right after @code{#ifndef} or @code{#if !defined} +directive for the multiple inclusion guard, which defines a different macro +from the guard macro with a similar name, the actual multiple inclusion guard +macro isn't defined at the corresponding @code{#ifndef} directive at the end +of the header, and the @code{#define} directive defines object-like macro with +empty definition. In such case, it often is just misspelled guard name, +either in the @code{#ifndef} or @code{#if !defined} directive or in the +subsequent @code{#define} directive. This warning is enabled +by @option{-Wall}. + @opindex Wstrict-prototypes @opindex Wno-strict-prototypes @item -Wstrict-prototypes @r{(C and Objective-C only)} --- gcc/c-family/c.opt.jj 2024-09-03 16:47:47.185033625 +0200 +++ gcc/c-family/c.opt 2024-09-11 20:07:15.786662712 +0200 @@ -814,6 +814,10 @@ Wglobal-module C++ ObjC++ Var(warn_global_module) Warning Init(1) Warn about the global module fragment not containing only preprocessing directives. +Wheader-guard +C ObjC C++ ObjC++ CPP(warn_header_guard) CppReason(CPP_W_HEADER_GUARD) Var(cpp_warn_header_guard) Init(0) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn when #ifndef of a header guard is followed by #define of a different macro with the header guard macro not defined at the end of header. + Wif-not-aligned C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning Warn when the field in a struct is not aligned. --- gcc/c-family/c.opt.urls.jj 2024-08-14 18:19:28.731129432 +0200 +++ gcc/c-family/c.opt.urls 2024-09-11 20:24:22.262751351 +0200 @@ -412,6 +412,9 @@ UrlSuffix(gcc/Warning-Options.html#index Wglobal-module UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wglobal-module) +Wheader-guard +UrlSuffix(gcc/Warning-Options.html#index-Wheader-guard) + Wif-not-aligned UrlSuffix(gcc/Warning-Options.html#index-Wif-not-aligned) --- gcc/c-family/c-ppoutput.cc.jj 2024-08-05 13:04:53.489121581 +0200 +++ gcc/c-family/c-ppoutput.cc 2024-09-11 20:02:06.500854915 +0200 @@ -164,6 +164,7 @@ init_pp_output (FILE *out_stream) cb->has_builtin = c_common_has_builtin; cb->has_feature = c_common_has_feature; cb->get_source_date_epoch = cb_get_source_date_epoch; + cb->get_suggestion = cb_get_suggestion; cb->remap_filename = remap_macro_filename; /* Initialize the print structure. */ --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1.c.jj 2024-09-11 19:26:43.967778739 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1.c 2024-09-11 19:26:20.772095324 +0200 @@ -0,0 +1,18 @@ +/* PR preprocessor/96842 */ +/* { dg-do preprocess } */ + +#include "Wheader-guard-1-1.h" +#include "Wheader-guard-1-2.h" +#include "Wheader-guard-1-3.h" +#include "Wheader-guard-1-4.h" +#include "Wheader-guard-1-5.h" +#include "Wheader-guard-1-6.h" +#include "Wheader-guard-1-7.h" +#define WHEADER_GUARD_8 +#include "Wheader-guard-1-8.h" +#include "Wheader-guard-1-9.h" +#include "Wheader-guard-1-10.h" +#include "Wheader-guard-1-11.h" +#include "Wheader-guard-1-12.h" + +int i; --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-1.h.jj 2024-09-11 19:26:39.912834079 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-1.h 2024-09-11 19:02:02.414054285 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_1 +#define WHEADER_GUARD_1 +/* This is how header guards should look like. */ +#define SOMETHING1 123 +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-2.h.jj 2024-09-11 19:26:39.914834052 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-2.h 2024-09-11 19:11:14.876484333 +0200 @@ -0,0 +1,4 @@ +#ifndef WHEADER_GUARD_2 +#define WHEADERGUARD2 1 +/* Don't warn if the different macro defines some tokens. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-3.h.jj 2024-09-11 19:26:39.915834039 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-3.h 2024-09-11 19:11:23.834361776 +0200 @@ -0,0 +1,4 @@ +#ifndef WHEADER_GUARD_3 +#define WHEADERGUARD3() +/* Won't warn if it is a function-like macro. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-4.h.jj 2024-09-11 19:26:39.918833998 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-4.h 2024-09-11 19:04:18.569185598 +0200 @@ -0,0 +1,3 @@ +#ifndef WHEADER_GUARD_4 +/* Don't warn if there is no define after #ifndef. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-5.h.jj 2024-09-11 19:26:39.919833984 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-5.h 2024-09-11 19:11:46.934045743 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_5 +int guard5; +#define WHEADERGUARD5 +/* Don't warn if there are tokens in between #ifndef and #define. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-6.h.jj 2024-09-11 19:26:39.921833957 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-6.h 2024-09-11 19:11:59.357875762 +0200 @@ -0,0 +1,8 @@ +#ifndef WHEADER_GUARD_6 +#define WHEADERGUARD6 +/* Don't warn if WHEADER_GUARD_6 is eventually defined later. */ +#if 0 +#else +#define WHEADER_GUARD_6 +#endif +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-7.h.jj 2024-09-11 19:26:39.923833930 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-7.h 2024-09-11 19:22:21.860358981 +0200 @@ -0,0 +1,4 @@ +#ifndef WHEADER_GUARD_7 +#define SOMETHING7 +/* Don't warn if the two macros don't have similar names. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-8.h.jj 2024-09-11 19:26:39.925833903 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-8.h 2024-09-11 19:23:13.884647219 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_8 +#define WHEADERGUARD8 +/* Don't warn if the guard macro is already defined before + including the header. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-9.h.jj 2024-09-11 19:26:39.926833889 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-9.h 2024-09-11 19:24:34.647543778 +0200 @@ -0,0 +1,5 @@ +int guard9; +#ifndef WHEADER_GUARD_9 +#define WHEADERGUARD9 +/* Don't warn if it actually isn't a valid header guard. */ +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-10.h.jj 2024-09-11 19:26:39.928833862 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-10.h 2024-09-11 19:25:02.012170293 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_10 +#define WHEADERGUARD10 +/* Don't warn if it actually isn't a valid header guard. */ +#else +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-11.h.jj 2024-09-11 19:26:39.930833835 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-11.h 2024-09-11 19:25:30.972775022 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_11 +#define WHEADERGUARD11 +/* Don't warn if it actually isn't a valid header guard. */ +#elif 1 +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-12.h.jj 2024-09-11 19:26:39.932833808 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-1-12.h 2024-09-11 19:25:56.790422645 +0200 @@ -0,0 +1,5 @@ +#ifndef WHEADER_GUARD_12 +#define WHEADERGUARD12 +/* Don't warn if it actually isn't a valid header guard. */ +#endif +#define ASOMETHING12 --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-2.c.jj 2024-09-11 19:28:02.611705351 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-2.c 2024-09-11 20:12:47.124171614 +0200 @@ -0,0 +1,10 @@ +/* PR preprocessor/96842 */ +/* { dg-do preprocess } */ +/* { dg-options "-Wheader-guard" } */ + +#include "Wheader-guard-2.h" + +int i; + +/* { dg-warning "header guard \"WHEADER_GUARD_2\" followed by \"#define\" of a different macro" "" { target *-*-* } 0 } */ +/* { dg-message "\"WHEADERGUARD2\" is defined here; did you mean \"WHEADER_GUARD_2\"\\\?" "" { target *-*-* } 0 } */ --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-2.h.jj 2024-09-11 19:28:45.893114621 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-2.h 2024-09-11 19:28:40.376189917 +0200 @@ -0,0 +1,4 @@ +#ifndef WHEADER_GUARD_2 +#define WHEADERGUARD2 +#define SOMETHING2 123 +#endif --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-3.c.jj 2024-09-11 19:29:52.333207805 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-3.c 2024-09-11 20:13:17.867754901 +0200 @@ -0,0 +1,10 @@ +/* PR preprocessor/96842 */ +/* { dg-do preprocess } */ +/* { dg-options "-Wall" } */ + +#include "Wheader-guard-3.h" + +int i; + +/* { dg-warning "header guard \"WHEADER_GUARD_3\" followed by \"#define\" of a different macro" "" { target *-*-* } 0 } */ +/* { dg-message "\"WHEADERGUARD3\" is defined here; did you mean \"WHEADER_GUARD_3\"\\\?" "" { target *-*-* } 0 } */ --- gcc/testsuite/c-c++-common/cpp/Wheader-guard-3.h.jj 2024-09-11 19:30:09.454974114 +0200 +++ gcc/testsuite/c-c++-common/cpp/Wheader-guard-3.h 2024-09-11 19:30:23.562781561 +0200 @@ -0,0 +1,4 @@ +#if !defined(WHEADER_GUARD_3) +#define WHEADERGUARD3 +#define SOMETHING3 123 +#endif Jakub