The current -Wignored-qualifiers warning fails at the following:

const char* const f(); // It points out the wrong 'const' as the culprit
char* const f(); // It doesn't warn at all.

Attaching a patch to fix this. I believe this also fixes part of
http://llvm.org/pr9058 (the part that concerns return type
qualifiers).

Please review.

Thanks,
Hans
Index: test/Sema/return.c
===================================================================
--- test/Sema/return.c	(revision 126218)
+++ test/Sema/return.c	(working copy)
@@ -242,6 +242,7 @@
 // Test warnings on ignored qualifiers on return types.
 const int ignored_c_quals(); // expected-warning{{'const' type qualifier on return type has no effect}}
 const volatile int ignored_cv_quals(); // expected-warning{{'const volatile' type qualifiers on return type have no effect}}
+char* const volatile restrict ignored_cvr_quals(); // expected-warning{{'const volatile restrict' type qualifiers on return type have no effect}}
 
 // Test that for switch(enum) that if the switch statement covers all the cases
 // that we don't consider that for -Wreturn-type.
@@ -254,4 +255,3 @@
   case C3: return 4;
   }
 } // no-warning
-
Index: test/SemaCXX/return.cpp
===================================================================
--- test/SemaCXX/return.cpp	(revision 126218)
+++ test/SemaCXX/return.cpp	(working copy)
@@ -24,5 +24,14 @@
 const volatile S class_cv();
 
 const int scalar_c(); // expected-warning{{'const' type qualifier on return type has no effect}}
+
+const
+char*
+const // expected-warning{{'const' type qualifier on return type has no effect}}
+f();
+
+char* const g(); // expected-warning{{'const' type qualifier on return type has no effect}}
+char* volatile h(); // expected-warning{{'volatile' type qualifier on return type has no effect}}
+
 const volatile int scalar_cv(); // expected-warning{{'const volatile' type qualifiers on return type have no effect}}
 }
Index: include/clang/Sema/DeclSpec.h
===================================================================
--- include/clang/Sema/DeclSpec.h	(revision 126218)
+++ include/clang/Sema/DeclSpec.h	(working copy)
@@ -825,6 +825,25 @@
   struct PointerTypeInfo : TypeInfoCommon {
     /// The type qualifiers: const/volatile/restrict.
     unsigned TypeQuals : 3;
+
+    /// The location of the const-qualifier, if any.
+    unsigned ConstQualLoc;
+
+    /// The location of the volatile-qualifier, if any.
+    unsigned VolatileQualLoc;
+
+    /// The location of the restrict-qualifier, if any.
+    unsigned RestrictQualLoc;
+
+    SourceLocation constQualLoc() const {
+      return SourceLocation::getFromRawEncoding(ConstQualLoc);
+    }
+    SourceLocation volatileQualLoc() const {
+      return SourceLocation::getFromRawEncoding(VolatileQualLoc);
+    }
+    SourceLocation restrictQualLoc() const {
+      return SourceLocation::getFromRawEncoding(RestrictQualLoc);
+    }
     void destroy() {
     }
   };
@@ -1055,12 +1074,18 @@
   /// getPointer - Return a DeclaratorChunk for a pointer.
   ///
   static DeclaratorChunk getPointer(unsigned TypeQuals, SourceLocation Loc,
+                                    SourceLocation ConstQualLoc,
+                                    SourceLocation VolatileQualLoc,
+                                    SourceLocation RestrictQualLoc,
                                     const ParsedAttributes &attrs) {
     DeclaratorChunk I;
-    I.Kind          = Pointer;
-    I.Loc           = Loc;
-    I.Ptr.TypeQuals = TypeQuals;
-    I.Ptr.AttrList  = attrs.getList();
+    I.Kind                = Pointer;
+    I.Loc                 = Loc;
+    I.Ptr.TypeQuals       = TypeQuals;
+    I.Ptr.ConstQualLoc    = ConstQualLoc.getRawEncoding();
+    I.Ptr.VolatileQualLoc = VolatileQualLoc.getRawEncoding();
+    I.Ptr.RestrictQualLoc = RestrictQualLoc.getRawEncoding();
+    I.Ptr.AttrList        = attrs.getList();
     return I;
   }
 
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp	(revision 126218)
+++ lib/Sema/SemaType.cpp	(working copy)
@@ -1395,6 +1395,49 @@
   return QT;
 }
 
+static void DiagnoseIgnoredQualifiers(unsigned Quals,
+                                      SourceLocation constQualLoc,
+                                      SourceLocation volatileQualLoc,
+                                      SourceLocation restrictQualLoc,
+                                      Sema& S) {
+  std::string QualStr;
+  unsigned NumQuals = 0;
+  SourceLocation Loc;
+
+  if (Quals & Qualifiers::Const) {
+    Loc = constQualLoc;
+    ++NumQuals;
+    QualStr = "const";
+  }
+  if (Quals & Qualifiers::Volatile) {
+    if (NumQuals == 0) {
+      Loc = volatileQualLoc;
+      QualStr = "volatile";
+    } else
+      QualStr += " volatile";
+    ++NumQuals;
+  }
+  if (Quals & Qualifiers::Restrict) {
+    if (NumQuals == 0) {
+      Loc = restrictQualLoc;
+      QualStr = "restrict";
+    } else
+      QualStr += " restrict";
+    ++NumQuals;
+  }
+
+  assert(NumQuals > 0 && "No known qualifiers?");
+  Sema::SemaDiagnosticBuilder DB = S.Diag(Loc, diag::warn_qual_return_type);
+
+  DB << QualStr << NumQuals;
+  if (Quals & Qualifiers::Const)
+    DB << FixItHint::CreateRemoval(constQualLoc);
+  if (Quals & Qualifiers::Volatile)
+    DB << FixItHint::CreateRemoval(volatileQualLoc);
+  if (Quals & Qualifiers::Restrict)
+    DB << FixItHint::CreateRemoval(restrictQualLoc);
+}
+
 /// GetTypeForDeclarator - Convert the type for the specified
 /// declarator to Type instances.
 ///
@@ -1678,46 +1721,28 @@
 
       // cv-qualifiers on return types are pointless except when the type is a
       // class type in C++.
-      if (T.getCVRQualifiers() && D.getDeclSpec().getTypeQualifiers() &&
+      if (T->isPointerType() && T.getCVRQualifiers() &&
+          (!getLangOptions().CPlusPlus || !T->isDependentType())) {
+        assert(chunkIndex + 1 < e && "No DeclaratorChunk for the return type?");
+        DeclaratorChunk ReturnTypeChunk = D.getTypeObject(chunkIndex + 1);
+        assert(ReturnTypeChunk.Kind == DeclaratorChunk::Pointer);
+
+        DiagnoseIgnoredQualifiers(ReturnTypeChunk.Ptr.TypeQuals,
+                                  ReturnTypeChunk.Ptr.constQualLoc(),
+                                  ReturnTypeChunk.Ptr.volatileQualLoc(),
+                                  ReturnTypeChunk.Ptr.restrictQualLoc(),
+                                  *this);
+      } else if (T.getCVRQualifiers() && D.getDeclSpec().getTypeQualifiers() &&
           (!getLangOptions().CPlusPlus ||
            (!T->isDependentType() && !T->isRecordType()))) {
-        unsigned Quals = D.getDeclSpec().getTypeQualifiers();
-        std::string QualStr;
-        unsigned NumQuals = 0;
-        SourceLocation Loc;
-        if (Quals & Qualifiers::Const) {
-          Loc = D.getDeclSpec().getConstSpecLoc();
-          ++NumQuals;
-          QualStr = "const";
-        }
-        if (Quals & Qualifiers::Volatile) {
-          if (NumQuals == 0) {
-            Loc = D.getDeclSpec().getVolatileSpecLoc();
-            QualStr = "volatile";
-          } else
-            QualStr += " volatile";
-          ++NumQuals;
-        }
-        if (Quals & Qualifiers::Restrict) {
-          if (NumQuals == 0) {
-            Loc = D.getDeclSpec().getRestrictSpecLoc();
-            QualStr = "restrict";
-          } else
-            QualStr += " restrict";
-          ++NumQuals;
-        }
-        assert(NumQuals > 0 && "No known qualifiers?");
-            
-        SemaDiagnosticBuilder DB = Diag(Loc, diag::warn_qual_return_type);
-        DB << QualStr << NumQuals;
-        if (Quals & Qualifiers::Const)
-          DB << FixItHint::CreateRemoval(D.getDeclSpec().getConstSpecLoc());
-        if (Quals & Qualifiers::Volatile)
-          DB << FixItHint::CreateRemoval(D.getDeclSpec().getVolatileSpecLoc());
-        if (Quals & Qualifiers::Restrict)
-          DB << FixItHint::CreateRemoval(D.getDeclSpec().getRestrictSpecLoc());
+
+        DiagnoseIgnoredQualifiers(D.getDeclSpec().getTypeQualifiers(),
+                                  D.getDeclSpec().getConstSpecLoc(),
+                                  D.getDeclSpec().getVolatileSpecLoc(),
+                                  D.getDeclSpec().getRestrictSpecLoc(),
+                                  *this);
       }
-      
+
       if (getLangOptions().CPlusPlus && D.getDeclSpec().isTypeSpecOwned()) {
         // C++ [dcl.fct]p6:
         //   Types shall not be defined in return or parameter types.
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp	(revision 126218)
+++ lib/Parse/ParseDecl.cpp	(working copy)
@@ -2746,6 +2746,9 @@
     if (Kind == tok::star)
       // Remember that we parsed a pointer type, and remember the type-quals.
       D.AddTypeInfo(DeclaratorChunk::getPointer(DS.getTypeQualifiers(), Loc,
+                                                DS.getConstSpecLoc(),
+                                                DS.getVolatileSpecLoc(),
+                                                DS.getRestrictSpecLoc(),
                                                 DS.takeAttributes()),
                     SourceLocation());
     else
@@ -3742,4 +3745,3 @@
   }
   return false;
 }
-  
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to