[cfe-commits] r167783 - in /cfe/trunk: include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp test/SemaCXX/warn-unused-result.cpp

[Kaelyn Uhrain]

On Mon, Nov 12, 2012 at 3:56 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Mon, Nov 12, 2012 at 3:48 PM, Kaelyn Uhrain <rikka at google.com> wrote:
> > Author: rikka
> > Date: Mon Nov 12 17:48:05 2012
> > New Revision: 167783
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=167783&view=rev
> > Log:
> > Enable C++11 attribute syntax for warn_unused_result and allow it to be
> > applied to CXXRecordDecls, where functions with that return type will
> > inherit the warn_unused_result attribute.
> >
> > Also includes a tiny fix (with no discernable behavior change for
> > existing code) to re-sync AttributeDeclKind enum and
> > err_attribute_wrong_decl_type with warn_attribute_wrong_decl_type since
> > the enum is used with both diagnostic messages to chose the correct
> > description.
> >
> > Modified:
> >     cfe/trunk/include/clang/Basic/Attr.td
> >     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >     cfe/trunk/lib/Sema/SemaDecl.cpp
> >     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >     cfe/trunk/test/SemaCXX/warn-unused-result.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=167783&r1=167782&r2=167783&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Mon Nov 12 17:48:05 2012
> > @@ -699,7 +699,7 @@
> >  }
> >
> >  def WarnUnusedResult : InheritableAttr {
> > -  let Spellings = [GNU<"warn_unused_result">];
> > +  let Spellings = [GNU<"warn_unused_result">,
> CXX11<"","warn_unused_result">];
>
> Please use CXX11<"clang", "warn_unused_result">.
>

Fixed in r167791.

>
> >  }
> >
> >  def Weak : InheritableAttr {
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=167783&r1=167782&r2=167783&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Nov 12
> 17:48:05 2012
> > @@ -1773,17 +1773,18 @@
> >  def warn_attribute_wrong_decl_type : Warning<
> >    "%0 attribute only applies to %select{functions|unions|"
> >    "variables and functions|functions and methods|parameters|"
> > -  "functions, methods and blocks|functions, methods, and parameters|"
> > -  "classes|variables|methods|variables, functions and labels|"
> > -  "fields and global variables|structs|"
> > +  "functions, methods and blocks|functions, methods, and classes|"
> > +  "functions, methods, and parameters|classes|variables|methods|"
> > +  "variables, functions and labels|fields and global variables|structs|"
> >    "variables, functions and tag types|thread-local variables}1">,
> >    InGroup<IgnoredAttributes>;
> >  def err_attribute_wrong_decl_type : Error<
> >    "%0 attribute only applies to %select{functions|unions|"
> >    "variables and functions|functions and methods|parameters|"
> > -  "functions, methods and blocks|functions, methods, and parameters|"
> > -  "classes|variables|methods|variables, functions and labels|"
> > -  "fields and global variables|structs|thread-local variables}1">;
> > +  "functions, methods and blocks|functions, methods, and classes|"
> > +  "functions, methods, and parameters|classes|variables|methods|"
> > +  "variables, functions and labels|fields and global variables|structs|"
> > +  "variables, functions and tag types|thread-local variables}1">;
> >  def warn_function_attribute_wrong_type : Warning<
> >    "'%0' only applies to function types; type here is %1">,
> >    InGroup<IgnoredAttributes>;
> >
> > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=167783&r1=167782&r2=167783&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Nov 12 17:48:05 2012
> > @@ -5691,6 +5691,14 @@
> >    ProcessDeclAttributes(S, NewFD, D,
> >                          /*NonInheritable=*/false, /*Inheritable=*/true);
> >
> > +  QualType RetType = NewFD->getResultType();
> > +  const CXXRecordDecl *Ret = RetType->isRecordType() ?
> > +      RetType->getAsCXXRecordDecl() :
> RetType->getPointeeCXXRecordDecl();
> > +  if (!NewFD->isInvalidDecl() &&
> !NewFD->hasAttr<WarnUnusedResultAttr>() &&
> > +      Ret && Ret->hasAttr<WarnUnusedResultAttr>()) {
> > +    NewFD->addAttr(new (Context) WarnUnusedResultAttr(SourceRange(),
> Context));
> > +  }
> > +
> >    if (!getLangOpts().CPlusPlus) {
> >      // Perform semantic checking on the function declaration.
> >      bool isExplicitSpecialization=false;
> >
> > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=167783&r1=167782&r2=167783&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Nov 12 17:48:05 2012
> > @@ -37,6 +37,7 @@
> >    ExpectedFunctionOrMethod,
> >    ExpectedParameter,
> >    ExpectedFunctionMethodOrBlock,
> > +  ExpectedFunctionMethodOrClass,
> >    ExpectedFunctionMethodOrParameter,
> >    ExpectedClass,
> >    ExpectedVariable,
> > @@ -44,6 +45,7 @@
> >    ExpectedVariableFunctionOrLabel,
> >    ExpectedFieldOrGlobalVar,
> >    ExpectedStruct,
> > +  ExpectedVariableFunctionOrTag,
>
> Is this enum value used anywhere?
>

Not currently, but without it the enum doesn't match up to the choices in
(warn|err)_attribute_wrong_decl_type--specifically ExpectedTLSVar, which
had the right value for one of the diagnostics but not the other. The two
diagnostics being out of sync with each other and with this enum is also a
potential landmine for anyone modifying either diagnostic in the future,
particularly since the comment on the enum claims it matches the choices in
both diagnostics.


> >    ExpectedTLSVar
> >  };
> >
> > @@ -2445,7 +2447,7 @@
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return;
> >
> > -  if (!isFunction(D) && !isa<ObjCMethodDecl>(D)) {
> > +  if (!isFunction(D) && !isa<ObjCMethodDecl>(D) &&
> !isa<CXXRecordDecl>(D)) {
> >      S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> >        << Attr.getName() << ExpectedFunctionOrMethod;
>
> Should this be updated to ExpectedFunctionMethodOrClass?
>

Whoops! Good catch :)

>
> >      return;
> >
> > Modified: cfe/trunk/test/SemaCXX/warn-unused-result.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-result.cpp?rev=167783&r1=167782&r2=167783&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/SemaCXX/warn-unused-result.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/warn-unused-result.cpp Mon Nov 12 17:48:05
> 2012
> > @@ -1,4 +1,4 @@
> > -// RUN: %clang_cc1 -fsyntax-only -verify %s
> > +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
> >
> >  int f() __attribute__((warn_unused_result));
> >
> > @@ -42,3 +42,33 @@
> >    x.foo(); // expected-warning {{ignoring return value}}
> >    x2->foo(); // expected-warning {{ignoring return value}}
> >  }
> > +
> > +namespace warn_unused_CXX11 {
> > +struct [[warn_unused_result]] Status {
> > +  bool ok() const;
> > +};
> > +Status DoSomething();
> > +Status& DoSomethingElse();
> > +Status* DoAnotherThing();
> > +Status** DoYetAnotherThing();
> > +void lazy() {
> > +  Status s = DoSomething();
> > +  if (!s.ok()) return;
> > +  Status &rs = DoSomethingElse();
> > +  if (!rs.ok()) return;
> > +  Status *ps = DoAnotherThing();
> > +  if (!ps->ok()) return;
> > +  Status **pps = DoYetAnotherThing();
> > +  if (!(*pps)->ok()) return;
> > +
> > +  (void)DoSomething();
> > +  (void)DoSomethingElse();
> > +  (void)DoAnotherThing();
> > +  (void)DoYetAnotherThing();
> > +
> > +  DoSomething(); // expected-warning {{ignoring return value}}
> > +  DoSomethingElse(); // expected-warning {{ignoring return value}}
> > +  DoAnotherThing(); // expected-warning {{ignoring return value}}
> > +  DoYetAnotherThing();
> > +}
> > +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121112/fcd56265/attachment.html>