Fri Jan 11 18:08:40 PST 2013

Doug, didn't you update unittests?
http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/2487

(I could add no-op VoidModuleLoader::makeModuleVisible())

...Takumi

2013/1/12 Douglas Gregor <dgregor at apple.com>:
> Author: dgregor
> Date: Fri Jan 11 19:29:50 2013
> New Revision: 172290
>
> URL: http://llvm.org/viewvc/llvm-project?rev=172290&view=rev
> Log:
> Provide Decl::getOwningModule(), which determines the (sub)module in
> which a particular declaration resides. Use this information to
> customize the "definition of 'blah' must be imported from another
> module" diagnostic with the module the user actually has to
> import. Additionally, recover by importing that module, so we don't
> complain about other names in that module.
>
> Still TODO: coming up with decent Fix-Its for these cases, and expand
> this recovery approach for other name lookup failures.
>
>
> Modified:
>     cfe/trunk/include/clang/AST/DeclBase.h
>     cfe/trunk/include/clang/AST/ExternalASTSource.h
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Frontend/ASTUnit.h
>     cfe/trunk/include/clang/Frontend/CompilerInstance.h
>     cfe/trunk/include/clang/Lex/ModuleLoader.h
>     cfe/trunk/include/clang/Sema/Lookup.h
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/include/clang/Serialization/ASTReader.h
>     cfe/trunk/lib/AST/DeclBase.cpp
>     cfe/trunk/lib/Frontend/CompilerInstance.cpp
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/lib/Sema/SemaType.cpp
>     cfe/trunk/lib/Serialization/ASTReader.cpp
>     cfe/trunk/test/Modules/decldef.mm
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Fri Jan 11 19:29:50 2013
> @@ -32,6 +32,7 @@
>  class EnumDecl;
>  class FunctionDecl;
>  class LinkageSpecDecl;
> +class Module;
>  class NamedDecl;
>  class NamespaceDecl;
>  class ObjCCategoryDecl;
> @@ -593,7 +594,18 @@
>
>      return 0;
>    }
> -
> +
> +private:
> +  Module *getOwningModuleSlow() const;
> +
> +public:
> +  Module *getOwningModule() const {
> +    if (!isFromASTFile())
> +      return 0;
> +
> +    return getOwningModuleSlow();
> +  }
> +
>    unsigned getIdentifierNamespace() const {
>      return IdentifierNamespace;
>    }
>
> Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
> +++ cfe/trunk/include/clang/AST/ExternalASTSource.h Fri Jan 11 19:29:50 2013
> @@ -25,6 +25,7 @@
>  class DeclarationName;
>  class ExternalSemaSource; // layering violation required for downcasting
>  class FieldDecl;
> +class Module;
>  class NamedDecl;
>  class RecordDecl;
>  class Selector;
> @@ -131,9 +132,12 @@
>    /// \brief Ensures that the table of all visible declarations inside this
>    /// context is up to date.
>    ///
> -  /// The default implementation of this functino is a no-op.
> +  /// The default implementation of this function is a no-op.
>    virtual void completeVisibleDeclsMap(const DeclContext *DC);
>
> +  /// \brief Retrieve the module that corresponds to the given module ID.
> +  virtual Module *getModule(unsigned ID) { return 0; }
> +
>    /// \brief Finds all declarations lexically contained within the given
>    /// DeclContext, after applying an optional filter predicate.
>    ///
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 11 19:29:50 2013
> @@ -6082,7 +6082,7 @@
>    "local %select{struct|interface|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">;
> +  "definition of %0 must be imported from module '%1' before it is required">;
>  }
>
>  let CategoryName = "Documentation Issue" in {
>
> Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
> +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Fri Jan 11 19:29:50 2013
> @@ -837,6 +837,10 @@
>      // ASTUnit doesn't know how to load modules (not that this matters).
>      return ModuleLoadResult();
>    }
> +
> +  virtual void makeModuleVisible(Module *Mod,
> +                                 Module::NameVisibilityKind Visibility) { }
> +
>  };
>
>  } // namespace clang
>
> Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
> +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Fri Jan 11 19:29:50 2013
> @@ -649,6 +649,10 @@
>                                        ModuleIdPath Path,
>                                        Module::NameVisibilityKind Visibility,
>                                        bool IsInclusionDirective);
> +
> +  virtual void makeModuleVisible(Module *Mod,
> +                                 Module::NameVisibilityKind Visibility);
> +
>  };
>
>  } // end namespace clang
>
> Modified: cfe/trunk/include/clang/Lex/ModuleLoader.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleLoader.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/ModuleLoader.h (original)
> +++ cfe/trunk/include/clang/Lex/ModuleLoader.h Fri Jan 11 19:29:50 2013
> @@ -80,6 +80,10 @@
>                                        ModuleIdPath Path,
>                                        Module::NameVisibilityKind Visibility,
>                                        bool IsInclusionDirective) = 0;
> +
> +  /// \brief Make the given module visible.
> +  virtual void makeModuleVisible(Module *Mod,
> +                                 Module::NameVisibilityKind Visibility) = 0;
>  };
>
>  }
>
> Modified: cfe/trunk/include/clang/Sema/Lookup.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Lookup.h (original)
> +++ cfe/trunk/include/clang/Sema/Lookup.h Fri Jan 11 19:29:50 2013
> @@ -138,7 +138,8 @@
>        IDNS(0),
>        Redecl(Redecl != Sema::NotForRedeclaration),
>        HideTags(true),
> -      Diagnose(Redecl == Sema::NotForRedeclaration)
> +      Diagnose(Redecl == Sema::NotForRedeclaration),
> +      AllowHidden(Redecl == Sema::ForRedeclaration)
>    {
>      configure();
>    }
> @@ -158,7 +159,8 @@
>        IDNS(0),
>        Redecl(Redecl != Sema::NotForRedeclaration),
>        HideTags(true),
> -      Diagnose(Redecl == Sema::NotForRedeclaration)
> +      Diagnose(Redecl == Sema::NotForRedeclaration),
> +      AllowHidden(Redecl == Sema::ForRedeclaration)
>    {
>      configure();
>    }
> @@ -176,7 +178,8 @@
>        IDNS(Other.IDNS),
>        Redecl(Other.Redecl),
>        HideTags(Other.HideTags),
> -      Diagnose(false)
> +      Diagnose(false),
> +      AllowHidden(Other.AllowHidden)
>    {}
>
>    ~LookupResult() {
> @@ -214,10 +217,16 @@
>      return Redecl;
>    }
>
> +  /// \brief Specify whether hidden declarations are visible, e.g.,
> +  /// for recovery reasons.
> +  void setAllowHidden(bool AH) {
> +    AllowHidden = AH;
> +  }
> +
>    /// \brief Determine whether this lookup is permitted to see hidden
>    /// declarations, such as those in modules that have not yet been imported.
>    bool isHiddenDeclarationVisible() const {
> -    return Redecl || LookupKind == Sema::LookupTagName;
> +    return AllowHidden || LookupKind == Sema::LookupTagName;
>    }
>
>    /// Sets whether tag declarations should be hidden by non-tag
> @@ -483,6 +492,7 @@
>    /// \brief Change this lookup's redeclaration kind.
>    void setRedeclarationKind(Sema::RedeclarationKind RK) {
>      Redecl = RK;
> +    AllowHidden = (RK == Sema::ForRedeclaration);
>      configure();
>    }
>
> @@ -644,6 +654,9 @@
>    bool HideTags;
>
>    bool Diagnose;
> +
> +  /// \brief True if we should allow hidden declarations to be 'visible'.
> +  bool AllowHidden;
>  };
>
>    /// \brief Consumes visible declarations found when searching for
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 11 19:29:50 2013
> @@ -275,12 +275,6 @@
>    /// 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.
>    OwningPtr<CXXFieldCollector> FieldCollector;
>
> @@ -1456,6 +1450,14 @@
>    DeclResult ActOnModuleImport(SourceLocation AtLoc, SourceLocation ImportLoc,
>                                 ModuleIdPath Path);
>
> +  /// \brief Create an implicit import of the given module at the given
> +  /// source location.
> +  ///
> +  /// This routine is typically used for error recovery, when the entity found
> +  /// by name lookup is actually hidden within a module that we know about but
> +  /// the user has forgotten to import.
> +  void createImplicitModuleImport(SourceLocation Loc, Module *Mod);
> +
>    /// \brief Retrieve a suitable printing policy.
>    PrintingPolicy getPrintingPolicy() const {
>      return getPrintingPolicy(Context, PP);
>
> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jan 11 19:29:50 2013
> @@ -1558,7 +1558,12 @@
>    /// \brief Retrieve the submodule that corresponds to a global submodule ID.
>    ///
>    Module *getSubmodule(serialization::SubmoduleID GlobalID);
> -
> +
> +  /// \brief Retrieve the module that corresponds to the given module ID.
> +  ///
> +  /// Note: overrides method in ExternalASTSource
> +  virtual Module *getModule(unsigned ID);
> +
>    /// \brief Retrieve a selector from the given module with its local ID
>    /// number.
>    Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
>
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Fri Jan 11 19:29:50 2013
> @@ -59,6 +59,11 @@
>    return Result;
>  }
>
> +Module *Decl::getOwningModuleSlow() const {
> +  assert(isFromASTFile() && "Not from AST file?");
> +  return getASTContext().getExternalSource()->getModule(getOwningModuleID());
> +}
> +
>  const char *Decl::getDeclKindName() const {
>    switch (DeclKind) {
>    default: llvm_unreachable("Declaration not in DeclNodes.inc!");
>
> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Jan 11 19:29:50 2013
> @@ -1201,3 +1201,9 @@
>    LastModuleImportResult = ModuleLoadResult(Module, false);
>    return LastModuleImportResult;
>  }
> +
> +void CompilerInstance::makeModuleVisible(Module *Mod,
> +                                         Module::NameVisibilityKind Visibility){
> +  ModuleManager->makeModuleVisible(Mod, Visibility);
> +}
> +
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jan 11 19:29:50 2013
> @@ -11173,6 +11173,18 @@
>    return Import;
>  }
>
> +void Sema::createImplicitModuleImport(SourceLocation Loc, Module *Mod) {
> +  // Create the implicit import declaration.
> +  TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl();
> +  ImportDecl *ImportD = ImportDecl::CreateImplicit(getASTContext(), TU,
> +                                                   Loc, Mod, Loc);
> +  TU->addDecl(ImportD);
> +  Consumer.HandleImplicitImportDecl(ImportD);
> +
> +  // Make the module visible.
> +  PP.getModuleLoader().makeModuleVisible(Mod, Module::AllVisible);
> +}
> +
>  void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name,
>                                        IdentifierInfo* AliasName,
>                                        SourceLocation PragmaLoc,
>
> Modified: cfe/trunk/lib/Sema/SemaType.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaType.cpp Fri Jan 11 19:29:50 2013
> @@ -4392,9 +4392,14 @@
>        // 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);
> +      Module *Owner = Def->getOwningModule();
> +      Diag(Loc, diag::err_module_private_definition)
> +        << T << Owner->getFullModuleName();
> +      Diag(Def->getLocation(), diag::note_previous_definition);
> +
> +      if (!isSFINAEContext()) {
> +        // Recover by implicitly importing this module.
> +        createImplicitModuleImport(Loc, Owner);
>        }
>      }
>
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jan 11 19:29:50 2013
> @@ -6200,7 +6200,11 @@
>
>    return SubmodulesLoaded[GlobalID - NUM_PREDEF_SUBMODULE_IDS];
>  }
> -
> +
> +Module *ASTReader::getModule(unsigned ID) {
> +  return getSubmodule(ID);
> +}
> +
>  Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
>    return DecodeSelector(getGlobalSelectorID(M, LocalID));
>  }
>
> Modified: cfe/trunk/test/Modules/decldef.mm
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/decldef.mm?rev=172290&r1=172289&r2=172290&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/decldef.mm (original)
> +++ cfe/trunk/test/Modules/decldef.mm Fri Jan 11 19:29:50 2013
> @@ -2,13 +2,7 @@
>  // RUN: %clang_cc1 -fmodules -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}}
> +// In other file: expected-note {{previous definition is here}}
>
>  @import decldef;
>  A *a1; // expected-error{{unknown type name 'A'}}
> @@ -19,10 +13,9 @@
>  B *b;
>
>  void testA(A *a) {
> -  a->ivar = 17; // expected-error{{definition of 'A' must be imported before it is required}}
> +  a->ivar = 17; // expected-error{{definition of 'A' must be imported from module 'decldef.Def' 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.
> +  B b; // Note: redundant error silenced
>  }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits