[cfe-commits] r62192 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/Sema/SemaDecl.cpp

URL: http://llvm.org/viewvc/llvm-project?rev=62192&view=rev
Log:
Permitting typedefs without a name is a Microsoft/GNU extension

Hey Doug,

+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Tue Jan 13 17:10:51 2009
@@ -593,6 +593,8 @@
     "expected unqualified-id")
DIAG(err_no_declarators, ERROR,
     "declaration does not declare anything")
+DIAG(ext_no_declarators, EXTENSION,
+ "typedef without a name is a Microsoft extension")

Should this be an EXTWARN so that it warns by default?

+ // Permit typedefs without declarators as a Microsoft extension.
  if (!DS.isMissingDeclaratorOk()) {
+ if (getLangOptions().Microsoft &&
+ DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {
+ Diag(DS.getSourceRange().getBegin(), diag::ext_no_declarators)
+ << DS.getSourceRange();
+ return Tag;
+ }

This touches on meta-design issues, but do you think it is better to test for getLangOptions.MS here, or do you think it is better to have the Clang driver map this onto ERROR by default when not is ms extensions mode? Both approaches work, but they have different tradeoffs. I'm curious what you (and others!) think.

-Chris

Hi Chris,

URL: http://llvm.org/viewvc/llvm-project?rev=62192&view=rev
Log:
Permitting typedefs without a name is a Microsoft/GNU extension

Hey Doug,

Hi Chris!

+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Tue Jan 13 17:10:51 2009
@@ -593,6 +593,8 @@
    "expected unqualified-id")
DIAG(err_no_declarators, ERROR,
    "declaration does not declare anything")
+DIAG(ext_no_declarators, EXTENSION,
+ "typedef without a name is a Microsoft extension")

Should this be an EXTWARN so that it warns by default?

Yes. I'd changed it to WARNING when we found that GNU also had the same extensions (used in system headers nonetheless), but EXTWARN is the right thing. I'll fix it.

+ // Permit typedefs without declarators as a Microsoft extension.
if (!DS.isMissingDeclaratorOk()) {
+ if (getLangOptions().Microsoft &&
+ DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {
+ Diag(DS.getSourceRange().getBegin(), diag::ext_no_declarators)
+ << DS.getSourceRange();
+ return Tag;
+ }

This touches on meta-design issues, but do you think it is better to test for getLangOptions.MS here, or do you think it is better to have the Clang driver map this onto ERROR by default when not is ms extensions mode? Both approaches work, but they have different tradeoffs. I'm curious what you (and others!) think.

It's an apt meta-issue, but I think in this case, since it's also a GNU extension, it should be an EXTWARN regardless of dialect. On the meta-issue itself, I think I like having the Clang driver make decisions between WARNING/EXTWARN/EXTENSION, but to me it seems like switching a diagnostic from or to ERROR is just asking for trouble: even if it works when we initially do it, we're likely to evolve Sema or the Parser to assume that an error really is an error, and screw up processing when that error turns out not to be an error.

  - Doug

+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Tue Jan 13 17:10:51 2009
@@ -593,6 +593,8 @@
   "expected unqualified-id")
DIAG(err_no_declarators, ERROR,
   "declaration does not declare anything")
+DIAG(ext_no_declarators, EXTENSION,
+ "typedef without a name is a Microsoft extension")

Should this be an EXTWARN so that it warns by default?

Yes. I'd changed it to WARNING when we found that GNU also had the same extensions (used in system headers nonetheless), but EXTWARN is the right thing. I'll fix it.

Thanks, ok.

+ // Permit typedefs without declarators as a Microsoft extension.
if (!DS.isMissingDeclaratorOk()) {
+ if (getLangOptions().Microsoft &&
+ DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {
+ Diag(DS.getSourceRange().getBegin(), diag::ext_no_declarators)
+ << DS.getSourceRange();
+ return Tag;
+ }

This touches on meta-design issues, but do you think it is better to test for getLangOptions.MS here, or do you think it is better to have the Clang driver map this onto ERROR by default when not is ms extensions mode? Both approaches work, but they have different tradeoffs. I'm curious what you (and others!) think.

It's an apt meta-issue, but I think in this case, since it's also a GNU extension, it should be an EXTWARN regardless of dialect. On the meta-issue itself, I think I like having the Clang driver make decisions between WARNING/EXTWARN/EXTENSION, but to me it seems like switching a diagnostic from or to ERROR is just asking for trouble: even if it works when we initially do it, we're likely to evolve Sema or the Parser to assume that an error really is an error, and screw up processing when that error turns out not to be an error.

It doesn't matter in this case anymore, however, the point that I was trying to make was that LangOptions and fine-grain warning control are very similar in some cases. In this case, the compiler (including down-stream clients) clearly has to be able to handle this feature. Using a langoption to control a warning just seems strange...

-Chris

Yeah, I agree with that. When a diagnostic doesn’t actually effect what the compiler is going to do, we shouldn’t be checking LangOptions to decide whether to emit the warning; the diagnostics system can make that decision.

  • Doug