[llvm-branch-commits] [cfe-branch] r119421 - in /cfe/branches/Apple/whitney: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclAttr.cpp test/SemaCXX/attr-format.cpp test/SemaCXX/attr-nonnull.cpp test/SemaCXX/format-attribute.cpp

Daniel Dunbar daniel at zuster.org
Tue Nov 16 16:23:49 PST 2010


Author: ddunbar
Date: Tue Nov 16 18:23:49 2010
New Revision: 119421

URL: http://llvm.org/viewvc/llvm-project?rev=119421&view=rev
Log:
Merge r119339:
--
Author: Chandler Carruth <chandlerc at gmail.com>
Date:   Tue Nov 16 08:35:43 2010 +0000

    Re-work the handling of implicit 'this' arguments and silly GCC-style attribute
    argument indexes. This handles the offsets in a consistent manner for all of
    the attributes which I saw working with these concepts. I've also added tests
    for the attribute that motivated this: nonnull.

    I consolidated the tests for format attributes into one file, and fleshed them
    out a bit to trigger more of the warning cases. Also improved the quality of
    some of the diagnostics that occur with invalid argument indices.

    The only really questionable change here is supporting the implicit this
    argument for the ownership attribute. I'm not sure it's really a sensible
    concept there, but implemented the logic for consistency.

Added:
    cfe/branches/Apple/whitney/test/SemaCXX/attr-nonnull.cpp
Removed:
    cfe/branches/Apple/whitney/test/SemaCXX/format-attribute.cpp
Modified:
    cfe/branches/Apple/whitney/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/branches/Apple/whitney/lib/Sema/SemaDeclAttr.cpp
    cfe/branches/Apple/whitney/test/SemaCXX/attr-format.cpp

Modified: cfe/branches/Apple/whitney/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/whitney/include/clang/Basic/DiagnosticSemaKinds.td?rev=119421&r1=119420&r2=119421&view=diff
==============================================================================
--- cfe/branches/Apple/whitney/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/branches/Apple/whitney/include/clang/Basic/DiagnosticSemaKinds.td Tue Nov 16 18:23:49 2010
@@ -911,6 +911,8 @@
   "attribute may only be applied to an Objective-C interface">;
 def warn_nonnull_pointers_only : Warning<
   "nonnull attribute only applies to pointer arguments">;
+def err_attribute_invalid_implicit_this_argument : Error<
+  "'%0' attribute is invalid for the implicit this argument">;
 def err_ownership_type : Error<
   "%0 attribute only applies to %1 arguments">;
 def err_format_strftime_third_parameter : Error<
@@ -919,6 +921,9 @@
   "format attribute requires variadic function">;
 def err_format_attribute_not : Error<"format argument not %0">;
 def err_format_attribute_result_not : Error<"function does not return %0">;
+def err_format_attribute_implicit_this_format_string : Error<
+  "format attribute cannot specify the implicit this argument as the format "
+  "string">;
 def err_attribute_invalid_size : Error<
   "vector size not an integral multiple of component size">;
 def err_attribute_zero_size : Error<"zero vector size">;

Modified: cfe/branches/Apple/whitney/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/whitney/lib/Sema/SemaDeclAttr.cpp?rev=119421&r1=119420&r2=119421&view=diff
==============================================================================
--- cfe/branches/Apple/whitney/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/branches/Apple/whitney/lib/Sema/SemaDeclAttr.cpp Tue Nov 16 18:23:49 2010
@@ -127,6 +127,12 @@
   }
 }
 
+static bool isInstanceMethod(const Decl *d) {
+  if (const CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(d))
+    return MethodDecl->isInstance();
+  return false;
+}
+
 static inline bool isNSStringType(QualType T, ASTContext &Ctx) {
   const ObjCObjectPointerType *PT = T->getAs<ObjCObjectPointerType>();
   if (!PT)
@@ -323,7 +329,10 @@
     return;
   }
 
-  unsigned NumArgs = getFunctionOrMethodNumArgs(d);
+  // In C++ the implicit 'this' function parameter also counts, and they are
+  // counted from one.
+  bool HasImplicitThisParam = isInstanceMethod(d);
+  unsigned NumArgs  = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam;
 
   // The nonnull attribute only applies to pointers.
   llvm::SmallVector<unsigned, 10> NonNullArgs;
@@ -351,6 +360,15 @@
     }
 
     --x;
+    if (HasImplicitThisParam) {
+      if (x == 0) {
+        S.Diag(Attr.getLoc(),
+               diag::err_attribute_invalid_implicit_this_argument)
+          << "nonnull" << Ex->getSourceRange();
+        return;
+      }
+      --x;
+    }
 
     // Is the function argument a pointer type?
     QualType T = getFunctionOrMethodArgType(d, x);
@@ -454,7 +472,10 @@
     return;
   }
 
-  unsigned NumArgs = getFunctionOrMethodNumArgs(d);
+  // In C++ the implicit 'this' function parameter also counts, and they are
+  // counted from one.
+  bool HasImplicitThisParam = isInstanceMethod(d);
+  unsigned NumArgs  = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam;
 
   llvm::StringRef Module = AL.getParameterName()->getName();
 
@@ -484,6 +505,15 @@
       continue;
     }
     --x;
+    if (HasImplicitThisParam) {
+      if (x == 0) {
+        S.Diag(AL.getLoc(), diag::err_attribute_invalid_implicit_this_argument)
+          << "ownership" << IdxExpr->getSourceRange();
+        return;
+      }
+      --x;
+    }
+
     switch (K) {
     case OwnershipAttr::Takes:
     case OwnershipAttr::Holds: {
@@ -1397,10 +1427,13 @@
     << Attr.getName() << 0 /*function*/;
     return;
   }
-  // FIXME: in C++ the implicit 'this' function parameter also counts.  this is
-  // needed in order to be compatible with GCC the index must start with 1.
-  unsigned NumArgs  = getFunctionOrMethodNumArgs(d);
+
+  // In C++ the implicit 'this' function parameter also counts, and they are
+  // counted from one.
+  bool HasImplicitThisParam = isInstanceMethod(d);
+  unsigned NumArgs  = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam;
   unsigned FirstIdx = 1;
+
   // checks for the 2nd argument
   Expr *IdxExpr = static_cast<Expr *>(Attr.getArg(0));
   llvm::APSInt Idx(32);
@@ -1419,6 +1452,15 @@
 
   unsigned ArgIdx = Idx.getZExtValue() - 1;
 
+  if (HasImplicitThisParam) {
+    if (ArgIdx == 0) {
+      S.Diag(Attr.getLoc(), diag::err_attribute_invalid_implicit_this_argument)
+        << "format_arg" << IdxExpr->getSourceRange();
+      return;
+    }
+    ArgIdx--;
+  }
+
   // make sure the format string is really a string
   QualType Ty = getFunctionOrMethodArgType(d, ArgIdx);
 
@@ -1445,7 +1487,8 @@
     return;
   }
 
-  d->addAttr(::new (S.Context) FormatArgAttr(Attr.getLoc(), S.Context, Idx.getZExtValue()));
+  d->addAttr(::new (S.Context) FormatArgAttr(Attr.getLoc(), S.Context,
+                                             Idx.getZExtValue()));
 }
 
 enum FormatAttrKind {
@@ -1551,7 +1594,10 @@
     return;
   }
 
-  unsigned NumArgs  = getFunctionOrMethodNumArgs(d);
+  // In C++ the implicit 'this' function parameter also counts, and they are
+  // counted from one.
+  bool HasImplicitThisParam = isInstanceMethod(d);
+  unsigned NumArgs  = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam;
   unsigned FirstIdx = 1;
 
   llvm::StringRef Format = Attr.getParameterName()->getName();
@@ -1582,16 +1628,6 @@
     return;
   }
 
-  // FIXME: We should handle the implicit 'this' parameter in a more generic
-  // way that can be used for other arguments.
-  bool HasImplicitThisParam = false;
-  if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(d)) {
-    if (MD->isInstance()) {
-      HasImplicitThisParam = true;
-      NumArgs++;
-    }
-  }
-
   if (Idx.getZExtValue() < FirstIdx || Idx.getZExtValue() > NumArgs) {
     S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)
       << "format" << 2 << IdxExpr->getSourceRange();
@@ -1603,8 +1639,9 @@
 
   if (HasImplicitThisParam) {
     if (ArgIdx == 0) {
-      S.Diag(Attr.getLoc(), diag::err_format_attribute_not)
-        << "a string type" << IdxExpr->getSourceRange();
+      S.Diag(Attr.getLoc(),
+             diag::err_format_attribute_implicit_this_format_string)
+        << IdxExpr->getSourceRange();
       return;
     }
     ArgIdx--;

Modified: cfe/branches/Apple/whitney/test/SemaCXX/attr-format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/whitney/test/SemaCXX/attr-format.cpp?rev=119421&r1=119420&r2=119421&view=diff
==============================================================================
--- cfe/branches/Apple/whitney/test/SemaCXX/attr-format.cpp (original)
+++ cfe/branches/Apple/whitney/test/SemaCXX/attr-format.cpp Tue Nov 16 18:23:49 2010
@@ -1,8 +1,23 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 struct S {
   static void f(const char*, ...) __attribute__((format(printf, 1, 2)));
+  static const char* f2(const char*) __attribute__((format_arg(1)));
 
   // GCC has a hidden 'this' argument in member functions which is why
   // the format argument is argument 2 here.
   void g(const char*, ...) __attribute__((format(printf, 2, 3)));
+  const char* g2(const char*) __attribute__((format_arg(2)));
+
+  void h(const char*, ...) __attribute__((format(printf, 1, 4))); // \
+      expected-error{{implicit this argument as the format string}}
+  void h2(const char*, ...) __attribute__((format(printf, 2, 1))); // \
+      expected-error{{out of bounds}}
+  const char* h3(const char*) __attribute__((format_arg(1))); // \
+      expected-error{{invalid for the implicit this argument}}
 };
+
+// PR5521
+struct A { void a(const char*,...) __attribute((format(printf,2,3))); };
+void b(A x) {
+  x.a("%d", 3);
+}

Added: cfe/branches/Apple/whitney/test/SemaCXX/attr-nonnull.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/whitney/test/SemaCXX/attr-nonnull.cpp?rev=119421&view=auto
==============================================================================
--- cfe/branches/Apple/whitney/test/SemaCXX/attr-nonnull.cpp (added)
+++ cfe/branches/Apple/whitney/test/SemaCXX/attr-nonnull.cpp Tue Nov 16 18:23:49 2010
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+struct S {
+  static void f(const char*, const char*) __attribute__((nonnull(1)));
+
+  // GCC has a hidden 'this' argument in member functions, so the middle
+  // argument is the one that must not be null.
+  void g(const char*, const char*, const char*) __attribute__((nonnull(3)));
+
+  void h(const char*) __attribute__((nonnull(1))); // \
+      expected-error{{invalid for the implicit this argument}}
+};
+
+void test(S s) {
+  s.f(0, ""); // expected-warning{{null passed}}
+  s.f("", 0);
+  s.g("", 0, ""); // expected-warning{{null passed}}
+  s.g(0, "", 0);
+}

Removed: cfe/branches/Apple/whitney/test/SemaCXX/format-attribute.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/whitney/test/SemaCXX/format-attribute.cpp?rev=119420&view=auto
==============================================================================
--- cfe/branches/Apple/whitney/test/SemaCXX/format-attribute.cpp (original)
+++ cfe/branches/Apple/whitney/test/SemaCXX/format-attribute.cpp (removed)
@@ -1,8 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s 
-
-// PR5521
-struct A { void a(const char*,...) __attribute((format(printf,2,3))); };
-void b(A x) {
-  x.a("%d", 3);
-}
-struct X { void a(const char*,...) __attribute((format(printf,1,3))); }; // expected-error {{format argument not a string type}}





More information about the llvm-branch-commits mailing list