Re: [C++-11] User defined literals

[Jason Merrill <jason@redhat.com>]

Since we're starting to come up on the end of stage 1, I'll go ahead and 
review the current patch even though it isn't finished yet.  Thanks for 
all your work on this, it definitely seems to be coming together.

On 09/13/2011 10:35 AM, Ed Smith-Rowland wrote:
> +#define CPP_N_USERDEF  0x1000000 /* C++0x user-defned literal.  */

"defined"

> -extern unsigned cpp_classify_number (cpp_reader *, const cpp_token *);
> +extern unsigned cpp_classify_number (cpp_reader *, const cpp_token *,
> +                                    char **ud_suffix);
> +
> +/* Return the classification flags for a float suffix.  */
> +unsigned int cpp_interpret_float_suffix (const char *s, size_t len);
> +
> +/* Return the classification flags for an int suffix.  */
> +unsigned int cpp_interpret_int_suffix (const char *s, size_t len);
>
>  /* Evaluate a token classified as category CPP_N_INTEGER.  */
>  extern cpp_num cpp_interpret_integer (cpp_reader *, const cpp_token *,

Let's follow the pattern of the existing declarations by adding explicit 
'extern' and not naming the parameters.

> @@ -327,6 +327,10 @@ pp_cxx_constant (cxx_pretty_printer *pp, tree t)
> +    case USERDEF_LITERAL:
> +      pp_c_constant (pp_c_base (pp), USERDEF_LITERAL_VALUE (t));
> +      break;
> @@ -1755,6 +1757,11 @@ dump_expr (tree t, int flags)
> +    case USERDEF_LITERAL:
> +      pp_constant (cxx_pp, USERDEF_LITERAL_VALUE (t));
> +      dump_decl (USERDEF_LITERAL_SUFFIX_ID (t), flags);
> +      break;

These should be the same--or just call pp_cxx_constant from dump_expr.

> +/* Extract the suffix from a user-defined literal string or char.  */
> +void
> +cpp_get_userdef_suffix (cpp_string string, char delim, char *suffix)
> +{
> +  unsigned int len = string.len;
> +  const char *text = (const char *)string.text;
> +  int i;
> +  for (i = len; i > 0; --i)
> +    {
> +      if (text[i - 1] == delim)
> +       break;
> +    }
> +  strncpy (suffix, text + i, len - i);
> +  suffix[len - i] = '\0';
> +}

Since you're just going to be passing the pointer to get_identifier 
anyway, we don't need to copy the suffix into the pointer.  Just return 
a pointer to the start of the suffix.

> +             if (ud_suffix)
> +               *ud_suffix = xstrdup ((const char *)str);

Similarly here.

> +             const char *curr_suffix = NULL;
> +             suffix_id = USERDEF_LITERAL_SUFFIX_ID (tok->u.value);
> +             curr_suffix = IDENTIFIER_POINTER (suffix_id);
> +             if (have_suffix_p == 0)
> +               {
> +                 suffix = xstrdup (curr_suffix);
> +                 have_suffix_p = 1;
> +               }
> +             else if (have_suffix_p == 1 && strcmp (suffix, curr_suffix) != 0)

And here, you can compare the identifiers with ==, since identifiers are 
unique.

> +                 error ("inconsistent user-defined literal suffixes");

Let's print the suffixes involved.

> +             error ("%qD has illegal argument list", decl);

Use "invalid" rather than "illegal".

> +         suffix = UDLIT_OP_SUFFIX (DECL_NAME (decl));
> +         if (long_long_unsigned_p)
> +           {
> +             if (cpp_interpret_int_suffix (suffix, strlen (suffix)))
> +               warning (0, "integer suffix shadowed by implementation");
> +           }
> +         else if (long_double_p)
> +           {
> +             if (cpp_interpret_float_suffix (suffix, strlen (suffix)))
> +               warning (0, "floating point suffix"
> +                           " shadowed by implementation");
> +           }

Let's print the suffix as part of the warning.

> +  if (dname
> +      && TREE_CODE (dname) == IDENTIFIER_NODE
> +      && UDLIT_OPER_P (dname)
> +      && innermost_code != cdk_function
> +      && ! (ctype && !declspecs->any_specifiers_p))

I think we can drop the last check, as it only applies to constructors.

> +/* User-defined literal operator.
> +DEF_SIMPLE_OPERATOR ("\"\"", USERDEF_LITERAL_EXPR, "ud", 1)  */

Are you planning to use this for something?

> +/* Parse a user-defined char constant.  Returns a call to a user-defined
> +   literal operator taking the character as an argument.  */
> +static tree
> +cp_parser_userdef_char_literal (cp_parser *parser)

There should be a blank line after the comment here and before the other 
new functions, at least in gcc/.

> +make_numeric_string(tree value)

This function tries to recreate a printed form for a numeric constant, 
but that's not right; we need the actual source characters, which are 
likely to be different, especially for floating-point numbers.  So we 
need to get back to the actual spelling of the token here.

I notice that later in cp_parser_userdef_numeric_literal when calling 
the variadic template version you assume that you have the spelling of 
the token, which is indeed what we want.

> +check_literal_operator_args(const_tree decl,

Space before (.

> +      tree const_char_ptr_type_node
> +          = build_pointer_type(build_type_variant(char_type_node, 1, 0));
> +      tree const_wchar_ptr_type_node
> +          = build_pointer_type(build_type_variant(wchar_type_node, 1, 0));
> +      tree const_char16_ptr_type_node
> +          = build_pointer_type(build_type_variant(char16_type_node, 1, 0));
> +      tree const_char32_ptr_type_node
> +          = build_pointer_type(build_type_variant(char32_type_node, 1, 0));

It seems to me that rather than build these up to compare against, it it 
would be faster to check for POINTER_TYPE, then check that the 
pointed-to type is CP_TYPE_CONST_NON_VOLATILE_P, and then compare the 
TYPE_MAIN_VARIANT to char_type_node, etc.

> +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-operator-neg.C
> @@ -0,0 +1,16 @@
> +// { dg-options "-std=c++0x" }
> +
> +//  Both of these can't have *both* of these.
> +
> +int
> +operator"" _abc(const char*)
> +  {
> +    return 42;
> +  }
> +
> +template<char...>
> +  int
> +  operator"" _abc()
> +  {
> +    return 13;
> +  }

Shouldn't this have a dg-error tag somewhere?

Jason