Teach -Wigored-qualifiers about exotic flavors of declarator and the _Atomic type qualifier.

llvm-svn: 178217

clang/lib/Sema/SemaType.cpp

@@ -31,6 +31,7 @@
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/Template.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 using namespace clang;
 
@@ -1880,60 +1881,117 @@ static void inferARCWriteback(TypeProcessingState &state,
   // TODO: mark whether we did this inference?
 }
 
-static void DiagnoseIgnoredQualifiers(unsigned Quals,
+static void diagnoseIgnoredQualifiers(
-                                      SourceLocation ConstQualLoc,
+    Sema &S, unsigned Quals,
-                                      SourceLocation VolatileQualLoc,
+    SourceLocation FallbackLoc,
-                                      SourceLocation RestrictQualLoc,
+    SourceLocation ConstQualLoc = SourceLocation(),
-                                      SourceLocation AtomicQualLoc,
+    SourceLocation VolatileQualLoc = SourceLocation(),
-                                      Sema& S) {
+    SourceLocation RestrictQualLoc = SourceLocation(),
-  std::string QualStr;
+    SourceLocation AtomicQualLoc = SourceLocation()) {
-  unsigned NumQuals = 0;
+  assert(Quals && "no qualifiers to diagnose");
-  SourceLocation Loc;
-
-  FixItHint ConstFixIt;
-  FixItHint VolatileFixIt;
-  FixItHint RestrictFixIt;
-  FixItHint AtomicFixIt;
-
   const SourceManager &SM = S.getSourceManager();
 
-  // FIXME: The locations here are set kind of arbitrarily. It'd be nicer to
+  struct Qual {
-  // find a range and grow it to encompass all the qualifiers, regardless of
+    unsigned Mask;
-  // the order in which they textually appear.
+    const char *Name;
-  if (Quals & DeclSpec::TQ_const) {
+    SourceLocation Loc;
-    ConstFixIt = FixItHint::CreateRemoval(ConstQualLoc);
+  } const QualKinds[4] = {
-    QualStr = "const";
+    { DeclSpec::TQ_const, "const", ConstQualLoc },
-    ++NumQuals;
+    { DeclSpec::TQ_volatile, "volatile", VolatileQualLoc },
-    if (!Loc.isValid() || SM.isBeforeInTranslationUnit(ConstQualLoc, Loc))
+    { DeclSpec::TQ_restrict, "restrict", RestrictQualLoc },
-      Loc = ConstQualLoc;
+    { DeclSpec::TQ_atomic, "_Atomic", AtomicQualLoc }
-  }
+  };
-  if (Quals & DeclSpec::TQ_volatile) {
+
-    VolatileFixIt = FixItHint::CreateRemoval(VolatileQualLoc);
+  llvm::SmallString<32> QualStr;
-    QualStr += (NumQuals == 0 ? "volatile" : " volatile");
+  unsigned NumQuals = 0;
-    ++NumQuals;
+  SourceLocation Loc;
-    if (!Loc.isValid() || SM.isBeforeInTranslationUnit(VolatileQualLoc, Loc))
+  FixItHint FixIts[4];
-      Loc = VolatileQualLoc;
+
-  }
+  // Build a string naming the redundant qualifiers.
-  if (Quals & DeclSpec::TQ_restrict) {
+  for (unsigned I = 0; I != 4; ++I) {
-    RestrictFixIt = FixItHint::CreateRemoval(RestrictQualLoc);
+    if (Quals & QualKinds[I].Mask) {
-    QualStr += (NumQuals == 0 ? "restrict" : " restrict");
+      if (!QualStr.empty()) QualStr += ' ';
-    ++NumQuals;
+      QualStr += QualKinds[I].Name;
-    if (!Loc.isValid() || SM.isBeforeInTranslationUnit(RestrictQualLoc, Loc))
+
-      Loc = RestrictQualLoc;
+      // If we have a location for the qualifier, offer a fixit.
-  }
+      SourceLocation QualLoc = QualKinds[I].Loc;
-  if (Quals & DeclSpec::TQ_atomic) {
+      if (!QualLoc.isInvalid()) {
-    AtomicFixIt = FixItHint::CreateRemoval(AtomicQualLoc);
+        FixIts[NumQuals] = FixItHint::CreateRemoval(QualLoc);
-    QualStr += (NumQuals == 0 ? "_Atomic" : " _Atomic");
+        if (Loc.isInvalid() || SM.isBeforeInTranslationUnit(QualLoc, Loc))
-    ++NumQuals;
+          Loc = QualLoc;
-    if (!Loc.isValid() || SM.isBeforeInTranslationUnit(AtomicQualLoc, Loc))
-      Loc = AtomicQualLoc;
       }
 
-  assert(NumQuals > 0 && "No known qualifiers?");
+      ++NumQuals;
+    }
+  }
 
-  S.Diag(Loc, diag::warn_qual_return_type)
+  S.Diag(Loc.isInvalid() ? FallbackLoc : Loc, diag::warn_qual_return_type)
-    << QualStr << NumQuals
+    << QualStr << NumQuals << FixIts[0] << FixIts[1] << FixIts[2] << FixIts[3];
-    << ConstFixIt << VolatileFixIt << RestrictFixIt << AtomicFixIt;
+}
+
+// Diagnose pointless type qualifiers on the return type of a function.
+static void diagnoseIgnoredFunctionQualifiers(Sema &S, QualType RetTy,
+                                              Declarator &D,
+                                              unsigned FunctionChunkIndex) {
+  unsigned AtomicQual = RetTy->isAtomicType() ? DeclSpec::TQ_atomic : 0;
+
+  if (D.getTypeObject(FunctionChunkIndex).Fun.hasTrailingReturnType()) {
+    // FIXME: TypeSourceInfo doesn't preserve location information for
+    // qualifiers.
+    diagnoseIgnoredQualifiers(S, RetTy.getCVRQualifiers() | AtomicQual,
+                              D.getIdentifierLoc());
+    return;
+  }
+
+  for (unsigned OuterChunkIndex = FunctionChunkIndex + 1,
+                End = D.getNumTypeObjects();
+       OuterChunkIndex != End; ++OuterChunkIndex) {
+    DeclaratorChunk &OuterChunk = D.getTypeObject(OuterChunkIndex);
+    switch (OuterChunk.Kind) {
+    case DeclaratorChunk::Paren:
+      continue;
+
+    case DeclaratorChunk::Pointer: {
+      DeclaratorChunk::PointerTypeInfo &PTI = OuterChunk.Ptr;
+      diagnoseIgnoredQualifiers(
+          S, PTI.TypeQuals,
+          SourceLocation(),
+          SourceLocation::getFromRawEncoding(PTI.ConstQualLoc),
+          SourceLocation::getFromRawEncoding(PTI.VolatileQualLoc),
+          SourceLocation::getFromRawEncoding(PTI.RestrictQualLoc),
+          SourceLocation::getFromRawEncoding(PTI.AtomicQualLoc));
+      return;
+    }
+
+    case DeclaratorChunk::Function:
+    case DeclaratorChunk::BlockPointer:
+    case DeclaratorChunk::Reference:
+    case DeclaratorChunk::Array:
+    case DeclaratorChunk::MemberPointer:
+      // FIXME: We can't currently provide an accurate source location and a
+      // fix-it hint for these.
+      diagnoseIgnoredQualifiers(S, RetTy.getCVRQualifiers() | AtomicQual,
+                                D.getIdentifierLoc());
+      return;
+    }
+
+    llvm_unreachable("unknown declarator chunk kind");
+  }
+
+  // If the qualifiers come from a conversion function type, don't diagnose
+  // them -- they're not necessarily redundant, since such a conversion
+  // operator can be explicitly called as "x.operator const int()".
+  if (D.getName().getKind() == UnqualifiedId::IK_ConversionFunctionId)
+    return;
+
+  // Just parens all the way out to the decl specifiers. Diagnose any qualifiers
+  // which are present there.
+  diagnoseIgnoredQualifiers(S, D.getDeclSpec().getTypeQualifiers() | AtomicQual,
+                            D.getIdentifierLoc(),
+                            D.getDeclSpec().getConstSpecLoc(),
+                            D.getDeclSpec().getVolatileSpecLoc(),
+                            D.getDeclSpec().getRestrictSpecLoc(),
+                            D.getDeclSpec().getAtomicSpecLoc());
 }
 
 static QualType GetDeclSpecTypeForDeclarator(TypeProcessingState &state,
@@ -2530,33 +2588,10 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
       // cv-qualifiers on return types are pointless except when the type is a
       // class type in C++.
-      if (isa<PointerType>(T) && T.getLocalCVRQualifiers() &&
+      if ((T.getCVRQualifiers() || T->isAtomicType()) &&
-          (D.getName().getKind() != UnqualifiedId::IK_ConversionFunctionId) &&
+          !(S.getLangOpts().CPlusPlus &&
-          (!LangOpts.CPlusPlus || !T->isDependentType())) {
+            (T->isDependentType() || T->isRecordType())))
-        assert(chunkIndex + 1 < e && "No DeclaratorChunk for the return type?");
+        diagnoseIgnoredFunctionQualifiers(S, T, D, chunkIndex);
-        DeclaratorChunk ReturnTypeChunk = D.getTypeObject(chunkIndex + 1);
-        assert(ReturnTypeChunk.Kind == DeclaratorChunk::Pointer);
-
-        DeclaratorChunk::PointerTypeInfo &PTI = ReturnTypeChunk.Ptr;
-
-        DiagnoseIgnoredQualifiers(PTI.TypeQuals,
-            SourceLocation::getFromRawEncoding(PTI.ConstQualLoc),
-            SourceLocation::getFromRawEncoding(PTI.VolatileQualLoc),
-            SourceLocation::getFromRawEncoding(PTI.RestrictQualLoc),
-            SourceLocation::getFromRawEncoding(PTI.AtomicQualLoc),
-            S);
-
-      } else if (T.getCVRQualifiers() && D.getDeclSpec().getTypeQualifiers() &&
-                 (!LangOpts.CPlusPlus ||
-                  (!T->isDependentType() && !T->isRecordType()))) {
-
-        DiagnoseIgnoredQualifiers(D.getDeclSpec().getTypeQualifiers(),
-                                  D.getDeclSpec().getConstSpecLoc(),
-                                  D.getDeclSpec().getVolatileSpecLoc(),
-                                  D.getDeclSpec().getRestrictSpecLoc(),
-                                  D.getDeclSpec().getAtomicSpecLoc(),
-                                  S);
-      }
 
       // Objective-C ARC ownership qualifiers are ignored on the function
       // return type (by type canonicalization). Complain if this attribute

clang/test/SemaCXX/return.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fcxx-exceptions -fexceptions -fsyntax-only -Wignored-qualifiers -verify
+// RUN: %clang_cc1 %s -std=c++11 -fcxx-exceptions -fexceptions -fsyntax-only -Wignored-qualifiers -verify
 
 int test1() {
   throw;
@@ -45,6 +45,28 @@ const
 j();
 
 const volatile int scalar_cv(); // expected-warning{{'const volatile' type qualifiers on return type have no effect}}
+
+// FIXME: Maintain enough information that we can point the diagnostic at the 'volatile' keyword.
+const
+int S::*
+volatile
+mixed_ret(); // expected-warning {{'volatile' type qualifier on return type has no effect}}
+
+const int volatile // expected-warning {{'const volatile' type qualifiers on return type have no effect}}
+    (((parens())));
+
+_Atomic(int)
+    atomic(); // expected-warning {{'_Atomic' type qualifier on return type has no effect}}
+
+_Atomic // expected-warning {{'_Atomic' type qualifier on return type has no effect}}
+    int
+    atomic();
+
+auto
+    trailing_return_type() -> // expected-warning {{'const' type qualifier on return type has no effect}}
+    const int;
+
+const int ret_array()[4]; // expected-error {{cannot return array}}
 }
 
 namespace PR9328 {
@@ -56,6 +78,7 @@ namespace PR9328 {
 }
 
 class foo  {
+  operator const int ();
   operator int * const ();
 };