[PATCH] Header guard warning is too agressive

[David Blaikie]

On Wed, Sep 11, 2013 at 2:10 PM, Richard Trieu <rtrieu at google.com> wrote:

>
>   If I understand the issue correctly, and from reading PR17053, the
> problem is that the main file in the translation unit would have a
> structure similar to a header guard but it should not be considered a
> header guard.  This patch does not fix that issue.  Instead, it looks at
> the edit distance between #define and #ifndef macro names.  This only
> coincidentally fixes the examples given.  The test case also does not
> follow the examples as it is still inside a header, not the main file.
>
>   It would be better to detect if the lexer is at the top most file and
> skip the diagnostic then.
>
>
> ================
> Comment at: lib/Lex/PPLexerChange.cpp:280
> @@ +279,3 @@
> +              << ControllingMacro
> +              << FixItHint::CreateReplacement(
> +
>  CurPPLexer->MIOpt.GetDefinedLocation(),
> ----------------
> Ismail Pazarbasi wrote:
> > I haven't tried thoroughly, but I guess it'd be better to print the
> fix-it on the warning instead of the note, because clang-check will ignore
> the note (and I need clang-check to apply the fixit).
> This seems like a valid point.  There is only one suggestion, so it should
> be placed in the warning instead of the note, but that change should go in
> a separate patch.


Generally we don't put semantic-changing fixits on warnings directly as, if
they are promoted to error, the fixit may be automatically applied & change
the semantics of the program (the post-fixit'd code will differ in
semantics to the compiled code/output).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130911/1895677b/attachment.html>