Re: Patch for constexpr variable templates

[Jason Merrill <jason at redhat dot com>]

First of all, thanks a lot for taking this on!  A few nitpicks:

On 07/21/2014 11:06 PM, Braden Obrzut wrote:
>  grokvardecl (tree type,
>              tree name,
> +                tree orig_declarator,
>              const cp_decl_specifier_seq *declspecs,
>              int initialized,
>              int constp,
> +                int template_count,

Indentation mismatch.

> +  if (orig_declarator && (processing_template_decl
> +      || TREE_CODE (orig_declarator) == TEMPLATE_ID_EXPR))

The indentation is wrong, since the || is part of the inner 
parenthesized expression.

> +      /* Variable templates will need to have the class context.  */
> +      if (VAR_P (value))
> +        DECL_CONTEXT (value) = current_class_type;

Why isn't this covered by grokvardecl?

> -  if (!is_overloaded_fn (fns))
> +  bool var_templ = variable_template_p (fns);
> +  if (!is_overloaded_fn (fns) && !var_templ)
>      {
>        error ("%qD is not a function template", fns);

I think here we should check whether 'fns' and 'decl' are both variables 
or both functions so that we can give a better diagnostic.

> +      if (cxx_dialect < cxx1y)
> +        permerror (DECL_SOURCE_LOCATION (decl),
> +                   "%qD is not a static data member of a class template", decl);

It's customary to use pedwarn and mention the relevant -std= flag.

> +  if ((specialization || member_specialization)
> +         /* Variable templates don't apply.  */
> +         && (TREE_CODE (TREE_TYPE (decl)) == FUNCTION_TYPE
> +             || TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE))

Indentation mismatch.

> +      // Namespace scope variable templates should have a template header.

"Namespace-scope"

> +         if (!variable_template_p (tmpl)
> +                 && DECL_STATIC_FUNCTION_P (tmpl)
...
> +         if (!variable_template_p (tmpl))
> +           copy_default_args_to_explicit_spec (decl);

I'd prefer to check for a function template here rather than "not a 
variable template".

> +      else if (VAR_P (decl))
> +        {
> +          if (!DECL_DECLARED_CONSTEXPR_P (decl))
> +            error ("template declaration of non-constexpr variable %qD", decl);
> +        }

As Ed and Andrew pointed out, non-constexpr variable templates are fine.

> +  bool var_templ = DECL_TEMPLATE_INFO (decl)
> +                   && variable_template_p (DECL_TI_TEMPLATE (decl));

An expression on multiple lines should be parenthesized to preserve 
indentation.

> +         bool enter_context = CLASS_TYPE_P (DECL_CONTEXT (d));

You can use DECL_CLASS_SCOPE_P.

> +  return instantiate_template (TREE_OPERAND (var, 0), TREE_OPERAND (var, 1), tf_error);

Line too long.

Jason