Diagnose cases where the definition of a particular type is required,

is known (to Clang), but is not visible because the module has not yet
been imported.

llvm-svn: 147436

clang/include/clang/AST/Type.h

@@ -1324,7 +1324,11 @@ public:
   /// A type that can describe objects, but which lacks information needed to
   /// determine its size (e.g. void, or a fwd declared struct). Clients of this
   /// routine will need to determine if the size is actually required.
-  bool isIncompleteType() const;
+  ///
+  /// \brief Def If non-NULL, and the type refers to some kind of declaration
+  /// that can be completed (such as a C struct, C++ class, or Objective-C
+  /// class), will be set to the declaration.
+  bool isIncompleteType(NamedDecl **Def = 0) const;
 
   /// isIncompleteOrObjectType - Return true if this is an incomplete or object
   /// type, in other words, not a function type.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5167,6 +5167,8 @@ def err_module_private_local : Error<
 def err_module_private_local_class : Error<
   "local %select{struct|union|class|enum}0 cannot be declared "
   "__module_private__">;
+def err_module_private_definition : Error<
+  "definition of %0 must be imported before it is required">;
 }
 
 } // end of sema component.

clang/include/clang/Sema/Sema.h

@@ -247,6 +247,12 @@ public:
   /// This is only necessary for issuing pretty diagnostics.
   ExtVectorDeclsType ExtVectorDecls;
 
+  /// \brief The set of types for which we have already complained about the
+  /// definitions being hidden.
+  ///
+  /// This set is used to suppress redundant diagnostics.
+  llvm::SmallPtrSet<NamedDecl *, 4> HiddenDefinitions;
+  
   /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes.
   llvm::OwningPtr<CXXFieldCollector> FieldCollector;
 

clang/lib/AST/Type.cpp

@@ -897,37 +897,56 @@ bool Type::isConstantSizeType() const {
 /// isIncompleteType - Return true if this is an incomplete type (C99 6.2.5p1)
 /// - a type that can describe objects, but which lacks information needed to
 /// determine its size.
-bool Type::isIncompleteType() const {
+bool Type::isIncompleteType(NamedDecl **Def) const {
+  if (Def)
+    *Def = 0;
+  
   switch (CanonicalType->getTypeClass()) {
   default: return false;
   case Builtin:
     // Void is the only incomplete builtin type.  Per C99 6.2.5p19, it can never
     // be completed.
     return isVoidType();
-  case Enum:
+  case Enum: {
+    EnumDecl *EnumD = cast<EnumType>(CanonicalType)->getDecl();
+    if (Def)
+      *Def = EnumD;
+    
     // An enumeration with fixed underlying type is complete (C++0x 7.2p3).
-    if (cast<EnumType>(CanonicalType)->getDecl()->isFixed())
-        return false;
-    // Fall through.
-  case Record:
+    if (EnumD->isFixed())
+      return false;
+    
+    return !EnumD->isCompleteDefinition();
+  }
+  case Record: {
     // A tagged type (struct/union/enum/class) is incomplete if the decl is a
     // forward declaration, but not a full definition (C99 6.2.5p22).
-    return !cast<TagType>(CanonicalType)->getDecl()->isCompleteDefinition();
+    RecordDecl *Rec = cast<RecordType>(CanonicalType)->getDecl();
+    if (Def)
+      *Def = Rec;
+    return !Rec->isCompleteDefinition();
+  }
   case ConstantArray:
     // An array is incomplete if its element type is incomplete
     // (C++ [dcl.array]p1).
     // We don't handle variable arrays (they're not allowed in C++) or
     // dependent-sized arrays (dependent types are never treated as incomplete).
-    return cast<ArrayType>(CanonicalType)->getElementType()->isIncompleteType();
+    return cast<ArrayType>(CanonicalType)->getElementType()
+             ->isIncompleteType(Def);
   case IncompleteArray:
     // An array of unknown size is an incomplete type (C99 6.2.5p22).
     return true;
   case ObjCObject:
     return cast<ObjCObjectType>(CanonicalType)->getBaseType()
-                                                         ->isIncompleteType();
-  case ObjCInterface:
+             ->isIncompleteType(Def);
+  case ObjCInterface: {
     // ObjC interfaces are incomplete if they are @class, not @interface.
-    return !cast<ObjCInterfaceType>(CanonicalType)->getDecl()->hasDefinition();
+    ObjCInterfaceDecl *Interface
+      = cast<ObjCInterfaceType>(CanonicalType)->getDecl();
+    if (Def)
+      *Def = Interface;
+    return !Interface->hasDefinition();
+  }
   }
 }
 

clang/lib/Sema/SemaExprMember.cpp

@@ -1084,6 +1084,11 @@ Sema::LookupMemberExpr(LookupResult &R, ExprResult &BaseExpr,
       goto fail;
     }
 
+    if (RequireCompleteType(OpLoc, BaseType, 
+                            PDiag(diag::err_typecheck_incomplete_tag)
+                              << BaseExpr.get()->getSourceRange()))
+      return ExprError();
+    
     ObjCInterfaceDecl *ClassDeclared;
     ObjCIvarDecl *IV = IDecl->lookupInstanceVariable(Member, ClassDeclared);
 

clang/lib/Sema/SemaType.cpp

@@ -27,6 +27,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Lookup.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/ErrorHandling.h"
 using namespace clang;
@@ -4059,8 +4060,23 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
   //         "Can't ask whether a dependent type is complete");
 
   // If we have a complete type, we're done.
-  if (!T->isIncompleteType())
+  NamedDecl *Def = 0;
+  if (!T->isIncompleteType(&Def)) {
+    // If we know about the definition but it is not visible, complain.
+    if (diag != 0 && Def && !LookupResult::isVisible(Def)) {
+      // Suppress this error outside of a SFINAE context if we've already
+      // emitted the error once for this type. There's no usefulness in 
+      // repeating the diagnostic.
+      // FIXME: Add a Fix-It that imports the corresponding module or includes
+      // the header.
+      if (isSFINAEContext() || HiddenDefinitions.insert(Def)) {
+        Diag(Loc, diag::err_module_private_definition) << T;
+        Diag(Def->getLocation(), diag::note_previous_definition);
+      }
+    }
+    
     return false;
+  }
 
   const TagType *Tag = T->getAs<TagType>();
   const ObjCInterfaceType *IFace = 0;

clang/test/Modules/Inputs/def.h

@@ -1,4 +1,11 @@
-@interface A
+
+
+
+
+@interface A {
+@public
+  int ivar;
+}
 @end
 
 struct B {

clang/test/Modules/decldef.mm

@@ -1,15 +1,28 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -I %S/Inputs -fmodule-cache-path %t %s -verify
 
+
+// in other file: expected-note{{previous definition is here}}
+
+
+
+
+
+// in other file: expected-note{{previous definition is here}}
+
 __import_module__ decldef;
 A *a1; // expected-error{{unknown type name 'A'}}
 B *b1; // expected-error{{unknown type name 'B'}}
-
 __import_module__ decldef.Decl;
 
 A *a2;
 B *b;
 
-void testB() {
-  B b; // FIXME: Should error, because we can't see the definition.
+void testA(A *a) {
+  a->ivar = 17; // expected-error{{definition of 'A' must be imported before it is required}}
+}
+
+void testB() {
+  B b; // expected-error{{definition of 'B' must be imported before it is required}}
+  B b2; // Note: the reundant error was silenced.
 }