bruno created this revision.
bruno added a reviewer: rsmith.
Herald added a subscriber: jkorous.

Support for ObjC/C ODR-like semantics with structural equivalence checking was 
added back in r306918. There enums are handled and also checked for structural 
equivalence. However, at use time of EnumConstantDecl, support was missing for 
preventing ambiguous name lookup.

Add the missing bits for properly merging EnumConstantDecl.

rdar://problem/38374569


Repository:
  rC Clang

https://reviews.llvm.org/D46165

Files:
  lib/Serialization/ASTReaderDecl.cpp
  test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
  test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
  test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
  test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
  test/Modules/non-ambiguous-enum.m


Index: test/Modules/non-ambiguous-enum.m
===================================================================
--- /dev/null
+++ test/Modules/non-ambiguous-enum.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-F%S/Inputs/non-ambiguous-enum -fsyntax-only %s -verify
+#import <B/B.h>
+#import <A/A.h>
+
+// expected-no-diagnostics
+
+int foo() {
+  return MyEnumCst;
+}
Index: test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
@@ -0,0 +1,6 @@
+typedef long NSInteger;
+typedef enum __attribute__((flag_enum,enum_extensibility(open))) MyObjCEnum : 
NSInteger MyObjCEnum;
+
+enum MyObjCEnum : NSInteger {
+    MyEnumCst = 1,
+} __attribute__((availability(ios,introduced=11.0))) 
__attribute__((availability(tvos,unavailable))) ;
Index: 
test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
@@ -0,0 +1,6 @@
+
+framework module A {
+  header "a.h"
+  //module * { export * }
+  export *
+}
Index: test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
@@ -0,0 +1 @@
+#include <B/b.h>
Index: test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
@@ -0,0 +1 @@
+#include <A/a0.h>
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2541,6 +2541,20 @@
   }
 }
 
+/// ODR-like semantics for C/ObjC allow us to merge tag types and a structural
+/// check in Sema guarantees the types can be merged (see C11 6.2.7/1 or C89
+/// 6.1.2.6/1). Although most merging is done in Sema, we need to guarantee
+/// that some types are mergeable during deserialization, otherwise name
+/// lookup fails. This is the case for EnumConstantDecl.
+bool allowODRLikeMergeInC(NamedDecl *ND) {
+  if (!ND)
+    return false;
+  // TODO: implement merge for other necessary decls.
+  if (dyn_cast<EnumConstantDecl>(ND))
+    return true;
+  return false;
+}
+
 /// \brief Attempts to merge the given declaration (D) with another declaration
 /// of the same entity, for the case where the entity is not actually
 /// redeclarable. This happens, for instance, when merging the fields of
@@ -2553,8 +2567,10 @@
 
   // ODR-based merging is only performed in C++. In C, identically-named things
   // in different translation units are not redeclarations (but may still have
-  // compatible types).
-  if (!Reader.getContext().getLangOpts().CPlusPlus)
+  // compatible types). However, clang is able to merge tag types with ODR-like
+  // semantics if it meets some requirements.
+  if (!Reader.getContext().getLangOpts().CPlusPlus &&
+      !allowODRLikeMergeInC(dyn_cast<NamedDecl>(static_cast<T*>(D))))
     return;
 
   if (FindExistingResult ExistingRes = findExisting(static_cast<T*>(D)))


Index: test/Modules/non-ambiguous-enum.m
===================================================================
--- /dev/null
+++ test/Modules/non-ambiguous-enum.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/non-ambiguous-enum -fsyntax-only %s -verify
+#import <B/B.h>
+#import <A/A.h>
+
+// expected-no-diagnostics
+
+int foo() {
+  return MyEnumCst;
+}
Index: test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
@@ -0,0 +1,6 @@
+typedef long NSInteger;
+typedef enum __attribute__((flag_enum,enum_extensibility(open))) MyObjCEnum : NSInteger MyObjCEnum;
+
+enum MyObjCEnum : NSInteger {
+    MyEnumCst = 1,
+} __attribute__((availability(ios,introduced=11.0))) __attribute__((availability(tvos,unavailable))) ;
Index: test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
@@ -0,0 +1,6 @@
+
+framework module A {
+  header "a.h"
+  //module * { export * }
+  export *
+}
Index: test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
@@ -0,0 +1 @@
+#include <B/b.h>
Index: test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
@@ -0,0 +1 @@
+#include <A/a0.h>
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2541,6 +2541,20 @@
   }
 }
 
+/// ODR-like semantics for C/ObjC allow us to merge tag types and a structural
+/// check in Sema guarantees the types can be merged (see C11 6.2.7/1 or C89
+/// 6.1.2.6/1). Although most merging is done in Sema, we need to guarantee
+/// that some types are mergeable during deserialization, otherwise name
+/// lookup fails. This is the case for EnumConstantDecl.
+bool allowODRLikeMergeInC(NamedDecl *ND) {
+  if (!ND)
+    return false;
+  // TODO: implement merge for other necessary decls.
+  if (dyn_cast<EnumConstantDecl>(ND))
+    return true;
+  return false;
+}
+
 /// \brief Attempts to merge the given declaration (D) with another declaration
 /// of the same entity, for the case where the entity is not actually
 /// redeclarable. This happens, for instance, when merging the fields of
@@ -2553,8 +2567,10 @@
 
   // ODR-based merging is only performed in C++. In C, identically-named things
   // in different translation units are not redeclarations (but may still have
-  // compatible types).
-  if (!Reader.getContext().getLangOpts().CPlusPlus)
+  // compatible types). However, clang is able to merge tag types with ODR-like
+  // semantics if it meets some requirements.
+  if (!Reader.getContext().getLangOpts().CPlusPlus &&
+      !allowODRLikeMergeInC(dyn_cast<NamedDecl>(static_cast<T*>(D))))
     return;
 
   if (FindExistingResult ExistingRes = findExisting(static_cast<T*>(D)))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D46165: [... Bruno Cardoso Lopes via Phabricator via cfe-commits

Reply via email to