Fri Jan 11 18:18:32 PST 2013

Applied in r172295. Please confirm.

...Takumi

2013/1/12 NAKAMURA Takumi <geek4civic at gmail.com>:
> 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