Teach clang how to merge typedef over anonymous structs in C mode.

[v.g.vassilev]

Summary

empty.h makes module A available. textual.h brings the definition of fsid_t and a.h imports again the definition of fsid_t.

In C mode clang fails to merge the textually included definition with the one imported from a module. The C lookup rules fail to find the imported definition because its linkage is internal in non C++ mode.

This patch reinstates some of the ODR merging rules for typedefs of anonymous tags for languages other than C++.

Patch by Raphael Isemann and I.

[rsmith]

https://reviews.llvm.org/D31778 is related. We need to have a design for how ODR-like issues in C will be resolved before we'll know what the right fix is here. Prior to D31778 my intention had been to implement C's structural typing ("compatible type") rules and keep multiple definitions of RecordDecls in C as separate entities, but D31778 is suggesting that we use something more ODR-like instead, which is also more in line with the direction that this patch takes.

It's notable that C has no notion of "typedef name for linkage", so this patch does not address all the typedef-related issues that might show up in C code. For example:

typedef struct { ... } *ptr;

in two different modules would not be merged into a single type with this approach (you'd need to actually implement the C compatible type rules to get that to work). But maybe that's OK -- the above also does not (formally) work in C++, and it doesn't seem to cause major problems.

[v.g.vassilev]

@bruno ping...

[bruno]

Hi @v.g.vassilev, sorry for the delay.

Just updated D31778. I agree with Richard's observations, do you think you can extend it to work in the same way as D31778 does? The structural checking is already abstracted there (should be straightforward to use).

[bruno]

Tested it locally, we actually need all these changes on top of D31778.

LGTM

[v.g.vassilev]

Landed in r306964.

lib/AST/Decl.cpp

switch (D->getKind()) {
     case Decl::EnumConstant:
       // C++ [basic.link]p4: an enumerator has the linkage of its enumeration.
       return getLVForDecl(cast<EnumDecl>(D->getDeclContext()), computation);
 
     case Decl::Typedef:
     case Decl::TypeAlias:
       // A typedef declaration has linkage if it gives a type a name for
       // linkage purposes.
-      if (!D->getASTContext().getLangOpts().CPlusPlus ||
-          !cast<TypedefNameDecl>(D)
+      if (!cast<TypedefNameDecl>(D)
                ->getAnonDeclWithTypedefName(/*AnyRedecl*/true))
         return LinkageInfo::none();
       break;
 
     case Decl::TemplateTemplateParm: // count these as external
     case Decl::NonTypeTemplateParm:
     case Decl::ObjCAtDefsField:
     case Decl::ObjCCategory:

lib/Sema/SemaDecl.cpp

while (Filter.hasNext()) {
     // different linkages.
     if (auto *OldTD = dyn_cast<TypedefNameDecl>(Old)) {
       if (S.Context.hasSameType(OldTD->getUnderlyingType(),
                                 Decl->getUnderlyingType()))
         continue;
 
       // If both declarations give a tag declaration a typedef name for linkage
       // purposes, then they declare the same entity.
-      if (S.getLangOpts().CPlusPlus &&
-          OldTD->getAnonDeclWithTypedefName(/*AnyRedecl*/true) &&
+      if (OldTD->getAnonDeclWithTypedefName(/*AnyRedecl*/true) &&
           Decl->getAnonDeclWithTypedefName())
         continue;
     }
 
     Filter.erase();
   }
 
   Filter.done();
void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
   // If the old declaration is invalid, just give up here.
   if (Old->isInvalidDecl())
     return New->setInvalidDecl();
 
   if (auto *OldTD = dyn_cast<TypedefNameDecl>(Old)) {
     auto *OldTag = OldTD->getAnonDeclWithTypedefName(/*AnyRedecl*/true);
     auto *NewTag = New->getAnonDeclWithTypedefName();
     NamedDecl *Hidden = nullptr;
-    if (getLangOpts().CPlusPlus && OldTag && NewTag &&
+    if (OldTag && NewTag &&
         OldTag->getCanonicalDecl() != NewTag->getCanonicalDecl() &&
         !hasVisibleDefinition(OldTag, &Hidden)) {
       // There is a definition of this tag, but it is not visible. Use it
       // instead of our tag.
       New->setTypeForDecl(OldTD->getTypeForDecl());
       if (OldTD->isModed())
         New->setModedTypeSourceInfo(OldTD->getTypeSourceInfo(),
                                     OldTD->getUnderlyingType());

test/Modules/Inputs/merge-typedefs-c/a.h

+#ifndef A_H
+#define A_H
+#include "textual.h"
+#endif

test/Modules/Inputs/merge-typedefs-c/empty.h

+// This is a module trigger.

test/Modules/Inputs/merge-typedefs-c/module.modulemap

+module "A" {
+   export *
+   module "a.h" {
+     export *
+     header "a.h"
+   }
+   module "empty.h" {
+     export *
+     header "empty.h"
+   }
+ }

test/Modules/Inputs/merge-typedefs-c/textual.h

+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+typedef struct { } __fsid_t;
+
+#endif
+

test/Modules/merge-typedefs-c.c

+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -xc -fmodules-cache-path=%t -I%S/Inputs/merge-typedefs-c/ -fimplicit-module-maps -verify %s
+// RUN: %clang_cc1 -fmodules -xc++ -fmodules-cache-path=%t -I%S/Inputs/merge-typedefs-c/ -fimplicit-module-maps -verify %s
+
+// expected-no-diagnostics
+#include "empty.h"
+#include "textual.h"
+#include "a.h"
+
+__fsid_t use;