r279486 - Fix regression introduced by r279164: only pass definitions as the PatternDef

Vassil Vassilev via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 23 14:08:38 PDT 2016


Thanks a lot!
On 23/08/16 22:50, Richard Smith via cfe-commits wrote:
> Thanks. Fixed and reapplied as r279557.
>
> On Mon, Aug 22, 2016 at 7:08 PM, Chandler Carruth <chandlerc at gmail.com 
> <mailto:chandlerc at gmail.com>> wrote:
>
>     Reverted this per Richard's request in r279500.
>
>
>     On Mon, Aug 22, 2016 at 3:33 PM Richard Smith via cfe-commits
>     <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
>     wrote:
>
>         Author: rsmith
>         Date: Mon Aug 22 17:25:03 2016
>         New Revision: 279486
>
>         URL: http://llvm.org/viewvc/llvm-project?rev=279486&view=rev
>         <http://llvm.org/viewvc/llvm-project?rev=279486&view=rev>
>         Log:
>         Fix regression introduced by r279164: only pass definitions as
>         the PatternDef
>         to DiagnoseUninstantiableTemplate, teach hasVisibleDefinition
>         to correctly
>         determine whether a function definition is visible, and mark
>         both the function
>         and the template as visible when merging function template
>         definitions to
>         provide hasVisibleDefinition with the relevant information.
>
>         The change to always pass the right declaration as the
>         PatternDef to
>         DiagnoseUninstantiableTemplate also caused those checks to
>         happen before other
>         diagnostics in InstantiateFunctionDefinition, giving worse
>         diagnostics for the
>         same situations, so I sunk the relevant diagnostics into
>         DiagnoseUninstantiableTemplate. Those parts of this patch are
>         based on changes
>         in reviews.llvm.org/D23492 <http://reviews.llvm.org/D23492> by
>         Vassil Vassilev.
>
>         Modified:
>             cfe/trunk/lib/Sema/SemaDecl.cpp
>             cfe/trunk/lib/Sema/SemaTemplate.cpp
>             cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>             cfe/trunk/lib/Sema/SemaType.cpp
>            
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
>            
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
>             cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>
>         Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>         +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Aug 22 17:25:03 2016
>         @@ -11274,9 +11274,8 @@ Sema::CheckForFunctionRedefinition(Funct
>              SkipBody->ShouldSkip = true;
>              if (auto *TD = Definition->getDescribedFunctionTemplate())
>                makeMergedDefinitionVisible(TD, FD->getLocation());
>         -    else
>         -     
>         makeMergedDefinitionVisible(const_cast<FunctionDecl*>(Definition),
>         - FD->getLocation());
>         +   
>         makeMergedDefinitionVisible(const_cast<FunctionDecl*>(Definition),
>         + FD->getLocation());
>              return;
>            }
>
>
>         Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
>         +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Aug 22 17:25:03 2016
>         @@ -487,8 +487,6 @@ bool Sema::DiagnoseUninstantiableTemplat
>            QualType InstantiationTy;
>            if (TagDecl *TD = dyn_cast<TagDecl>(Instantiation))
>              InstantiationTy = Context.getTypeDeclType(TD);
>         -  else
>         -    InstantiationTy =
>         cast<FunctionDecl>(Instantiation)->getType();
>            if (!Complain || (PatternDef && PatternDef->isInvalidDecl())) {
>              // Say nothing
>            } else if (PatternDef) {
>         @@ -500,15 +498,30 @@ bool Sema::DiagnoseUninstantiableTemplat
>              // we're lexically inside it.
>              Instantiation->setInvalidDecl();
>            } else if (InstantiatedFromMember) {
>         -    Diag(PointOfInstantiation,
>         -         diag::err_implicit_instantiate_member_undefined)
>         -      << InstantiationTy;
>         -    Diag(Pattern->getLocation(), diag::note_member_declared_at);
>         +    if (isa<FunctionDecl>(Instantiation)) {
>         +      Diag(PointOfInstantiation,
>         +           diag::err_explicit_instantiation_undefined_member)
>         +        << 1 << Instantiation->getDeclName() <<
>         Instantiation->getDeclContext();
>         +    } else {
>         +      Diag(PointOfInstantiation,
>         +           diag::err_implicit_instantiate_member_undefined)
>         +        << InstantiationTy;
>         +    }
>         +    Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
>         +                                     ?
>         diag::note_explicit_instantiation_here
>         +                                     :
>         diag::note_member_declared_at);
>            } else {
>         -    Diag(PointOfInstantiation,
>         diag::err_template_instantiate_undefined)
>         -      << (TSK != TSK_ImplicitInstantiation)
>         -      << InstantiationTy;
>         -    Diag(Pattern->getLocation(), diag::note_template_decl_here);
>         +    if (isa<FunctionDecl>(Instantiation))
>         +      Diag(PointOfInstantiation,
>         +         
>          diag::err_explicit_instantiation_undefined_func_template)
>         +        << Pattern;
>         +    else
>         +      Diag(PointOfInstantiation,
>         diag::err_template_instantiate_undefined)
>         +        << (TSK != TSK_ImplicitInstantiation)
>         +        << InstantiationTy;
>         +    Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
>         +                                     ?
>         diag::note_explicit_instantiation_here
>         +                                     :
>         diag::note_template_decl_here);
>            }
>
>            // In general, Instantiation isn't marked invalid to get
>         more than one
>
>         Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
>         +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Aug
>         22 17:25:03 2016
>         @@ -3554,23 +3554,38 @@ void Sema::InstantiateFunctionDefinition
>            const FunctionDecl *PatternDecl =
>         Function->getTemplateInstantiationPattern();
>            assert(PatternDecl && "instantiating a non-template");
>
>         -  Stmt *Pattern = PatternDecl->getBody(PatternDecl);
>         -  assert(PatternDecl && "template definition is not a template");
>         -  if (!Pattern) {
>         -    // Try to find a defaulted definition
>         -    PatternDecl->isDefined(PatternDecl);
>         -  }
>         -  assert(PatternDecl && "template definition is not a template");
>         +  const FunctionDecl *PatternDef = PatternDecl->getDefinition();
>         +  Stmt *Pattern = PatternDef->getBody(PatternDef);
>         +  if (PatternDef)
>         +    PatternDecl = PatternDef;
>
>            // FIXME: We need to track the instantiation stack in order
>         to know which
>            // definitions should be visible within this instantiation.
>            if (DiagnoseUninstantiableTemplate(PointOfInstantiation,
>         Function,
>                                        
>          Function->getInstantiatedFromMemberFunction(),
>         -                                     PatternDecl,
>         PatternDecl, TSK,
>         -                                   
>          /*Complain*/DefinitionRequired))
>         -     return;
>         -
>         +                                     PatternDecl, PatternDef,
>         TSK,
>         +                                   
>          /*Complain*/DefinitionRequired)) {
>         +    if (DefinitionRequired)
>         +      Function->setInvalidDecl();
>         +    else if (TSK == TSK_ExplicitInstantiationDefinition) {
>         +      // Try again at the end of the translation unit (at
>         which point a
>         +      // definition will be required).
>         +      assert(!Recursive);
>         +      PendingInstantiations.push_back(
>         +        std::make_pair(Function, PointOfInstantiation));
>         +    } else if (TSK == TSK_ImplicitInstantiation) {
>         +      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
>         +        Diag(PointOfInstantiation,
>         diag::warn_func_template_missing)
>         +          << Function;
>         +        Diag(PatternDecl->getLocation(),
>         diag::note_forward_template_decl);
>         +        if (getLangOpts().CPlusPlus11)
>         +          Diag(PointOfInstantiation,
>         diag::note_inst_declaration_hint)
>         +            << Function;
>         +      }
>         +    }
>
>         +    return;
>         +  }
>
>            // Postpone late parsed template instantiations.
>            if (PatternDecl->isLateTemplateParsed() &&
>         @@ -3604,40 +3619,9 @@ void Sema::InstantiateFunctionDefinition
>              Pattern = PatternDecl->getBody(PatternDecl);
>            }
>
>         -  // FIXME: Check if we could sink these diagnostics in
>         -  // DiagnoseUninstantiableTemplate.
>         -  if (!Pattern && !PatternDecl->isDefaulted()) {
>         -    if (DefinitionRequired) {
>         -      if (Function->getPrimaryTemplate())
>         -        Diag(PointOfInstantiation,
>         -           
>          diag::err_explicit_instantiation_undefined_func_template)
>         -          << Function->getPrimaryTemplate();
>         -      else
>         -        Diag(PointOfInstantiation,
>         -             diag::err_explicit_instantiation_undefined_member)
>         -          << 1 << Function->getDeclName() <<
>         Function->getDeclContext();
>         -
>         -      if (PatternDecl)
>         -        Diag(PatternDecl->getLocation(),
>         -             diag::note_explicit_instantiation_here);
>         -      Function->setInvalidDecl();
>         -    } else if (TSK == TSK_ExplicitInstantiationDefinition) {
>         -      assert(!Recursive);
>         -      PendingInstantiations.push_back(
>         -        std::make_pair(Function, PointOfInstantiation));
>         -    } else if (TSK == TSK_ImplicitInstantiation) {
>         -      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
>         -        Diag(PointOfInstantiation,
>         diag::warn_func_template_missing)
>         -          << Function;
>         -        Diag(PatternDecl->getLocation(),
>         diag::note_forward_template_decl);
>         -        if (getLangOpts().CPlusPlus11)
>         -          Diag(PointOfInstantiation,
>         diag::note_inst_declaration_hint)
>         -            << Function;
>         -      }
>         -    }
>         -
>         -    return;
>         -  }
>         +  // Note, we should never try to instantiate a deleted
>         function template.
>         +  assert((Pattern || PatternDecl->isDefaulted()) &&
>         +         "unexpected kind of function template definition");
>
>            // C++1y [temp.explicit]p10:
>            //   Except for inline functions, declarations with types
>         deduced from their
>
>         Modified: cfe/trunk/lib/Sema/SemaType.cpp
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         --- cfe/trunk/lib/Sema/SemaType.cpp (original)
>         +++ cfe/trunk/lib/Sema/SemaType.cpp Mon Aug 22 17:25:03 2016
>         @@ -6890,6 +6890,10 @@ bool Sema::hasVisibleDefinition(NamedDec
>                return false;
>              }
>              D = ED->getDefinition();
>         +  } else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
>         +    if (auto *Pattern = FD->getTemplateInstantiationPattern())
>         +      FD = Pattern;
>         +    D = FD->getDefinition();
>            }
>            assert(D && "missing definition for pattern of instantiated
>         definition");
>
>         @@ -6897,7 +6901,7 @@ bool Sema::hasVisibleDefinition(NamedDec
>            if (isVisible(D))
>              return true;
>
>         -  // The external source may have additional definitions of
>         this type that are
>         +  // The external source may have additional definitions of
>         this entity that are
>            // visible, so complete the redeclaration chain now and ask
>         again.
>            if (auto *Source = Context.getExternalSource()) {
>              Source->CompleteRedeclChain(D);
>
>         Modified:
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         ---
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
>         (original)
>         +++
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
>         Mon Aug 22 17:25:03 2016
>         @@ -3,3 +3,4 @@ template<typename T> struct B;
>
>          template<typename, typename> struct A {};
>          template<typename T> struct B : A<T> {};
>         +template<typename T> inline auto C(T) {}
>
>         Modified:
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         ---
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
>         (original)
>         +++
>         cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
>         Mon Aug 22 17:25:03 2016
>         @@ -3,7 +3,9 @@ template<typename T> struct B;
>
>          template<typename, typename> struct A {};
>          template<typename T> struct B : A<T> {};
>         +template<typename T> inline auto C(T) {}
>
>          inline void f() {
>            B<int> bi;
>         +  C(0);
>          }
>
>         Modified:
>         cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>         URL:
>         http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=279486&r1=279485&r2=279486&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=279486&r1=279485&r2=279486&view=diff>
>         ==============================================================================
>         ---
>         cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>         (original)
>         +++
>         cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>         Mon Aug 22 17:25:03 2016
>         @@ -1,4 +1,4 @@
>          // RUN: rm -rf %t
>         -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t
>         -fno-modules-error-recovery \
>         +// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery
>         -std=c++14 \
>          // RUN:            -fmodule-name=X -emit-module
>         %S/Inputs/merge-template-pattern-visibility/module.modulemap
>         -x c++ \
>         -// RUN:            -fmodules-local-submodule-visibility
>         +// RUN:            -fmodules-local-submodule-visibility -o
>         %t/X.pcm
>
>
>         _______________________________________________
>         cfe-commits mailing list
>         cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>         http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>         <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160824/0d345a86/attachment-0001.html>


More information about the cfe-commits mailing list