-Wformat-non-iso: warn about positional arguments (pr12017)

This renames the -Wformat-non-standard flag to -Wformat-non-iso,
rewords the current warnings a bit (pointing out that a format string
is not supported by ISO C rather than being "non standard"),
and adds a warning about positional arguments.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@152403 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h
index 9dc8f8878d5366ed98efd31f4a592bc91bfb5d04..9ec27ce91dba28817f7ffe5024c459860370a018 100644 (file)
--- a/include/clang/Analysis/Analyses/FormatString.h
+++ b/include/clang/Analysis/Analyses/FormatString.h
@@ -306,9 +306,9 @@ protected:
   LengthModifier LM;
   OptionalAmount FieldWidth;
   ConversionSpecifier CS;
-    /// Positional arguments, an IEEE extension:
-    ///  IEEE Std 1003.1, 2004 Edition
-    ///  http://www.opengroup.org/onlinepubs/009695399/functions/printf.html
+  /// Positional arguments, an IEEE extension:
+  ///  IEEE Std 1003.1, 2004 Edition
+  ///  http://www.opengroup.org/onlinepubs/009695399/functions/printf.html
   bool UsesPositionalArg;
   unsigned argIndex;
 public:
@@ -598,6 +598,8 @@ public:
 
   virtual void HandleNullChar(const char *nullCharacter) {}
 
+  virtual void HandlePosition(const char *startPos, unsigned posLen) {}
+
   virtual void HandleInvalidPosition(const char *startPos, unsigned posLen,
                                      PositionContext p) {}
 

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index f5de122e0e31fb5bf8ca7221a4073bc42784bcfb..26dcc40abaea33dd86c4637d63ca78788e26a2ab 100644 (file)
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -302,7 +302,7 @@ def Unused : DiagGroup<"unused",
 // Format settings.
 def FormatInvalidSpecifier : DiagGroup<"format-invalid-specifier">;
 def FormatSecurity : DiagGroup<"format-security">;
-def FormatNonStandard : DiagGroup<"format-non-standard">;
+def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def Format : DiagGroup<"format",
                        [FormatExtraArgs, FormatZeroLength, NonNull,

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index fadcdf9d360ec0744a5baf5cd0ec1b54b0502363..034741ed88978bf85f2c728be4bd1ec4f6db5376 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4944,10 +4944,13 @@ def warn_printf_nonsensical_flag: Warning<
 def warn_format_nonsensical_length: Warning<
   "length modifier '%0' results in undefined behavior or no effect with '%1' conversion specifier">,
   InGroup<Format>;
+def warn_format_non_standard_positional_arg: ExtWarn<
+  "positional arguments are not supported by ISO C">, InGroup<FormatNonStandard>, DefaultIgnore;
 def warn_format_non_standard: ExtWarn<
-  "'%0' is a non-standard %1">, InGroup<FormatNonStandard>, DefaultIgnore;
+  "'%0' %select{length modifier|conversion specifier}1 is not supported by ISO C">,
+  InGroup<FormatNonStandard>, DefaultIgnore;
 def warn_format_non_standard_conversion_spec: ExtWarn<
-  "using the length modifier '%0' with the conversion specifier '%1' is non-standard">,
+  "using length modifier '%0' with conversion specifier '%1' is not supported by ISO C">,
   InGroup<FormatNonStandard>, DefaultIgnore;
 def warn_printf_ignored_flag: Warning<
   "flag '%0' is ignored when flag '%1' is present">,

diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp
index 5785655c946f269b6e8802032eada307b7498815..dd2f2406d464ad489e7b659559dc5517bb7a4af5 100644 (file)
--- a/lib/Analysis/FormatString.cpp
+++ b/lib/Analysis/FormatString.cpp
@@ -156,6 +156,9 @@ clang::analyze_format_string::ParseArgPosition(FormatStringHandler &H,
   }
 
   if (Amt.getHowSpecified() == OptionalAmount::Constant && *(I++) == '$') {
+    // Warn that positional arguments are non-standard.
+    H.HandlePosition(Start, I - Start);
+
     // Special case: '%0$', since this is an easy mistake.
     if (Amt.getConstantAmount() == 0) {
       H.HandleZeroPosition(Start, I - Start);

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 3d9f5b3afd2c8c89df75671c9b14eec3517c65f9..ad763c346d415f6fa14e76b6df190bb0b5bba268 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1677,6 +1677,8 @@ public:
       const analyze_format_string::ConversionSpecifier &CS,
       const char *startSpecifier, unsigned specifierLen);
 
+  virtual void HandlePosition(const char *startPos, unsigned posLen);
+
   virtual void HandleInvalidPosition(const char *startSpecifier,
                                      unsigned specifierLen,
                                      analyze_format_string::PositionContext p);
@@ -1756,7 +1758,7 @@ void CheckFormatHandler::HandleNonStandardLengthModifier(
     const analyze_format_string::LengthModifier &LM,
     const char *startSpecifier, unsigned specifierLen) {
   EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << LM.toString()
-                       << "length modifier",
+                       << 0,
                        getLocationOfByte(LM.getStart()),
                        /*IsStringLocation*/true,
                        getSpecifierRange(startSpecifier, specifierLen));
@@ -1766,7 +1768,7 @@ void CheckFormatHandler::HandleNonStandardConversionSpecifier(
     const analyze_format_string::ConversionSpecifier &CS,
     const char *startSpecifier, unsigned specifierLen) {
   EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << CS.toString()
-                       << "conversion specifier",
+                       << 1,
                        getLocationOfByte(CS.getStart()),
                        /*IsStringLocation*/true,
                        getSpecifierRange(startSpecifier, specifierLen));
@@ -1783,6 +1785,14 @@ void CheckFormatHandler::HandleNonStandardConversionSpecification(
                        getSpecifierRange(startSpecifier, specifierLen));
 }
 
+void CheckFormatHandler::HandlePosition(const char *startPos,
+                                        unsigned posLen) {
+  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_positional_arg),
+                               getLocationOfByte(startPos),
+                               /*IsStringLocation*/true,
+                               getSpecifierRange(startPos, posLen));
+}
+
 void
 CheckFormatHandler::HandleInvalidPosition(const char *startPos, unsigned posLen,
                                      analyze_format_string::PositionContext p) {

diff --git a/test/Sema/format-strings-c90.c b/test/Sema/format-strings-c90.c
index 63d58532cb80a4c7dd603e7c5b9b720d5fde7e56..66ca5073475ce9d2899eb0b5974a95742feaa528 100644 (file)
--- a/test/Sema/format-strings-c90.c
+++ b/test/Sema/format-strings-c90.c
@@ -5,8 +5,8 @@ int scanf(const char * restrict, ...);
 int printf(const char *restrict, ...);
 
 void foo(char **sp, float *fp, int *ip) {
-  scanf("%as", sp); /* expected-warning{{'a' is a non-standard length modifier}} */
-  scanf("%a[abc]", sp); /* expected-warning{{'a' is a non-standard length modifier}} */
+  scanf("%as", sp); /* expected-warning{{'a' length modifier is not supported by ISO C}} */
+  scanf("%a[abc]", sp); /* expected-warning{{'a' length modifier is not supported by ISO C}} */
 
   /* TODO: Warn that the 'a' conversion specifier is a C99 feature. */
   scanf("%a", fp);
@@ -21,10 +21,10 @@ void foo(char **sp, float *fp, int *ip) {
 
   /* Test argument type check for the 'a' length modifier. */
   scanf("%as", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}}
-                       expected-warning{{'a' is a non-standard length modifier}} */
+                       expected-warning{{'a' length modifier is not supported by ISO C}} */
   scanf("%aS", fp); /* expected-warning{{format specifies type 'wchar_t **' (aka 'int **') but the argument has type 'float *'}}
-                       expected-warning{{'a' is a non-standard length modifier}}
-                       expected-warning{{'S' is a non-standard conversion specifier}} */
+                       expected-warning{{'a' length modifier is not supported by ISO C}}
+                       expected-warning{{'S' conversion specifier is not supported by ISO C}} */
   scanf("%a[abc]", fp); /* expected-warning{{format specifies type 'char **' but the argument has type 'float *'}}
-                           expected-warning{{'a' is a non-standard length modifier}} */
+                           expected-warning{{'a' length modifier is not supported by ISO C}} */
 }

diff --git a/test/Sema/format-strings-fixit.c b/test/Sema/format-strings-fixit.c
index 80b1be05787bcd4b6b03df2048a8c5da536a8353..800691ecc8cd374e7fc956dcff5f379c0ee49414 100644 (file)
--- a/test/Sema/format-strings-fixit.c
+++ b/test/Sema/format-strings-fixit.c
@@ -35,7 +35,10 @@ void test() {
   printf("%0-f", 1.23); // - flag should stay
 
   // Positional arguments
+#pragma clang diagnostic push // Don't warn about using positional arguments.
+#pragma clang diagnostic ignored "-Wformat-non-iso"
   printf("%1$f:%2$.*3$f:%4$.*3$f\n", 1, 2, 3, 4);
+#pragma clang diagnostic pop
 
   // Precision
   printf("%10.5d", 1l); // (bug 7394)
@@ -46,7 +49,10 @@ void test() {
 
   // Bad length modifiers
   printf("%hhs", "foo");
+#pragma clang diagnostic push // Don't warn about using positional arguments.
+#pragma clang diagnostic ignored "-Wformat-non-iso"
   printf("%1$zp", (void *)0);
+#pragma clang diagnostic pop
 
   // Preserve the original formatting for unsigned integers.
   unsigned long val = 42;

diff --git a/test/Sema/format-strings-non-iso.c b/test/Sema/format-strings-non-iso.c
new file mode 100644 (file)
index 0000000..ed8095f
--- /dev/null
+++ b/test/Sema/format-strings-non-iso.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -pedantic %s
+
+int printf(const char *restrict, ...);
+int scanf(const char * restrict, ...);
+
+void f(void) {
+  char *cp;
+
+  // The 'q' length modifier.
+  printf("%qd", (long long)42); // expected-warning{{'q' length modifier is not supported by ISO C}}
+  scanf("%qd", (long long *)0); // expected-warning{{'q' length modifier is not supported by ISO C}}
+
+  // The 'm' length modifier.
+  scanf("%ms", &cp); // expected-warning{{'m' length modifier is not supported by ISO C}}
+
+  // The 'S' and 'C' conversion specifiers.
+  printf("%S", L"foo"); // expected-warning{{'S' conversion specifier is not supported by ISO C}}
+  printf("%C", L'x'); // expected-warning{{'C' conversion specifier is not supported by ISO C}}
+
+  // Combining 'L' with an integer conversion specifier.
+  printf("%Li", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'i' is not supported by ISO C}}
+  printf("%Lo", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'o' is not supported by ISO C}}
+  printf("%Lu", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'u' is not supported by ISO C}}
+  printf("%Lx", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'x' is not supported by ISO C}}
+  printf("%LX", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'X' is not supported by ISO C}}
+
+  // Positional arguments.
+  printf("%1$d", 42); // expected-warning{{positional arguments are not supported by ISO C}}
+}

diff --git a/test/Sema/format-strings-non-standard.c b/test/Sema/format-strings-non-standard.c
deleted file mode 100644 (file)
index a24d43a..0000000
--- a/test/Sema/format-strings-non-standard.c
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -pedantic %s
-
-int printf(const char *restrict, ...);
-int scanf(const char * restrict, ...);
-
-void f(void) {
-  char *cp;
-
-  // The 'q' length modifier.
-  printf("%qd", (long long)42); // expected-warning{{'q' is a non-standard length modifier}}
-  scanf("%qd", (long long *)0); // expected-warning{{'q' is a non-standard length modifier}}
-
-  // The 'm' length modifier.
-  scanf("%ms", &cp); // expected-warning{{'m' is a non-standard length modifier}}
-
-  // The 'S' and 'C' conversion specifiers.
-  printf("%S", L"foo"); // expected-warning{{'S' is a non-standard conversion specifier}}
-  printf("%C", L'x'); // expected-warning{{'C' is a non-standard conversion specifier}}
-
-  // Combining 'L' with an integer conversion specifier.
-  printf("%Li", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'i' is non-standard}}
-  printf("%Lo", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'o' is non-standard}}
-  printf("%Lu", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'u' is non-standard}}
-  printf("%Lx", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'x' is non-standard}}
-  printf("%LX", (long long)42); // expected-warning{{using the length modifier 'L' with the conversion specifier 'X' is non-standard}}
-}

diff --git a/test/SemaCXX/format-strings.cpp b/test/SemaCXX/format-strings.cpp
index 314617d4914a7e9b11e7a64910e612c42dad332a..6b0df2935378fb85c9cec61bff0b10fc751436f9 100644 (file)
--- a/test/SemaCXX/format-strings.cpp
+++ b/test/SemaCXX/format-strings.cpp
@@ -9,7 +9,7 @@ extern int vprintf(const char *restrict, va_list);
 }
 
 void f(char **sp, float *fp) {
-  scanf("%as", sp); // expected-warning{{'a' is a non-standard length modifier}}
+  scanf("%as", sp); // expected-warning{{'a' length modifier is not supported by ISO C}}
 
   // TODO: Warn that the 'a' conversion specifier is a C++11 feature.
   printf("%a", 1.0);