Let's let the discussion _start_ before assuming it'll be protracted. ;) I don't think it'll kill us to run the next round of testing in C++98 mode. If the patch doesn't look close to acceptance in a couple days, I think it'll make sense to put the then-current version into the google branches while people are agreeing about what to do in the long run.
On Sun, Apr 22, 2012 at 8:14 PM, Ollie Wild <a...@google.com> wrote: > I'd like to get this into the google branches first because this is blocking > testing. > > I fully expect this to result in some protracted discussion before it can be > accepted into trunk. > > Ollie > > > On Sun, Apr 22, 2012 at 10:10 PM, Jeffrey Yasskin <jyass...@google.com> > wrote: >> >> Could you try to get this into mainline instead of just the google >> branches? In http://gcc.gnu.org/PR52538, Jonathan sounded like he'd >> consider accepting it. >> >> On Sun, Apr 22, 2012 at 7:54 PM, Ollie Wild <a...@google.com> wrote: >> > Add new option, -Wreserved-user-defined-literal. >> > >> > This option, which is enabled by default, causes the preprocessor to >> > warn >> > when a string or character literal is followed by a ud-suffix which does >> > not begin with an underscore. According to [lex.ext]p10, this is >> > ill-formed. >> > >> > Also modifies the preprocessor to treat such ill-formed suffixes as >> > separate >> > preprocessing tokens. This is consistent with the Clang front end (see >> > http://llvm.org/viewvc/llvm-project?view=rev&revision=152287), and >> > enables >> > backwards compatibility with code that uses formatting macros from >> > <inttypes.h>, as in the following code block: >> > >> > int main() { >> > int64_t i64 = 123; >> > printf("My int64: %"PRId64"\n", i64); >> > } >> > >> > Google ref b/6377711. >> > >> > 2012-04-22 Ollie Wild <a...@google.com> >> > >> > * gcc/c-family/c-common.c: >> > * gcc/c-family/c-opts.c (c_common_handle_option): >> > * gcc/c-family/c.opt: >> > * gcc/doc/invoke.texi (struct A): >> > * gcc/testsuite/g++.dg/cpp0x/Wreserved-user-defined-literal.C >> > (test): >> > (main): >> > * libcpp/include/cpplib.h (struct cpp_options): >> > * libcpp/init.c (cpp_create_reader): >> > * libcpp/lex.c (lex_raw_string): >> > (lex_string): >> > >> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> > index 1d19251..915dc25 100644 >> > --- a/gcc/c-family/c-common.c >> > +++ b/gcc/c-family/c-common.c >> > @@ -8724,6 +8724,7 @@ static const struct reason_option_codes_t >> > option_codes[] = { >> > {CPP_W_NORMALIZE, OPT_Wnormalized_}, >> > {CPP_W_INVALID_PCH, OPT_Winvalid_pch}, >> > {CPP_W_WARNING_DIRECTIVE, OPT_Wcpp}, >> > + {CPP_W_RESERVED_USER_LITERALS, >> > OPT_Wreserved_user_defined_literal}, >> > {CPP_W_NONE, 0} >> > }; >> > >> > diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c >> > index 5118928..dab6ce5 100644 >> > --- a/gcc/c-family/c-opts.c >> > +++ b/gcc/c-family/c-opts.c >> > @@ -509,6 +509,10 @@ c_common_handle_option (size_t scode, const char >> > *arg, int value, >> > break; >> > } >> > >> > + case OPT_Wreserved_user_defined_literal: >> > + cpp_opts->warn_reserved_user_literals = value; >> > + break; >> > + >> > case OPT_Wreturn_type: >> > warn_return_type = value; >> > break; >> > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >> > index 40ff96c..f610513 100644 >> > --- a/gcc/c-family/c.opt >> > +++ b/gcc/c-family/c.opt >> > @@ -589,6 +589,10 @@ Wreorder >> > C++ ObjC++ Var(warn_reorder) Warning >> > Warn when the compiler reorders code >> > >> > +Wreserved-user-defined-literal >> > +C++ ObjC++ Warning >> > +Warn when a string or character literal is followed by a ud-suffix >> > which does not begin with an underscore. >> > + >> > Wreturn-type >> > C ObjC C++ ObjC++ Var(warn_return_type) Warning >> > Warn whenever a function's return type defaults to \"int\" (C), or >> > about inconsistent return types (C++) >> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> > index 1b61e76..d425079 100644 >> > --- a/gcc/doc/invoke.texi >> > +++ b/gcc/doc/invoke.texi >> > @@ -198,7 +198,7 @@ in the following sections. >> > -fvisibility-ms-compat @gol >> > -Wabi -Wconversion-null -Wctor-dtor-privacy @gol >> > -Wdelete-non-virtual-dtor -Wnarrowing -Wnoexcept @gol >> > --Wnon-virtual-dtor -Wreorder @gol >> > +-Wnon-virtual-dtor -Wreorder -Wreserved-user-defined-literal @gol >> > -Weffc++ -Wstrict-null-sentinel @gol >> > -Wno-non-template-friend -Wold-style-cast @gol >> > -Woverloaded-virtual -Wno-pmf-conversions @gol >> > @@ -2474,6 +2474,30 @@ struct A @{ >> > The compiler will rearrange the member initializers for @samp{i} >> > and @samp{j} to match the declaration order of the members, emitting >> > a warning to that effect. This warning is enabled by @option{-Wall}. >> > + >> > +@item -Wreserved-user-defined-literal @r{(C++ and Objective-C++ only)} >> > +@opindex Wreserved-user-defined-literal >> > +@opindex Wno-reserved-user-defined-literal >> > +Warn when a string or character literal is followed by a ud-suffix >> > which does >> > +not begin with an underscore. As a conforming extension, GCC treats >> > such >> > +suffixes as separate preprocessing tokens in order to maintain >> > backwards >> > +compatibility with code that uses formatting macros from >> > @code{<inttypes.h>}. >> > +For example: >> > + >> > +@smallexample >> > +#define __STDC_FORMAT_MACROS >> > +#include <inttypes.h> >> > +#include <stdio.h> >> > + >> > +int main() @{ >> > + int64_t i64 = 123; >> > + printf("My int64: %"PRId64"\n", i64); >> > +@} >> > +@end smallexample >> > + >> > +In this case, @code{PRId64} is treated as a separate preprocessing >> > token. >> > + >> > +This warning is enabled by default. >> > @end table >> > >> > The following @option{-W@dots{}} options are not affected by >> > @option{-Wall}. >> > diff --git a/gcc/testsuite/g++.dg/cpp0x/Wreserved-user-defined-literal.C >> > b/gcc/testsuite/g++.dg/cpp0x/Wreserved-user-defined-literal.C >> > new file mode 100644 >> > index 0000000..66de5c0 >> > --- /dev/null >> > +++ b/gcc/testsuite/g++.dg/cpp0x/Wreserved-user-defined-literal.C >> > @@ -0,0 +1,29 @@ >> > +// { dg-do run } >> > +// { dg-options "-std=c++0x" } >> > + >> > +// Make sure -Wreserved-user-defined-literal is enabled by default and >> > +// triggers as expected. >> > + >> > +#define BAR "bar" >> > +#define PLUS_ONE + 1 >> > + >> > +#include <cstdint> >> > +#include <cassert> >> > + >> > + >> > +void >> > +test() >> > +{ >> > + char c = '3'PLUS_ONE; // { dg-warning "invalid suffix on >> > literal" } >> > + char s[] = "foo"BAR; // { dg-warning "invalid suffix on literal" } >> > + >> > + assert(c == '4'); >> > + assert(s[3] != '\0'); >> > + assert(s[3] == 'b'); >> > +} >> > + >> > +int >> > +main() >> > +{ >> > + test(); >> > +} >> > diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h >> > index bf59d01..84cacb0 100644 >> > --- a/libcpp/include/cpplib.h >> > +++ b/libcpp/include/cpplib.h >> > @@ -427,6 +427,10 @@ struct cpp_options >> > /* Nonzero for C++ 2011 Standard user-defnied literals. */ >> > unsigned char user_literals; >> > >> > + /* Nonzero means warn when a string or character literal is followed >> > by a >> > + ud-suffix which does not beging with an underscore. */ >> > + unsigned char warn_reserved_user_literals; >> > + >> > /* Holds the name of the target (execution) character set. */ >> > const char *narrow_charset; >> > >> > @@ -906,7 +910,8 @@ enum { >> > CPP_W_CXX_OPERATOR_NAMES, >> > CPP_W_NORMALIZE, >> > CPP_W_INVALID_PCH, >> > - CPP_W_WARNING_DIRECTIVE >> > + CPP_W_WARNING_DIRECTIVE, >> > + CPP_W_RESERVED_USER_LITERALS >> > }; >> > >> > /* Output a diagnostic of some kind. */ >> > diff --git a/libcpp/init.c b/libcpp/init.c >> > index 5fa82ca..2688699 100644 >> > --- a/libcpp/init.c >> > +++ b/libcpp/init.c >> > @@ -175,6 +175,7 @@ cpp_create_reader (enum c_lang lang, hash_table >> > *table, >> > CPP_OPTION (pfile, warn_variadic_macros) = 1; >> > CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1; >> > CPP_OPTION (pfile, warn_normalize) = normalized_C; >> > + CPP_OPTION (pfile, warn_reserved_user_literals) = 1; >> > >> > /* Default CPP arithmetic to something sensible for the host for the >> > benefit of dumb users like fix-header. */ >> > diff --git a/libcpp/lex.c b/libcpp/lex.c >> > index 0ad9660..ba8ed9e 100644 >> > --- a/libcpp/lex.c >> > +++ b/libcpp/lex.c >> > @@ -1491,14 +1491,30 @@ lex_raw_string (cpp_reader *pfile, cpp_token >> > *token, const uchar *base, >> > >> > if (CPP_OPTION (pfile, user_literals)) >> > { >> > + /* According to C++11 [lex.ext]p10, a ud-suffix not starting with >> > an >> > + underscore is ill-formed. Since this breaks programs using >> > macros >> > + from inttypes.h, we generate a warning and treat the ud-suffix >> > as a >> > + separate preprocessing token. This approach is under >> > discussion by >> > + the standards committee, and has been adopted as a conforming >> > + extension by other front ends such as clang. */ >> > + if (ISALPHA(*cur)) >> > + { >> > + // Raise a warning, but do not consume subsequent tokens. >> > + if (CPP_OPTION (pfile, warn_reserved_user_literals)) >> > + cpp_warning_with_line (pfile, CPP_W_RESERVED_USER_LITERALS, >> > + token->src_loc, 0, >> > + "invalid suffix on literal; C++11 >> > requires " >> > + "a space between literal and >> > identifier"); >> > + } >> > /* Grab user defined literal suffix. */ >> > - if (ISIDST (*cur)) >> > + else if (*cur == '_') >> > { >> > type = cpp_userdef_string_add_type (type); >> > ++cur; >> > + >> > + while (ISIDNUM (*cur)) >> > + ++cur; >> > } >> > - while (ISIDNUM (*cur)) >> > - ++cur; >> > } >> > >> > pfile->buffer->cur = cur; >> > @@ -1606,15 +1622,31 @@ lex_string (cpp_reader *pfile, cpp_token *token, >> > const uchar *base) >> > >> > if (CPP_OPTION (pfile, user_literals)) >> > { >> > + /* According to C++11 [lex.ext]p10, a ud-suffix not starting with >> > an >> > + underscore is ill-formed. Since this breaks programs using >> > macros >> > + from inttypes.h, we generate a warning and treat the ud-suffix >> > as a >> > + separate preprocessing token. This approach is under >> > discussion by >> > + the standards committee, and has been adopted as a conforming >> > + extension by other front ends such as clang. */ >> > + if (ISALPHA(*cur)) >> > + { >> > + // Raise a warning, but do not consume subsequent tokens. >> > + if (CPP_OPTION (pfile, warn_reserved_user_literals)) >> > + cpp_warning_with_line (pfile, CPP_W_RESERVED_USER_LITERALS, >> > + token->src_loc, 0, >> > + "invalid suffix on literal; C++11 >> > requires " >> > + "a space between literal and >> > identifier"); >> > + } >> > /* Grab user defined literal suffix. */ >> > - if (ISIDST (*cur)) >> > + else if (*cur == '_') >> > { >> > type = cpp_userdef_char_add_type (type); >> > type = cpp_userdef_string_add_type (type); >> > ++cur; >> > + >> > + while (ISIDNUM (*cur)) >> > + ++cur; >> > } >> > - while (ISIDNUM (*cur)) >> > - ++cur; >> > } >> > >> > pfile->buffer->cur = cur; >> > >> > -- >> > This patch is available for review at >> > http://codereview.appspot.com/6104051 > >
