r214192 - Fix PR10177 where non-type template arguments to alias templates are not marked as used in dependent contexts. The fix actually forces non-dependent names to be checked at template definition time as expected from the standard.

[Richard Smith]

On Tue, Jul 29, 2014 at 11:44 AM, Larisse Voufo <lvoufo at google.com> wrote:

> Author: lvoufo
> Date: Tue Jul 29 13:44:19 2014
> New Revision: 214192
>
> URL: http://llvm.org/viewvc/llvm-project?rev=214192&view=rev
> Log:
> Fix PR10177 where non-type template arguments to alias templates are not
> marked as used in dependent contexts. The fix actually forces non-dependent
> names to be checked at template definition time as expected from the
> standard.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaExpr.cpp
>     cfe/trunk/test/SemaCXX/PR10177.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=214192&r1=214191&r2=214192&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul 29 13:44:19 2014
> @@ -12439,6 +12439,8 @@ static void DoMarkVarDeclReferenced(Sema
>           "Invalid Expr argument to DoMarkVarDeclReferenced");
>    Var->setReferenced();
>
> +  TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind();
> +
>    // If the context is not potentially evaluated, this is not an odr-use
> and
>    // does not trigger instantiation.
>    if (!IsPotentiallyEvaluatedContext(SemaRef)) {
> @@ -12453,25 +12455,26 @@ static void DoMarkVarDeclReferenced(Sema
>      // arguments, where local variables can't be used.
>      const bool RefersToEnclosingScope =
>          (SemaRef.CurContext != Var->getDeclContext() &&
> -         Var->getDeclContext()->isFunctionOrMethod() &&
> -         Var->hasLocalStorage());
> -    if (!RefersToEnclosingScope)
> -      return;
> -
> -    if (LambdaScopeInfo *const LSI = SemaRef.getCurLambda()) {
> -      // If a variable could potentially be odr-used, defer marking it so
> -      // until we finish analyzing the full expression for any
> lvalue-to-rvalue
> -      // or discarded value conversions that would obviate odr-use.
> -      // Add it to the list of potential captures that will be analyzed
> -      // later (ActOnFinishFullExpr) for eventual capture and odr-use
> marking
> -      // unless the variable is a reference that was initialized by a
> constant
> -      // expression (this will never need to be captured or odr-used).
> -      assert(E && "Capture variable should be used in an expression.");
> -      if (!Var->getType()->isReferenceType() ||
> -          !IsVariableNonDependentAndAConstantExpression(Var,
> SemaRef.Context))
> -        LSI->addPotentialCapture(E->IgnoreParens());
> +         Var->getDeclContext()->isFunctionOrMethod() &&
> Var->hasLocalStorage());
> +    if (RefersToEnclosingScope) {
> +      if (LambdaScopeInfo *const LSI = SemaRef.getCurLambda()) {
> +        // If a variable could potentially be odr-used, defer marking it
> so
> +        // until we finish analyzing the full expression for any
> +        // lvalue-to-rvalue
> +        // or discarded value conversions that would obviate odr-use.
> +        // Add it to the list of potential captures that will be analyzed
> +        // later (ActOnFinishFullExpr) for eventual capture and odr-use
> marking
> +        // unless the variable is a reference that was initialized by a
> constant
> +        // expression (this will never need to be captured or odr-used).
> +        assert(E && "Capture variable should be used in an expression.");
> +        if (!Var->getType()->isReferenceType() ||
> +            !IsVariableNonDependentAndAConstantExpression(Var,
> SemaRef.Context))
> +          LSI->addPotentialCapture(E->IgnoreParens());
> +      }
>      }
> -    return;
> +
> +    if (!isTemplateInstantiation(TSK))
> +       return;
>

Should we really mark the variable as odr-used in this case but not for
non-template-instantiations? That inconsistency doesn't seem right to me. I
think the right behavior here is to *only* perform the instantiation and to
skip the marking as odr-used. In particular, for a case like:

  namespace { template<typename> extern int n; }
  template<typename T> int g() { return n<int>; }

... we should *not* produce a "variable '(anonymous namespace)::n<int>' has
internal linkage but is not defined" warning.

   }
>
>    VarTemplateSpecializationDecl *VarSpec =
> @@ -12483,7 +12486,6 @@ static void DoMarkVarDeclReferenced(Sema
>    // templates of class templates, and variable template specializations.
> Delay
>    // instantiations of variable templates, except for those that could be
> used
>    // in a constant expression.
> -  TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind();
>    if (isTemplateInstantiation(TSK)) {
>      bool TryInstantiating = TSK == TSK_ImplicitInstantiation;
>
>
> Modified: cfe/trunk/test/SemaCXX/PR10177.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR10177.cpp?rev=214192&r1=214191&r2=214192&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/PR10177.cpp (original)
> +++ cfe/trunk/test/SemaCXX/PR10177.cpp Tue Jul 29 13:44:19 2014
> @@ -9,22 +9,28 @@ struct U {
>    static int a;
>  };
>
> -template<int N> struct S; // expected-note 2{{here}}
> +template<int N> struct S; // expected-note 6{{here}}
>
>  template<int N>
> -int U<N>::a = S<N>::kError; // expected-error 2{{undefined}}
> +int U<N>::a = S<N>::kError; // expected-error 6{{undefined}}
>
>  template<typename T>
>  void f() {
> -  // FIXME: The standard suggests that U<0>::a is odr-used by this
> expression,
> -  // but it's not entirely clear that's the right behaviour.
> -  (void)alias_ref<int, int&, U<0>::a>();
> +  (void)alias_ref<int, int&, U<0>::a>(); // expected-note {{here}}
>    (void)func_ref<int, int&, U<1>::a>(); // expected-note {{here}}
>    (void)class_ref<int, int&, U<2>::a>(); // expected-note {{here}}
>  };
>
> +template<int N>
> +void fi() {
> +  (void)alias_ref<int, int&, U<N>::a>(); // expected-note {{here}}
> +  (void)func_ref<int, int&, U<N+1>::a>(); // expected-note {{here}}
> +  (void)class_ref<int, int&, U<N+2>::a>(); // expected-note {{here}}
> +};
> +
>  int main() {
> -  f<int>(); // expected-note 2{{here}}
> +  f<int>();   // NOTE: Non-dependent name uses are type-checked at
> template definition time.
> +  fi<10>();   // expected-note 3{{here}}
>  }
>
>  namespace N {
> @@ -38,3 +44,4 @@ namespace N {
>    }
>    int j = f<int>();
>  }
> +
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140729/0fa5846d/attachment.html>