[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Downgrading the C89 warning for duplicate qualifiers from typedefs / __typeof from `ExtWarn` to `Extension` seems very reasonable to me. However, I don't see a justification for removing the warning for duplicate explicit (non-typedef) qualifiers in C99, nor for downgrading the corresponding `ExtWarn` to an `Extension` in our other language modes; that seems like a regression.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:197
+  InGroup<DuplicateDeclSpecifier>;
+def extwarn_duplicate_declspec : ExtWarn<"duplicate '%0' declaration specifier">,
   InGroup<DuplicateDeclSpecifier>;
----------------
Please follow our naming convention for diagnostic names: both `Extension` and `ExtWarn` diagnostics should start `ext_`.


================
Comment at: lib/Sema/DeclSpec.cpp:441-442
   else
-    DiagID = IsExtension ? diag::ext_duplicate_declspec :
-                           diag::warn_duplicate_declspec;
+    DiagID = IsExtension ? diag::ext_duplicate_declspec
+                         : diag::extwarn_duplicate_declspec;
   return true;
----------------
This doesn't look like a correct change. When `IsExtension` is false, the duplicate is not ill-formed, so we shouldn't use an `ExtWarn` diagnostic (which results in an error under `-pedantic-errors`).


================
Comment at: lib/Sema/DeclSpec.cpp:854-857
+  // Duplicates are permitted in C99 onwards, but are not permitted in C++.  In
+  // C90, they are a warning if -pedantic.  We do not need to set the
+  // qualifier's location since we already have it.
+  if (TypeQualifiers & T && !Lang.C99) {
----------------
We still want to warn on duplicate decl specifiers in C99 mode.


Repository:
  rC Clang

https://reviews.llvm.org/D52248