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

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 19:08:59 PDT 2016


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> 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
> 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 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
> 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/20160823/f52229bd/attachment-0001.html>


More information about the cfe-commits mailing list