r200438 - Diagnose typedef names for linkage purposes that would change

[John McCall]

Author: rjmccall
Date: Wed Jan 29 19:12:53 2014
New Revision: 200438

URL: http://llvm.org/viewvc/llvm-project?rev=200438&view=rev
Log:
Diagnose typedef names for linkage purposes that would change
a previously-computed linkage as an unsupportable error condition.

Per discussion on cfe-commits, this appears to be a
difficult-to-resolve flaw in our implementation approach;
we may pursue this as a language defect, but for now it's
better to diagnose it as unsupported than to produce
inconsistent results (or assertions).  Anything that we can
do to limit how often this diagnostic fires, such as the
changes in r200380, is probably for the best, though.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/test/SemaCXX/anonymous-struct.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=200438&r1=200437&r2=200438&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Jan 29 19:12:53 2014
@@ -258,6 +258,16 @@ public:
   /// checking. Should always return true.
   bool isLinkageValid() const;
 
+  /// \brief True if something has required us to compute the linkage
+  /// of this declaration.
+  ///
+  /// Language features which can retroactively change linkage (like a
+  /// typedef name for linkage purposes) may need to consider this,
+  /// but hopefully only in transitory ways during parsing.
+  bool hasLinkageBeenComputed() const {
+    return hasCachedLinkage();
+  }
+
   /// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
   /// the underlying named decl.
   NamedDecl *getUnderlyingDecl() {

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=200438&r1=200437&r2=200438&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jan 29 19:12:53 2014
@@ -447,6 +447,10 @@ def ext_no_declarators : ExtWarn<"declar
 def ext_typedef_without_a_name : ExtWarn<"typedef requires a name">,
   InGroup<MissingDeclarations>;
 def err_typedef_not_identifier : Error<"typedef name must be an identifier">;
+def err_typedef_changes_linkage : Error<"unsupported: typedef changes linkage"
+  " of anonymous type, but linkage was already computed">;
+def note_typedef_changes_linkage : Note<"use a tag name here to establish "
+  "linkage prior to definition">;
 def err_statically_allocated_object : Error<
   "interface type cannot be statically allocated">;
 def err_object_cannot_be_passed_returned_by_value : Error<

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=200438&r1=200437&r2=200438&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jan 29 19:12:53 2014
@@ -10144,6 +10144,27 @@ TypedefDecl *Sema::ParseTypedefDecl(Scop
     if (!Context.hasSameType(T, Context.getTagDeclType(tagFromDeclSpec)))
       break;
 
+    // If we've already computed linkage for the anonymous tag, then
+    // adding a typedef name for the anonymous decl can change that
+    // linkage, which might be a serious problem.  Diagnose this as
+    // unsupported and ignore the typedef name.  TODO: we should
+    // pursue this as a language defect and establish a formal rule
+    // for how to handle it.
+    if (tagFromDeclSpec->hasLinkageBeenComputed()) {
+      Diag(D.getIdentifierLoc(), diag::err_typedef_changes_linkage);
+
+      SourceLocation tagLoc = D.getDeclSpec().getTypeSpecTypeLoc();
+      tagLoc = Lexer::getLocForEndOfToken(tagLoc, 0, getSourceManager(),
+                                          getLangOpts());
+
+      llvm::SmallString<40> textToInsert;
+      textToInsert += ' ';
+      textToInsert += D.getIdentifier()->getName();
+      Diag(tagLoc, diag::note_typedef_changes_linkage)
+        << FixItHint::CreateInsertion(tagLoc, textToInsert);
+      break;
+    }
+
     // Otherwise, set this is the anon-decl typedef for the tag.
     tagFromDeclSpec->setTypedefNameForAnonDecl(NewTD);
     break;

Modified: cfe/trunk/test/SemaCXX/anonymous-struct.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/anonymous-struct.cpp?rev=200438&r1=200437&r2=200438&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/anonymous-struct.cpp (original)
+++ cfe/trunk/test/SemaCXX/anonymous-struct.cpp Wed Jan 29 19:12:53 2014
@@ -14,3 +14,10 @@ struct E {
   static struct {
   };
 };
+
+template <class T> void foo(T);
+typedef struct { // expected-note {{use a tag name here to establish linkage prior to definition}} expected-note {{declared here}}
+  void test() {
+    foo(this); // expected-warning {{template argument uses unnamed type}}
+  }
+} A; // expected-error {{unsupported: typedef changes linkage of anonymous type, but linkage was already computed}}