[cfe-dev] Storage class as written in the source.

[Douglas Gregor]

Hi Enea,

On Apr 16, 2010, at 12:27 PM, Enea Zaffanella wrote:
> Some time ago, Paolo Bolzoni sent to the mailing list a patch meant to enhance VarDecl and FunctionDecl nodes so that they can record the storage class "as written" in the source code (in contrast with the semantic storage class, which could be inherited).
> 
> This was the message:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-February/007982.html
> 
> The patch was not applied and now is out-of-date.
> 
> I have just refreshed it to be a patch against a recent version (r101446, see attached file). It passes all of the clang tests.
> 
> It would be nice if someone could have a look on and possibly commit it.

Looks very good. I have a few comments that I'd like to see addressed before I commit.

Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h	(revision 101446)
+++ include/clang/AST/DeclCXX.h	(working copy)
@@ -1186,8 +1187,9 @@
   CXXConstructorDecl(CXXRecordDecl *RD, SourceLocation L,
                      DeclarationName N, QualType T, TypeSourceInfo *TInfo,
                      bool isExplicitSpecified, bool isInline, 
-                     bool isImplicitlyDeclared)
-    : CXXMethodDecl(CXXConstructor, RD, L, N, T, TInfo, false, isInline),
+                     bool isImplicitlyDeclared, StorageClass SCAsWritten)
+    : CXXMethodDecl(CXXConstructor, RD, L, N, T, TInfo, false, SCAsWritten,
+                    isInline),
       IsExplicitSpecified(isExplicitSpecified), ImplicitlyDefined(false),
       BaseOrMemberInitializers(0), NumBaseOrMemberInitializers(0) {
     setImplicit(isImplicitlyDeclared);
@@ -1199,7 +1201,8 @@
                                     SourceLocation L, DeclarationName N,
                                     QualType T, TypeSourceInfo *TInfo,
                                     bool isExplicit,
-                                    bool isInline, bool isImplicitlyDeclared);
+                                    bool isInline, bool isImplicitlyDeclared,
+                                    StorageClass SCAsWritten);
 
   /// isExplicitSpecified - Whether this constructor declaration has the
   /// 'explicit' keyword specified.
@@ -1329,8 +1332,10 @@
   
   CXXDestructorDecl(CXXRecordDecl *RD, SourceLocation L,
                     DeclarationName N, QualType T,
-                    bool isInline, bool isImplicitlyDeclared)
-    : CXXMethodDecl(CXXDestructor, RD, L, N, T, /*TInfo=*/0, false, isInline),
+                    bool isInline, bool isImplicitlyDeclared,
+                    StorageClass SCAsWritten)
+    : CXXMethodDecl(CXXDestructor, RD, L, N, T, /*TInfo=*/0, false,
+                    SCAsWritten, isInline),
       ImplicitlyDefined(false), OperatorDelete(0) {
     setImplicit(isImplicitlyDeclared);
   }
@@ -1339,7 +1344,8 @@
   static CXXDestructorDecl *Create(ASTContext &C, CXXRecordDecl *RD,
                                    SourceLocation L, DeclarationName N,
                                    QualType T, bool isInline,
-                                   bool isImplicitlyDeclared);
+                                   bool isImplicitlyDeclared,
+                                   StorageClass SCAsWritten);
 
   /// isImplicitlyDefined - Whether this destructor was implicitly
   /// defined. If false, then this destructor was defined by the
@@ -1385,15 +1391,18 @@
 
   CXXConversionDecl(CXXRecordDecl *RD, SourceLocation L,
                     DeclarationName N, QualType T, TypeSourceInfo *TInfo,
-                    bool isInline, bool isExplicitSpecified)
-    : CXXMethodDecl(CXXConversion, RD, L, N, T, TInfo, false, isInline),
+                    bool isInline, bool isExplicitSpecified,
+                    StorageClass SCAsWritten)
+    : CXXMethodDecl(CXXConversion, RD, L, N, T, TInfo, false, SCAsWritten,
+                    isInline),
       IsExplicitSpecified(isExplicitSpecified) { }
 
 public:
   static CXXConversionDecl *Create(ASTContext &C, CXXRecordDecl *RD,
                                    SourceLocation L, DeclarationName N,
                                    QualType T, TypeSourceInfo *TInfo,
-                                   bool isInline, bool isExplicit);
+                                   bool isInline, bool isExplicit,
+                                   StorageClass SCAsWritten);
 
   /// IsExplicitSpecified - Whether this conversion function declaration is 
   /// marked "explicit", meaning that it can only be applied when the user

Constructors, destructors, and conversions don't have storage classes in well-formed code, so we shouldn't make the storage-class specifier part of the constructors for the corresponding AST nodes. Just pass FunctionDecl::None down to the CXXMethodDecl constructor in the appropriate place.

Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp	(revision 101446)
+++ lib/Sema/SemaDecl.cpp	(working copy)
@@ -1729,12 +1732,29 @@
       SC = VarDecl::None;
       break;
     }
+    VarDecl::StorageClass SCAsWritten;
+    switch (DS.getStorageClassSpecAsWritten()) {
+      default: /* Uncorrect program: we mark the sintatic
+                  storage class as none. */
+      case DeclSpec::SCS_unspecified:
+        SCAsWritten = VarDecl::None; break;
+      case DeclSpec::SCS_extern:
+        SCAsWritten = VarDecl::Extern; break;
+      case DeclSpec::SCS_static:
+        SCAsWritten = VarDecl::Static; break;
+      case DeclSpec::SCS_auto:
+        SCAsWritten = VarDecl::Auto; break;
+      case DeclSpec::SCS_register:
+        SCAsWritten = VarDecl::Register; break;
+      case DeclSpec::SCS_private_extern:
+        SCAsWritten = VarDecl::PrivateExtern; break;
+    }

We don't need the default case here. It's better to write out all of the cases.

@@ -2350,6 +2370,23 @@
     SC = VarDecl::None;
     break;
   }
+  VarDecl::StorageClass SCAsWritten;
+  switch (D.getDeclSpec().getStorageClassSpecAsWritten()) {
+    default: /* Uncorrect program: we mark the sintatic
+                storage class as none. */
+    case DeclSpec::SCS_unspecified:
+      SCAsWritten = VarDecl::None; break;
+    case DeclSpec::SCS_extern:
+      SCAsWritten = VarDecl::Extern; break;
+    case DeclSpec::SCS_static:
+      SCAsWritten = VarDecl::Static; break;
+    case DeclSpec::SCS_auto:
+      SCAsWritten = VarDecl::Auto; break;
+    case DeclSpec::SCS_register:
+      SCAsWritten = VarDecl::Register; break;
+    case DeclSpec::SCS_private_extern:
+      SCAsWritten = VarDecl::PrivateExtern; break;
+  }

Copy-paste from above? Please factor this into its own utility routine.

@@ -2801,7 +2838,21 @@
   bool isInline = D.getDeclSpec().isInlineSpecified();
   bool isVirtual = D.getDeclSpec().isVirtualSpecified();
   bool isExplicit = D.getDeclSpec().isExplicitSpecified();
+  FunctionDecl::StorageClass SCAsWritten;
+  switch (D.getDeclSpec().getStorageClassSpecAsWritten()) {
+    default: /* Uncorrect program: we mark the sintatic
+                storage class as none. */
+    case DeclSpec::SCS_unspecified:
+      SCAsWritten = FunctionDecl::None; break;
+    case DeclSpec::SCS_extern:
+      SCAsWritten = FunctionDecl::Extern; break;
+    case DeclSpec::SCS_static:
+      SCAsWritten = FunctionDecl::Static; break;
+    case DeclSpec::SCS_private_extern:
+      SCAsWritten = FunctionDecl::PrivateExtern; break;
+  }

We don't want the default case here, either.

	- Doug