[cfe-commits] r166498 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp test/Sema/warn-missing-variable-declarations.c test/Sema/warn-missing-variable-declarations.cpp

[Eli Friedman]

On Wed, Oct 24, 2012 at 11:36 AM, Matthieu Monrocq
<matthieu.monrocq at gmail.com> wrote:
>
>
> On Tue, Oct 23, 2012 at 10:19 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
>>
>> Author: efriedma
>> Date: Tue Oct 23 15:19:32 2012
>> New Revision: 166498
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=166498&view=rev
>> Log:
>> Add a new warning -Wmissing-variable-declarations, to warn about variables
>> defined without a previous declaration.  This is similar to
>> -Wmissing-prototypes, but for variables instead of functions.
>>
>> Patch by Ed Schouten.
>>
>>
>> Added:
>>     cfe/trunk/test/Sema/warn-missing-variable-declarations.c
>>     cfe/trunk/test/Sema/warn-missing-variable-declarations.cpp
>> Modified:
>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>     cfe/trunk/lib/Sema/Sema.cpp
>>     cfe/trunk/lib/Sema/SemaDecl.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=166498&r1=166497&r2=166498&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 23
>> 15:19:32 2012
>> @@ -3139,6 +3139,9 @@
>>  def warn_missing_prototype : Warning<
>>    "no previous prototype for function %0">,
>>    InGroup<DiagGroup<"missing-prototypes">>, DefaultIgnore;
>> +def warn_missing_variable_declarations : Warning<
>> +  "no previous extern declaration for non-static variable %0">,
>> +  InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
>>  def err_redefinition : Error<"redefinition of %0">;
>>  def err_definition_of_implicitly_declared_member : Error<
>>    "definition of implicitly declared %select{default constructor|copy "
>>
>> Modified: cfe/trunk/lib/Sema/Sema.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=166498&r1=166497&r2=166498&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/Sema.cpp (original)
>> +++ cfe/trunk/lib/Sema/Sema.cpp Tue Oct 23 15:19:32 2012
>> @@ -665,6 +665,8 @@
>>
>> diag::err_tentative_def_incomplete_type))
>>        VD->setInvalidDecl();
>>
>> +    CheckCompleteVariableDeclaration(VD);
>> +
>>      // Notify the consumer that we've completed a tentative definition.
>>      if (!VD->isInvalidDecl())
>>        Consumer.CompleteTentativeDefinition(VD);
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=166498&r1=166497&r2=166498&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct 23 15:19:32 2012
>> @@ -7148,6 +7148,17 @@
>>      }
>>    }
>>
>> +  if (var->isThisDeclarationADefinition() &&
>> +      var->getLinkage() == ExternalLinkage) {
>> +    // Find a previous declaration that's not a definition.
>> +    VarDecl *prev = var->getPreviousDecl();
>> +    while (prev && prev->isThisDeclarationADefinition())
>> +      prev = prev->getPreviousDecl();
>> +
>> +    if (!prev)
>> +      Diag(var->getLocation(), diag::warn_missing_variable_declarations)
>> << var;
>> +  }
>> +
>>    // All the following checks are C++ only.
>>    if (!getLangOpts().CPlusPlus) return;
>>
>>
>> Added: cfe/trunk/test/Sema/warn-missing-variable-declarations.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-missing-variable-declarations.c?rev=166498&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/warn-missing-variable-declarations.c (added)
>> +++ cfe/trunk/test/Sema/warn-missing-variable-declarations.c Tue Oct 23
>> 15:19:32 2012
>> @@ -0,0 +1,36 @@
>> +// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang
>> -verify %s
>> +
>> +int vbad1; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad1'}}
>> +
>> +int vbad2;
>> +int vbad2 = 10; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad2'}}
>> +
>> +struct {
>> +  int mgood1;
>> +} vbad3; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad3'}}
>> +
>> +int vbad4;
>> +int vbad4 = 10; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad4'}}
>> +extern int vbad4;
>> +
>> +extern int vgood1;
>> +int vgood1;
>> +int vgood1 = 10;
>> +// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang
>> -verify %s
>> +
>> +int vbad1; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad1'}}
>> +
>> +int vbad2;
>> +int vbad2 = 10; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad2'}}
>> +
>> +struct {
>> +  int mgood1;
>> +} vbad3; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad3'}}
>> +
>> +int vbad4;
>> +int vbad4 = 10; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad4'}}
>> +extern int vbad4;
>> +
>> +extern int vgood1;
>> +int vgood1;
>> +int vgood1 = 10;
>>
> Did I miss something or is this test case "doubled" ?

Fixed in a subsequent commit.

>>
>> Added: cfe/trunk/test/Sema/warn-missing-variable-declarations.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-missing-variable-declarations.cpp?rev=166498&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/warn-missing-variable-declarations.cpp (added)
>> +++ cfe/trunk/test/Sema/warn-missing-variable-declarations.cpp Tue Oct 23
>> 15:19:32 2012
>> @@ -0,0 +1,39 @@
>> +// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang
>> -verify %s
>> +
>> +// Variable declarations that should trigger a warning.
>> +int vbad1; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad1'}}
>> +int vbad2 = 10; // expected-warning{{no previous extern declaration for
>> non-static variable 'vbad2'}}
>> +
>> +// Variable declarations that should not trigger a warning.
>> +static int vgood1;
>> +extern int vgood2;
>> +int vgood2;
>> +static struct {
>> +  int mgood1;
>> +} vgood3;
>> +
>> +// Functions should never trigger a warning.
>> +void fgood1(void);
>> +void fgood2(void) {
>> +  int lgood1;
>> +  static int lgood2;
>> +}
>> +static void fgood3(void) {
>> +  int lgood3;
>> +  static int lgood4;
>> +}
>> +
>> +// Structures, namespaces and classes should be unaffected.
>> +struct sgood1 {
>> +  int mgood2;
>> +};
>> +struct {
>> +  int mgood3;
>> +} sgood2;
>> +class CGood1 {
>> +  static int MGood1;
>> +};
>> +int CGood1::MGood1;
>> +namespace {
>> +  int mgood4;
>> +}
>>
>>
>
> How does this interact with "static" variables (at file scope) ?

Not affected.

> Also, there is no test in "named" namespaces, should not this be covered too
> ?

Sure; I can add something.

-Eli