public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C/C++] PR 13358 long long and C++ do not mix well
@ 2008-08-30  1:11 Manuel López-Ibáñez
  2008-08-30  2:15 ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2008-08-30  1:11 UTC (permalink / raw)
  To: Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

CPP and the FE were using different semantics. invoke.texi said that
Wlong-long was the default. This was not correct. It was the default
for CPP but -Wno-long-long was the default for the FE. Now there is
only one semantic: the one from the FE.

However, we were inhibiting warning even if the user requested it
through Wlong-long. Now explicit Wlong-long (or Wno-long-long)
overrides everything else, except in system headers.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu with
--enable-languages=all,ada with --target_board=\{-m32,-m64\}

OK for trunk?

2008-08-25  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

	PR 13358
	* doc/invoke.texi (-Wlong-long): Update description.
	* c-opts.c (sanitize_cpp_opts): Synchronize cpp's warn_long_long
	and front-end warn_long_long. Wlong-long only depends on other
	flags if it is uninitialized.
	* c-lex (interpret_integer): Only warn if there was no previous
	overflow and -Wlong-long is enabled.
	* c-decl.c (declspecs_add_type): Drop redundant flags.
	* c.opt (Wlong-long): Init to -1.
	* c-parser.c (disable_extension_diagnostics): warn_long_long is
	the same for CPP and FE.
	(restore_extension_diagnostics): Likewise.
cp/
	* parser.c (cp_parser_check_decl_spec): Drop redundant flags.
testsuite/
	* g++.dg/warn/pr13358.C: New.
	* g++.dg/warn/pr13358-2.C: New.
	* g++.dg/warn/pr13358-3.C: New.	
	* gcc.dg/wtr-int-type-1.c: Use two dg-warning to match two
	messages. Test for "long long" in system headers.

[-- Attachment #2: fix-pr13358-try2.diff --]
[-- Type: text/plain, Size: 10277 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 139674)
+++ gcc/doc/invoke.texi	(working copy)
@@ -4028,14 +4028,13 @@ Warn if a precompiled header (@pxref{Pre
 the search path but can't be used.
 
 @item -Wlong-long
 @opindex Wlong-long
 @opindex Wno-long-long
-Warn if @samp{long long} type is used.  This is default.  To inhibit
-the warning messages, use @option{-Wno-long-long}.  Flags
-@option{-Wlong-long} and @option{-Wno-long-long} are taken into account
-only when @option{-pedantic} flag is used.
+Warn if @samp{long long} type is used.  This is enabled by either
+@option{-pedantic} or @option{-Wtraditional} in ISO C90 and C++98
+modes.  To inhibit the warning messages, use @option{-Wno-long-long}.
 
 @item -Wvariadic-macros
 @opindex Wvariadic-macros
 @opindex Wno-variadic-macros
 Warn if variadic macros are used in pedantic ISO C90 mode, or the GNU
Index: gcc/c-lex.c
===================================================================
--- gcc/c-lex.c	(revision 139674)
+++ gcc/c-lex.c	(working copy)
@@ -580,17 +580,19 @@ interpret_integer (const cpp_token *toke
     /* cpplib has already issued a warning for overflow.  */
     type = ((flags & CPP_N_UNSIGNED)
 	    ? widest_unsigned_literal_type_node
 	    : widest_integer_literal_type_node);
   else
-    type = integer_types[itk];
-
-  if (itk > itk_unsigned_long
-      && (flags & CPP_N_WIDTH) != CPP_N_LARGE
-      && !in_system_header && !flag_isoc99)
-    pedwarn (input_location, 0, "integer constant is too large for %qs type",
-	     (flags & CPP_N_UNSIGNED) ? "unsigned long" : "long");
+    {
+      type = integer_types[itk];
+      if (itk > itk_unsigned_long
+	  && (flags & CPP_N_WIDTH) != CPP_N_LARGE)
+	pedwarn (input_location, OPT_Wlong_long,
+		 (flags & CPP_N_UNSIGNED) 
+		 ? "integer constant is too large for %<unsigned long%> type"
+		 : "integer constant is too large for %<long%> type");
+    }
 
   value = build_int_cst_wide (type, integer.low, integer.high);
 
   /* Convert imaginary to a complex type.  */
   if (flags & CPP_N_IMAGINARY)
Index: gcc/testsuite/gcc.dg/wtr-int-type-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wtr-int-type-1.c	(revision 139674)
+++ gcc/testsuite/gcc.dg/wtr-int-type-1.c	(working copy)
@@ -23,13 +23,21 @@ testfunc ()
      we can pretend it worked the way it does in C99.]  */
   i = 9223372036854775807;
 
   /* But this one should, since it doesn't fit in long (long), but
      does fit in unsigned long (long).  */
-  i = 18446744073709551615; /* { dg-warning "decimal constant|unsigned" "decimal constant" } */
-  
+  i = 18446744073709551615; /* { dg-warning "integer constant is so large that it is unsigned" "decimal constant" } */
+  /* { dg-warning "this decimal constant would be unsigned in ISO C90" "decimal constant" { target *-*-* } 28 } */
+
 # 29 "sys-header.h" 3
+}
+
+void
+testfunc2( ) 
+{ 
+  long long i;
+
 /* We are in system headers now, no -Wtraditional warnings should issue.  */
 
   i = 0x80000000;
   i = 0xFFFFFFFF;
   i = 037777777777;
@@ -39,5 +47,6 @@ testfunc ()
   i = 01777777777777777777777;
   
   i = 9223372036854775807;
   i = 18446744073709551615;
 }
+
Index: gcc/testsuite/g++.dg/warn/pr13358-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
@@ -0,0 +1,15 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use
+// { dg-do compile }
+// { dg-require-effective-target ilp32 }
+// { dg-options "-std=c++98 -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; // { dg-error "error: ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we error with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86; // { dg-error "error: integer constant is too large for 'long' type" }
+  x2 = 1956772631100509574; // { dg-error "error: integer constant is too large for 'long' type" }
+  x3 = 0154476645345674746606; // { dg-error "error: integer constant is too large for 'long' type" }
+}
Index: gcc/testsuite/g++.dg/warn/pr13358-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
@@ -0,0 +1,15 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++0x -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
Index: gcc/testsuite/g++.dg/warn/pr13358.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
@@ -0,0 +1,15 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++98 -Wno-long-long -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 139674)
+++ gcc/cp/parser.c	(working copy)
@@ -2158,12 +2158,11 @@ cp_parser_check_decl_spec (cp_decl_speci
       /* The "long" specifier is a special case because of "long long".  */
       if (ds == ds_long)
 	{
 	  if (count > 2)
 	    error ("%H%<long long long%> is too long for GCC", &location);
-	  else if (pedantic && !in_system_header && warn_long_long
-                   && cxx_dialect == cxx98)
+	  else 
 	    pedwarn (location, OPT_Wlong_long, 
 		     "ISO C++ 1998 does not support %<long long%>");
 	}
       else if (count > 1)
 	{
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 139674)
+++ gcc/c-decl.c	(working copy)
@@ -7189,12 +7189,11 @@ declspecs_add_type (struct c_declspecs *
 		    {
 		      error ("both %<long long%> and %<double%> in "
 			     "declaration specifiers");
 		      break;
 		    }
-		  if (pedantic && !flag_isoc99 && !in_system_header)
-		    pedwarn (input_location, OPT_Wlong_long, "ISO C90 does not support %<long long%>");
+		  pedwarn (input_location, OPT_Wlong_long, "ISO C90 does not support %<long long%>");
 		  specs->long_long_p = 1;
 		  break;
 		}
 	      if (specs->short_p)
 		error ("both %<long%> and %<short%> in "
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 139674)
+++ gcc/c.opt	(working copy)
@@ -275,11 +275,11 @@ Warn about invalid uses of the \"offseto
 Winvalid-pch
 C ObjC C++ ObjC++ Warning
 Warn about PCH files that are found but not used
 
 Wlong-long
-C ObjC C++ ObjC++ Var(warn_long_long) Init(1) Warning
+C ObjC C++ ObjC++ Var(warn_long_long) Init(-1) Warning
 Do not warn about using \"long long\" when -pedantic
 
 Wmain
 C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning
 Warn about suspicious declarations of \"main\"
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 139674)
+++ gcc/c-opts.c	(working copy)
@@ -1390,18 +1390,19 @@ sanitize_cpp_opts (void)
     error ("-MG may only be used with -M or -MM");
 
   cpp_opts->unsigned_char = !flag_signed_char;
   cpp_opts->stdc_0_in_system_headers = STDC_0_IN_SYSTEM_HEADERS;
 
-  /* We want -Wno-long-long to override -pedantic -std=non-c99
-     and/or -Wtraditional, whatever the ordering.  */
-  cpp_opts->warn_long_long
-    = warn_long_long && ((pedantic
-			  && (c_dialect_cxx ()
-			      ? cxx_dialect == cxx98
-			      : !flag_isoc99))
-                         || warn_traditional);
+  /* Wlong-long is disabled by default. It is enabled by:
+      [-pedantic | -Wtraditional] -std=[gnu|c]++98 ; or
+      [-pedantic | -Wtraditional] -std=non-c99 . 
+
+      Either -Wlong-long or -Wno-long-long override any other settings.  */
+  if (warn_long_long == -1)
+    warn_long_long = ((pedantic || warn_traditional)
+		      && (c_dialect_cxx () ? cxx_dialect == cxx98 : !flag_isoc99));
+  cpp_opts->warn_long_long = warn_long_long;
 
   /* Similarly with -Wno-variadic-macros.  No check for c99 here, since
      this also turns off warnings about GCCs extension.  */
   cpp_opts->warn_variadic_macros
     = warn_variadic_macros && (pedantic || warn_traditional);
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 139674)
+++ gcc/c-parser.c	(working copy)
@@ -815,18 +815,16 @@ disable_extension_diagnostics (void)
 {
   int ret = (pedantic
 	     | (warn_pointer_arith << 1)
 	     | (warn_traditional << 2)
 	     | (flag_iso << 3)
-	     | (warn_long_long << 4)
-	     | (cpp_opts->warn_long_long << 5));
+	     | (warn_long_long << 4));
   cpp_opts->pedantic = pedantic = 0;
   warn_pointer_arith = 0;
   cpp_opts->warn_traditional = warn_traditional = 0;
   flag_iso = 0;
-  warn_long_long = 0;
-  cpp_opts->warn_long_long = 0;
+  cpp_opts->warn_long_long = warn_long_long = 0;
   return ret;
 }
 
 /* Restore the warning flags which are controlled by __extension__.
    FLAGS is the return value from disable_extension_diagnostics.  */
@@ -836,12 +834,11 @@ restore_extension_diagnostics (int flags
 {
   cpp_opts->pedantic = pedantic = flags & 1;
   warn_pointer_arith = (flags >> 1) & 1;
   cpp_opts->warn_traditional = warn_traditional = (flags >> 2) & 1;
   flag_iso = (flags >> 3) & 1;
-  warn_long_long = (flags >> 4) & 1;
-  cpp_opts->warn_long_long = (flags >> 5) & 1;
+  cpp_opts->warn_long_long = warn_long_long = (flags >> 4) & 1;
 }
 
 /* Possibly kinds of declarator to parse.  */
 typedef enum c_dtr_syn {
   /* A normal declarator with an identifier.  */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2008-08-30  1:11 [C/C++] PR 13358 long long and C++ do not mix well Manuel López-Ibáñez
@ 2008-08-30  2:15 ` Joseph S. Myers
  2008-08-30  9:52   ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Joseph S. Myers @ 2008-08-30  2:15 UTC (permalink / raw)
  To: Manuel López-Ibáñez; +Cc: Gcc Patch List

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 714 bytes --]

On Fri, 29 Aug 2008, Manuel López-Ibáñez wrote:

> CPP and the FE were using different semantics. invoke.texi said that
> Wlong-long was the default. This was not correct. It was the default
> for CPP but -Wno-long-long was the default for the FE. Now there is
> only one semantic: the one from the FE.
> 
> However, we were inhibiting warning even if the user requested it
> through Wlong-long. Now explicit Wlong-long (or Wno-long-long)
> overrides everything else, except in system headers.

If you allow -Wlong-long to have an effect in C99 mode, -std=c99 
-pedantic-errors -Wlong-long should give the diagnostics as warnings not 
errors.  Does it?

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2008-08-30  2:15 ` Joseph S. Myers
@ 2008-08-30  9:52   ` Manuel López-Ibáñez
  2008-10-23  2:42     ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2008-08-30  9:52 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Gcc Patch List

2008/8/29 Joseph S. Myers <joseph@codesourcery.com>:
> On Fri, 29 Aug 2008, Manuel López-Ibáñez wrote:
>
>> CPP and the FE were using different semantics. invoke.texi said that
>> Wlong-long was the default. This was not correct. It was the default
>> for CPP but -Wno-long-long was the default for the FE. Now there is
>> only one semantic: the one from the FE.
>>
>> However, we were inhibiting warning even if the user requested it
>> through Wlong-long. Now explicit Wlong-long (or Wno-long-long)
>> overrides everything else, except in system headers.
>
> If you allow -Wlong-long to have an effect in C99 mode, -std=c99
> -pedantic-errors -Wlong-long should give the diagnostics as warnings not
> errors.  Does it?

No but I can make it do that just using pedwarn_c90 instead of pedwarn
and add a testcase. I'll do the same for c++98. I''ll need to add an
equivalent of pedwarn_c90 to C++ and CPP. In C++ is probably easy.

Is there a way to tell in CPP if we are being called from the C++ FE?
and which standard? I cannot find anything in the sources apart from
CPP_OPTION (pfile, c99).

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2008-08-30  9:52   ` Manuel López-Ibáñez
@ 2008-10-23  2:42     ` Manuel López-Ibáñez
  2009-04-10 19:12       ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2008-10-23  2:42 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

I think the following patch addresses all requirements and in the
process it fixes some small deficiencies of the current status, such
as talking about C99 when invoked from C++.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu with
--enable-languages=all,obj-c++ --enable-decimal-float and
--target_board=\{-m32,-m64\}

OK for trunk?

2008-10-23  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

	PR 13358
	* doc/invoke.texi (-Wlong-long): Update description.
	* c-lex (interpret_integer): Only warn if there was no previous
	overflow and -Wlong-long is enabled.
	* c-decl.c (declspecs_add_type): Drop redundant flags.
	* c.opt (Wlong-long): Init to -1.
	* c-opts.c (sanitize_cpp_opts): Synchronize cpp's warn_long_long
	and front-end warn_long_long. Wlong-long only depends on other
	flags if it is uninitialized.
	* c-parser.c (disable_extension_diagnostics): warn_long_long is
	the same for CPP and FE.
	(restore_extension_diagnostics): Likewise.
libcpp/
	* init.c (cpp_create_reader): Wlong_long is disabled by default.
	* expr.c (cpp_classify_number): Give different messages for C and
	C++ front-ends.
cp/
	* parser.c (cp_parser_check_decl_spec): Drop redundant flags.
	* error.c (pedwarn_cxx98): New.
	* cp-tree.h (pedwarn_cxx98): Declare.
	
testsuite/
	* gcc.dg/wtr-int-type-1.c: Use two dg-warning to match two
	messages. Test for "long long" in system headers.
	* gcc.dg/c99-longlong-2.c: New.
	* g++.dg/warn/pr13358.C: New.
	* g++.dg/warn/pr13358-2.C: New.
	* g++.dg/warn/pr13358-3.C: New.
	* g++.dg/warn/pr13358-4.C: New.



2008/8/29 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> 2008/8/29 Joseph S. Myers <joseph@codesourcery.com>:
>> On Fri, 29 Aug 2008, Manuel López-Ibáñez wrote:
>>
>>> CPP and the FE were using different semantics. invoke.texi said that
>>> Wlong-long was the default. This was not correct. It was the default
>>> for CPP but -Wno-long-long was the default for the FE. Now there is
>>> only one semantic: the one from the FE.
>>>
>>> However, we were inhibiting warning even if the user requested it
>>> through Wlong-long. Now explicit Wlong-long (or Wno-long-long)
>>> overrides everything else, except in system headers.
>>
>> If you allow -Wlong-long to have an effect in C99 mode, -std=c99
>> -pedantic-errors -Wlong-long should give the diagnostics as warnings not
>> errors.  Does it?
>
> No but I can make it do that just using pedwarn_c90 instead of pedwarn
> and add a testcase. I'll do the same for c++98. I''ll need to add an
> equivalent of pedwarn_c90 to C++ and CPP. In C++ is probably easy.
>
> Is there a way to tell in CPP if we are being called from the C++ FE?
> and which standard? I cannot find anything in the sources apart from
> CPP_OPTION (pfile, c99).
>
> Cheers,
>
> Manuel.
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-pr13358-try2.diff --]
[-- Type: text/x-diff; name=fix-pr13358-try2.diff, Size: 16593 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 141265)
+++ gcc/doc/invoke.texi	(working copy)
@@ -4059,14 +4059,13 @@ Warn if a precompiled header (@pxref{Pre
 the search path but can't be used.
 
 @item -Wlong-long
 @opindex Wlong-long
 @opindex Wno-long-long
-Warn if @samp{long long} type is used.  This is default.  To inhibit
-the warning messages, use @option{-Wno-long-long}.  Flags
-@option{-Wlong-long} and @option{-Wno-long-long} are taken into account
-only when @option{-pedantic} flag is used.
+Warn if @samp{long long} type is used.  This is enabled by either
+@option{-pedantic} or @option{-Wtraditional} in ISO C90 and C++98
+modes.  To inhibit the warning messages, use @option{-Wno-long-long}.
 
 @item -Wvariadic-macros
 @opindex Wvariadic-macros
 @opindex Wno-variadic-macros
 Warn if variadic macros are used in pedantic ISO C90 mode, or the GNU
Index: gcc/c-lex.c
===================================================================
--- gcc/c-lex.c	(revision 141265)
+++ gcc/c-lex.c	(working copy)
@@ -580,17 +580,22 @@ interpret_integer (const cpp_token *toke
     /* cpplib has already issued a warning for overflow.  */
     type = ((flags & CPP_N_UNSIGNED)
 	    ? widest_unsigned_literal_type_node
 	    : widest_integer_literal_type_node);
   else
-    type = integer_types[itk];
-
-  if (itk > itk_unsigned_long
-      && (flags & CPP_N_WIDTH) != CPP_N_LARGE
-      && !in_system_header && !flag_isoc99)
-    pedwarn (input_location, 0, "integer constant is too large for %qs type",
-	     (flags & CPP_N_UNSIGNED) ? "unsigned long" : "long");
+    {
+      type = integer_types[itk];
+      if (itk > itk_unsigned_long
+	  && (flags & CPP_N_WIDTH) != CPP_N_LARGE)
+	emit_diagnostic 
+	  ((c_dialect_cxx () ? cxx_dialect == cxx98 : !flag_isoc99)
+	   ? DK_PEDWARN : DK_WARNING,
+	   input_location, OPT_Wlong_long,
+	   (flags & CPP_N_UNSIGNED) 
+	   ? "integer constant is too large for %<unsigned long%> type"
+	   : "integer constant is too large for %<long%> type");
+    }
 
   value = build_int_cst_wide (type, integer.low, integer.high);
 
   /* Convert imaginary to a complex type.  */
   if (flags & CPP_N_IMAGINARY)
Index: gcc/testsuite/gcc.dg/wtr-int-type-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wtr-int-type-1.c	(revision 141265)
+++ gcc/testsuite/gcc.dg/wtr-int-type-1.c	(working copy)
@@ -23,13 +23,21 @@ testfunc ()
      we can pretend it worked the way it does in C99.]  */
   i = 9223372036854775807;
 
   /* But this one should, since it doesn't fit in long (long), but
      does fit in unsigned long (long).  */
-  i = 18446744073709551615; /* { dg-warning "decimal constant|unsigned" "decimal constant" } */
-  
+  i = 18446744073709551615; /* { dg-warning "integer constant is so large that it is unsigned" "decimal constant" } */
+  /* { dg-warning "this decimal constant would be unsigned in ISO C90" "decimal constant" { target *-*-* } 28 } */
+
 # 29 "sys-header.h" 3
+}
+
+void
+testfunc2( ) 
+{ 
+  long long i;
+
 /* We are in system headers now, no -Wtraditional warnings should issue.  */
 
   i = 0x80000000;
   i = 0xFFFFFFFF;
   i = 037777777777;
@@ -39,5 +47,6 @@ testfunc ()
   i = 01777777777777777777777;
   
   i = 9223372036854775807;
   i = 18446744073709551615;
 }
+
Index: gcc/testsuite/gcc.dg/c99-longlong-2.c
===================================================================
--- gcc/testsuite/gcc.dg/c99-longlong-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/c99-longlong-2.c	(revision 0)
@@ -0,0 +1,6 @@
+/* Test for long long: if explicit Wlong-long, in C99 only warn, not
+   pedwarn.  */
+/* { dg-do compile } */
+/* { dg-options "-std=iso9899:1999 -pedantic-errors -Wlong-long" } */
+
+long long foo; /* { dg-warning "long long" } */
Index: gcc/testsuite/g++.dg/warn/pr13358-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus }
+// { dg-options "-std=c++98 -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; // { dg-error "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we error with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x2 = 1956772631100509574; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x3 = 0154476645345674746606; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; // { dg-error "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we error with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL; // { dg-error "long long" }
+  x2 = 1956772631100509574LL; // { dg-error "long long" }
+  x3 = 0154476645345674746606LL; // { dg-error "long long" }
+}
Index: gcc/testsuite/g++.dg/warn/pr13358-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++0x -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL;
+  x2 = 1956772631100509574LL;
+  x3 = 0154476645345674746606LL;
+}
Index: gcc/testsuite/g++.dg/warn/pr13358-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-4.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++0x -pedantic-errors -Wlong-long" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; // { dg-warning "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we warn with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x2 = 1956772631100509574; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x3 = 0154476645345674746606; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; // { dg-warning "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we warn with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL; // { dg-warning "long long" }
+  x2 = 1956772631100509574LL; // { dg-warning "long long" }
+  x3 = 0154476645345674746606LL; // { dg-warning "long long" }
+}
Index: gcc/testsuite/g++.dg/warn/pr13358.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++98 -Wno-long-long -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL;
+  x2 = 1956772631100509574LL;
+  x3 = 0154476645345674746606LL;
+}
Index: gcc/cp/error.c
===================================================================
--- gcc/cp/error.c	(revision 141265)
+++ gcc/cp/error.c	(working copy)
@@ -2704,5 +2704,24 @@ maybe_warn_cpp0x (const char* str)
 void
 maybe_warn_variadic_templates (void)
 {
   maybe_warn_cpp0x ("variadic templates");
 }
+
+
+/* Issue an ISO C++98 pedantic warning MSGID.  This function is supposed to
+   be used for matters that are allowed in ISO C++0x but not supported in
+   ISO C++98, thus we explicitly don't pedwarn when ISO c++0x is specified.  */
+
+bool
+pedwarn_cxx98 (location_t location, int opt, const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
+		       (cxx_dialect == cxx98) ? DK_PEDWARN : DK_WARNING);
+  diagnostic.option_index = opt;
+  va_end (ap);
+  return report_diagnostic (&diagnostic);
+}
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 141265)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -4432,10 +4432,11 @@ extern const char *lang_decl_name		(tree
 extern const char *language_to_string		(enum languages);
 extern const char *class_key_or_enum_as_string	(tree);
 extern void print_instantiation_context		(void);
 extern void maybe_warn_variadic_templates       (void);
 extern void maybe_warn_cpp0x			(const char *);
+extern bool pedwarn_cxx98                       (location_t, int, const char *, ...) ATTRIBUTE_GCC_CXXDIAG(3,4);
 
 /* in except.c */
 extern void init_exception_processing		(void);
 extern tree expand_start_catch_block		(tree);
 extern void expand_end_catch_block		(void);
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 141265)
+++ gcc/cp/parser.c	(working copy)
@@ -2162,14 +2162,13 @@ cp_parser_check_decl_spec (cp_decl_speci
       /* The "long" specifier is a special case because of "long long".  */
       if (ds == ds_long)
 	{
 	  if (count > 2)
 	    error ("%H%<long long long%> is too long for GCC", &location);
-	  else if (pedantic && !in_system_header && warn_long_long
-                   && cxx_dialect == cxx98)
-	    pedwarn (location, OPT_Wlong_long, 
-		     "ISO C++ 1998 does not support %<long long%>");
+	  else 
+	    pedwarn_cxx98 (location, OPT_Wlong_long, 
+			   "ISO C++ 1998 does not support %<long long%>");
 	}
       else if (count > 1)
 	{
 	  static const char *const decl_spec_names[] = {
 	    "signed",
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 141265)
+++ gcc/c-decl.c	(working copy)
@@ -7214,12 +7214,12 @@ declspecs_add_type (struct c_declspecs *
 		    {
 		      error ("both %<long long%> and %<double%> in "
 			     "declaration specifiers");
 		      break;
 		    }
-		  if (pedantic && !flag_isoc99 && !in_system_header)
-		    pedwarn (input_location, OPT_Wlong_long, "ISO C90 does not support %<long long%>");
+		  pedwarn_c90 (input_location, OPT_Wlong_long, 
+			       "ISO C90 does not support %<long long%>");
 		  specs->long_long_p = 1;
 		  break;
 		}
 	      if (specs->short_p)
 		error ("both %<long%> and %<short%> in "
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 141265)
+++ gcc/c.opt	(working copy)
@@ -279,11 +279,11 @@ Warn about invalid uses of the \"offseto
 Winvalid-pch
 C ObjC C++ ObjC++ Warning
 Warn about PCH files that are found but not used
 
 Wlong-long
-C ObjC C++ ObjC++ Var(warn_long_long) Init(1) Warning
+C ObjC C++ ObjC++ Var(warn_long_long) Init(-1) Warning
 Do not warn about using \"long long\" when -pedantic
 
 Wmain
 C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning
 Warn about suspicious declarations of \"main\"
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 141265)
+++ gcc/c-opts.c	(working copy)
@@ -1394,18 +1394,19 @@ sanitize_cpp_opts (void)
     error ("-MG may only be used with -M or -MM");
 
   cpp_opts->unsigned_char = !flag_signed_char;
   cpp_opts->stdc_0_in_system_headers = STDC_0_IN_SYSTEM_HEADERS;
 
-  /* We want -Wno-long-long to override -pedantic -std=non-c99
-     and/or -Wtraditional, whatever the ordering.  */
-  cpp_opts->warn_long_long
-    = warn_long_long && ((pedantic
-			  && (c_dialect_cxx ()
-			      ? cxx_dialect == cxx98
-			      : !flag_isoc99))
-                         || warn_traditional);
+  /* Wlong-long is disabled by default. It is enabled by:
+      [-pedantic | -Wtraditional] -std=[gnu|c]++98 ; or
+      [-pedantic | -Wtraditional] -std=non-c99 . 
+
+      Either -Wlong-long or -Wno-long-long override any other settings.  */
+  if (warn_long_long == -1)
+    warn_long_long = ((pedantic || warn_traditional)
+		      && (c_dialect_cxx () ? cxx_dialect == cxx98 : !flag_isoc99));
+  cpp_opts->warn_long_long = warn_long_long;
 
   /* Similarly with -Wno-variadic-macros.  No check for c99 here, since
      this also turns off warnings about GCCs extension.  */
   cpp_opts->warn_variadic_macros
     = warn_variadic_macros && (pedantic || warn_traditional);
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 141265)
+++ gcc/c-parser.c	(working copy)
@@ -815,18 +815,16 @@ disable_extension_diagnostics (void)
 {
   int ret = (pedantic
 	     | (warn_pointer_arith << 1)
 	     | (warn_traditional << 2)
 	     | (flag_iso << 3)
-	     | (warn_long_long << 4)
-	     | (cpp_opts->warn_long_long << 5));
+	     | (warn_long_long << 4));
   cpp_opts->pedantic = pedantic = 0;
   warn_pointer_arith = 0;
   cpp_opts->warn_traditional = warn_traditional = 0;
   flag_iso = 0;
-  warn_long_long = 0;
-  cpp_opts->warn_long_long = 0;
+  cpp_opts->warn_long_long = warn_long_long = 0;
   return ret;
 }
 
 /* Restore the warning flags which are controlled by __extension__.
    FLAGS is the return value from disable_extension_diagnostics.  */
@@ -836,12 +834,11 @@ restore_extension_diagnostics (int flags
 {
   cpp_opts->pedantic = pedantic = flags & 1;
   warn_pointer_arith = (flags >> 1) & 1;
   cpp_opts->warn_traditional = warn_traditional = (flags >> 2) & 1;
   flag_iso = (flags >> 3) & 1;
-  warn_long_long = (flags >> 4) & 1;
-  cpp_opts->warn_long_long = (flags >> 5) & 1;
+  cpp_opts->warn_long_long = warn_long_long = (flags >> 4) & 1;
 }
 
 /* Possibly kinds of declarator to parse.  */
 typedef enum c_dtr_syn {
   /* A normal declarator with an identifier.  */
Index: libcpp/init.c
===================================================================
--- libcpp/init.c	(revision 141265)
+++ libcpp/init.c	(working copy)
@@ -157,11 +157,11 @@ cpp_create_reader (enum c_lang lang, has
   CPP_OPTION (pfile, tabstop) = 8;
   CPP_OPTION (pfile, operator_names) = 1;
   CPP_OPTION (pfile, warn_trigraphs) = 2;
   CPP_OPTION (pfile, warn_endif_labels) = 1;
   CPP_OPTION (pfile, warn_deprecated) = 1;
-  CPP_OPTION (pfile, warn_long_long) = !CPP_OPTION (pfile, c99);
+  CPP_OPTION (pfile, warn_long_long) = 0;
   CPP_OPTION (pfile, dollars_in_ident) = 1;
   CPP_OPTION (pfile, warn_dollars) = 1;
   CPP_OPTION (pfile, warn_variadic_macros) = 1;
   CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1;
   CPP_OPTION (pfile, warn_normalize) = normalized_C;
Index: libcpp/expr.c
===================================================================
--- libcpp/expr.c	(revision 141265)
+++ libcpp/expr.c	(working copy)
@@ -393,14 +393,16 @@ cpp_classify_number (cpp_reader *pfile, 
 		       "traditional C rejects the \"%.*s\" suffix",
 		       (int) (limit - str), str);
 	}
 
       if ((result & CPP_N_WIDTH) == CPP_N_LARGE
-	  && ! CPP_OPTION (pfile, c99)
 	  && CPP_OPTION (pfile, warn_long_long))
-	cpp_error (pfile, CPP_DL_PEDWARN,
-		   "use of C99 long long integer constant");
+	cpp_error (pfile, 
+		   CPP_OPTION (pfile, c99) ? CPP_DL_WARNING : CPP_DL_PEDWARN,
+		   CPP_OPTION (pfile, cplusplus) 
+		   ? "use of C++0x long long integer constant"
+		   : "use of C99 long long integer constant");
 
       result |= CPP_N_INTEGER;
     }
 
  syntax_ok:

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2008-10-23  2:42     ` Manuel López-Ibáñez
@ 2009-04-10 19:12       ` Manuel López-Ibáñez
  2009-04-10 21:41         ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2009-04-10 19:12 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 3371 bytes --]

PING

Updated to a recent revision. Bootstrapped and regression tested on
x86_64-unknown-linux-gnu with
--enable-languages=all,obj-c++ --enable-decimal-float and
--target_board=\{-m32,-m64\}

OK for trunk?

Manuel.


2008/10/23 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> I think the following patch addresses all requirements and in the
> process it fixes some small deficiencies of the current status, such
> as talking about C99 when invoked from C++.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu with
> --enable-languages=all,obj-c++ --enable-decimal-float and
> --target_board=\{-m32,-m64\}
>
> OK for trunk?
>
> 2008-10-23  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>
>        PR 13358
>        * doc/invoke.texi (-Wlong-long): Update description.
>        * c-lex (interpret_integer): Only warn if there was no previous
>        overflow and -Wlong-long is enabled.
>        * c-decl.c (declspecs_add_type): Drop redundant flags.
>        * c.opt (Wlong-long): Init to -1.
>        * c-opts.c (sanitize_cpp_opts): Synchronize cpp's warn_long_long
>        and front-end warn_long_long. Wlong-long only depends on other
>        flags if it is uninitialized.
>        * c-parser.c (disable_extension_diagnostics): warn_long_long is
>        the same for CPP and FE.
>        (restore_extension_diagnostics): Likewise.
> libcpp/
>        * init.c (cpp_create_reader): Wlong_long is disabled by default.
>        * expr.c (cpp_classify_number): Give different messages for C and
>        C++ front-ends.
> cp/
>        * parser.c (cp_parser_check_decl_spec): Drop redundant flags.
>        * error.c (pedwarn_cxx98): New.
>        * cp-tree.h (pedwarn_cxx98): Declare.
>
> testsuite/
>        * gcc.dg/wtr-int-type-1.c: Use two dg-warning to match two
>        messages. Test for "long long" in system headers.
>        * gcc.dg/c99-longlong-2.c: New.
>        * g++.dg/warn/pr13358.C: New.
>        * g++.dg/warn/pr13358-2.C: New.
>        * g++.dg/warn/pr13358-3.C: New.
>        * g++.dg/warn/pr13358-4.C: New.
>
>
>
> 2008/8/29 Manuel López-Ibáñez <lopezibanez@gmail.com>:
>> 2008/8/29 Joseph S. Myers <joseph@codesourcery.com>:
>>> On Fri, 29 Aug 2008, Manuel López-Ibáñez wrote:
>>>
>>>> CPP and the FE were using different semantics. invoke.texi said that
>>>> Wlong-long was the default. This was not correct. It was the default
>>>> for CPP but -Wno-long-long was the default for the FE. Now there is
>>>> only one semantic: the one from the FE.
>>>>
>>>> However, we were inhibiting warning even if the user requested it
>>>> through Wlong-long. Now explicit Wlong-long (or Wno-long-long)
>>>> overrides everything else, except in system headers.
>>>
>>> If you allow -Wlong-long to have an effect in C99 mode, -std=c99
>>> -pedantic-errors -Wlong-long should give the diagnostics as warnings not
>>> errors.  Does it?
>>
>> No but I can make it do that just using pedwarn_c90 instead of pedwarn
>> and add a testcase. I'll do the same for c++98. I''ll need to add an
>> equivalent of pedwarn_c90 to C++ and CPP. In C++ is probably easy.
>>
>> Is there a way to tell in CPP if we are being called from the C++ FE?
>> and which standard? I cannot find anything in the sources apart from
>> CPP_OPTION (pfile, c99).
>>
>> Cheers,
>>
>> Manuel.
>>
>

[-- Attachment #2: fix-pr13358-try2.diff --]
[-- Type: text/plain, Size: 17229 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 145853)
+++ gcc/doc/invoke.texi	(working copy)
@@ -4128,14 +4128,13 @@ Warn if a precompiled header (@pxref{Pre
 the search path but can't be used.
 
 @item -Wlong-long
 @opindex Wlong-long
 @opindex Wno-long-long
-Warn if @samp{long long} type is used.  This is default.  To inhibit
-the warning messages, use @option{-Wno-long-long}.  Flags
-@option{-Wlong-long} and @option{-Wno-long-long} are taken into account
-only when @option{-pedantic} flag is used.
+Warn if @samp{long long} type is used.  This is enabled by either
+@option{-pedantic} or @option{-Wtraditional} in ISO C90 and C++98
+modes.  To inhibit the warning messages, use @option{-Wno-long-long}.
 
 @item -Wvariadic-macros
 @opindex Wvariadic-macros
 @opindex Wno-variadic-macros
 Warn if variadic macros are used in pedantic ISO C90 mode, or the GNU
Index: gcc/c-lex.c
===================================================================
--- gcc/c-lex.c	(revision 145853)
+++ gcc/c-lex.c	(working copy)
@@ -580,17 +580,22 @@ interpret_integer (const cpp_token *toke
     /* cpplib has already issued a warning for overflow.  */
     type = ((flags & CPP_N_UNSIGNED)
 	    ? widest_unsigned_literal_type_node
 	    : widest_integer_literal_type_node);
   else
-    type = integer_types[itk];
-
-  if (itk > itk_unsigned_long
-      && (flags & CPP_N_WIDTH) != CPP_N_LARGE
-      && !in_system_header && !flag_isoc99)
-    pedwarn (input_location, 0, "integer constant is too large for %qs type",
-	     (flags & CPP_N_UNSIGNED) ? "unsigned long" : "long");
+    {
+      type = integer_types[itk];
+      if (itk > itk_unsigned_long
+	  && (flags & CPP_N_WIDTH) != CPP_N_LARGE)
+	emit_diagnostic 
+	  ((c_dialect_cxx () ? cxx_dialect == cxx98 : !flag_isoc99)
+	   ? DK_PEDWARN : DK_WARNING,
+	   input_location, OPT_Wlong_long,
+	   (flags & CPP_N_UNSIGNED) 
+	   ? "integer constant is too large for %<unsigned long%> type"
+	   : "integer constant is too large for %<long%> type");
+    }
 
   value = build_int_cst_wide (type, integer.low, integer.high);
 
   /* Convert imaginary to a complex type.  */
   if (flags & CPP_N_IMAGINARY)
Index: gcc/testsuite/gcc.dg/c90-longlong-1.c
===================================================================
--- gcc/testsuite/gcc.dg/c90-longlong-1.c	(revision 145853)
+++ gcc/testsuite/gcc.dg/c90-longlong-1.c	(working copy)
@@ -1,7 +1,6 @@
 /* Test for long long: in C99 only.  */
 /* Origin: Joseph Myers <jsm28@cam.ac.uk> */
 /* { dg-do compile } */
 /* { dg-options "-std=iso9899:1990 -pedantic-errors" } */
 
-long long foo; /* { dg-bogus "warning" "warning in place of error" } */
-/* { dg-error "long long" "long long not in C90" { target *-*-* } 6 } */
+long long foo; /* { dg-error "long long" "long long not in C90" } */
Index: gcc/testsuite/gcc.dg/wtr-int-type-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wtr-int-type-1.c	(revision 145853)
+++ gcc/testsuite/gcc.dg/wtr-int-type-1.c	(working copy)
@@ -23,13 +23,21 @@ testfunc ()
      we can pretend it worked the way it does in C99.]  */
   i = 9223372036854775807;
 
   /* But this one should, since it doesn't fit in long (long), but
      does fit in unsigned long (long).  */
-  i = 18446744073709551615; /* { dg-warning "decimal constant|unsigned" "decimal constant" } */
-  
+  i = 18446744073709551615; /* { dg-warning "integer constant is so large that it is unsigned" "decimal constant" } */
+  /* { dg-warning "this decimal constant would be unsigned in ISO C90" "decimal constant" { target *-*-* } 28 } */
+
 # 29 "sys-header.h" 3
+}
+
+void
+testfunc2( ) 
+{ 
+  long long i;
+
 /* We are in system headers now, no -Wtraditional warnings should issue.  */
 
   i = 0x80000000;
   i = 0xFFFFFFFF;
   i = 037777777777;
@@ -39,5 +47,6 @@ testfunc ()
   i = 01777777777777777777777;
   
   i = 9223372036854775807;
   i = 18446744073709551615;
 }
+
Index: gcc/testsuite/gcc.dg/c99-longlong-2.c
===================================================================
--- gcc/testsuite/gcc.dg/c99-longlong-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/c99-longlong-2.c	(revision 0)
@@ -0,0 +1,6 @@
+/* Test for long long: if explicit Wlong-long, in C99 only warn, not
+   pedwarn.  */
+/* { dg-do compile } */
+/* { dg-options "-std=iso9899:1999 -pedantic-errors -Wlong-long" } */
+
+long long foo; /* { dg-warning "long long" } */
Index: gcc/testsuite/g++.dg/warn/pr13358-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-2.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus }
+// { dg-options "-std=c++98 -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; // { dg-error "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we error with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x2 = 1956772631100509574; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x3 = 0154476645345674746606; // { dg-error "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; // { dg-error "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we error with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL; // { dg-error "long long" }
+  x2 = 1956772631100509574LL; // { dg-error "long long" }
+  x3 = 0154476645345674746606LL; // { dg-error "long long" }
+}
Index: gcc/testsuite/g++.dg/warn/pr13358-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-3.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++0x -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL;
+  x2 = 1956772631100509574LL;
+  x3 = 0154476645345674746606LL;
+}
Index: gcc/testsuite/g++.dg/warn/pr13358-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358-4.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++0x -pedantic-errors -Wlong-long" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; // { dg-warning "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we warn with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x2 = 1956772631100509574; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+  x3 = 0154476645345674746606; // { dg-warning "integer constant is too large for 'long' type" "long long" { target ilp32 } }
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; // { dg-warning "ISO C\\+\\+ 1998 does not support 'long long'" }
+  // make sure we warn with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL; // { dg-warning "long long" }
+  x2 = 1956772631100509574LL; // { dg-warning "long long" }
+  x3 = 0154476645345674746606LL; // { dg-warning "long long" }
+}
Index: gcc/testsuite/g++.dg/warn/pr13358.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr13358.C	(revision 0)
@@ -0,0 +1,24 @@
+// PR c++/13358: g++ should accept a long long constant sans LL suffix
+// if -Wno-long-long is in use.
+// { dg-do compile }
+// { dg-require-effective-target int32plus } 
+// { dg-options "-std=c++98 -Wno-long-long -pedantic-errors" }
+
+
+void use_longlong ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86;
+  x2 = 1956772631100509574;
+  x3 = 0154476645345674746606;
+}
+
+void use_longlong2 ()
+{
+  unsigned long long x1, x2, x3; 
+  // make sure it's ok with hex, decimal and octal
+  x1 = 0x1b27da572ef3cd86LL;
+  x2 = 1956772631100509574LL;
+  x3 = 0154476645345674746606LL;
+}
Index: gcc/cp/error.c
===================================================================
--- gcc/cp/error.c	(revision 145853)
+++ gcc/cp/error.c	(working copy)
@@ -2802,5 +2802,24 @@ maybe_warn_cpp0x (const char* str)
 void
 maybe_warn_variadic_templates (void)
 {
   maybe_warn_cpp0x ("variadic templates");
 }
+
+
+/* Issue an ISO C++98 pedantic warning MSGID.  This function is supposed to
+   be used for matters that are allowed in ISO C++0x but not supported in
+   ISO C++98, thus we explicitly don't pedwarn when ISO c++0x is specified.  */
+
+bool
+pedwarn_cxx98 (location_t location, int opt, const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, location,
+		       (cxx_dialect == cxx98) ? DK_PEDWARN : DK_WARNING);
+  diagnostic.option_index = opt;
+  va_end (ap);
+  return report_diagnostic (&diagnostic);
+}
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 145853)
+++ gcc/cp/parser.c	(working copy)
@@ -2164,14 +2164,13 @@ cp_parser_check_decl_spec (cp_decl_speci
       /* The "long" specifier is a special case because of "long long".  */
       if (ds == ds_long)
 	{
 	  if (count > 2)
 	    error ("%H%<long long long%> is too long for GCC", &location);
-	  else if (pedantic && !in_system_header && warn_long_long
-                   && cxx_dialect == cxx98)
-	    pedwarn (location, OPT_Wlong_long, 
-		     "ISO C++ 1998 does not support %<long long%>");
+	  else 
+	    pedwarn_cxx98 (location, OPT_Wlong_long, 
+			   "ISO C++ 1998 does not support %<long long%>");
 	}
       else if (count > 1)
 	{
 	  static const char *const decl_spec_names[] = {
 	    "signed",
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 145853)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -4453,10 +4453,11 @@ extern const char *lang_decl_name		(tree
 extern const char *language_to_string		(enum languages);
 extern const char *class_key_or_enum_as_string	(tree);
 extern void print_instantiation_context		(void);
 extern void maybe_warn_variadic_templates       (void);
 extern void maybe_warn_cpp0x			(const char *);
+extern bool pedwarn_cxx98                       (location_t, int, const char *, ...) ATTRIBUTE_GCC_CXXDIAG(3,4);
 
 /* in except.c */
 extern void init_exception_processing		(void);
 extern tree expand_start_catch_block		(tree);
 extern void expand_end_catch_block		(void);
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 145853)
+++ gcc/c-decl.c	(working copy)
@@ -7296,12 +7296,12 @@ declspecs_add_type (struct c_declspecs *
 		    {
 		      error ("both %<long long%> and %<double%> in "
 			     "declaration specifiers");
 		      break;
 		    }
-		  if (pedantic && !flag_isoc99 && !in_system_header)
-		    pedwarn (input_location, OPT_Wlong_long, "ISO C90 does not support %<long long%>");
+		  pedwarn_c90 (input_location, OPT_Wlong_long, 
+			       "ISO C90 does not support %<long long%>");
 		  specs->long_long_p = 1;
 		  break;
 		}
 	      if (specs->short_p)
 		error ("both %<long%> and %<short%> in "
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 145853)
+++ gcc/c.opt	(working copy)
@@ -283,11 +283,11 @@ Warn about invalid uses of the \"offseto
 Winvalid-pch
 C ObjC C++ ObjC++ Warning
 Warn about PCH files that are found but not used
 
 Wlong-long
-C ObjC C++ ObjC++ Var(warn_long_long) Init(1) Warning
+C ObjC C++ ObjC++ Var(warn_long_long) Init(-1) Warning
 Do not warn about using \"long long\" when -pedantic
 
 Wmain
 C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning
 Warn about suspicious declarations of \"main\"
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 145853)
+++ gcc/c-opts.c	(working copy)
@@ -1410,18 +1410,19 @@ sanitize_cpp_opts (void)
     error ("-MG may only be used with -M or -MM");
 
   cpp_opts->unsigned_char = !flag_signed_char;
   cpp_opts->stdc_0_in_system_headers = STDC_0_IN_SYSTEM_HEADERS;
 
-  /* We want -Wno-long-long to override -pedantic -std=non-c99
-     and/or -Wtraditional, whatever the ordering.  */
-  cpp_opts->warn_long_long
-    = warn_long_long && ((pedantic
-			  && (c_dialect_cxx ()
-			      ? cxx_dialect == cxx98
-			      : !flag_isoc99))
-                         || warn_traditional);
+  /* Wlong-long is disabled by default. It is enabled by:
+      [-pedantic | -Wtraditional] -std=[gnu|c]++98 ; or
+      [-pedantic | -Wtraditional] -std=non-c99 . 
+
+      Either -Wlong-long or -Wno-long-long override any other settings.  */
+  if (warn_long_long == -1)
+    warn_long_long = ((pedantic || warn_traditional)
+		      && (c_dialect_cxx () ? cxx_dialect == cxx98 : !flag_isoc99));
+  cpp_opts->warn_long_long = warn_long_long;
 
   /* Similarly with -Wno-variadic-macros.  No check for c99 here, since
      this also turns off warnings about GCCs extension.  */
   cpp_opts->warn_variadic_macros
     = warn_variadic_macros && (pedantic || warn_traditional);
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 145853)
+++ gcc/c-parser.c	(working copy)
@@ -815,18 +815,16 @@ disable_extension_diagnostics (void)
 {
   int ret = (pedantic
 	     | (warn_pointer_arith << 1)
 	     | (warn_traditional << 2)
 	     | (flag_iso << 3)
-	     | (warn_long_long << 4)
-	     | (cpp_opts->warn_long_long << 5));
+	     | (warn_long_long << 4));
   cpp_opts->pedantic = pedantic = 0;
   warn_pointer_arith = 0;
   cpp_opts->warn_traditional = warn_traditional = 0;
   flag_iso = 0;
-  warn_long_long = 0;
-  cpp_opts->warn_long_long = 0;
+  cpp_opts->warn_long_long = warn_long_long = 0;
   return ret;
 }
 
 /* Restore the warning flags which are controlled by __extension__.
    FLAGS is the return value from disable_extension_diagnostics.  */
@@ -836,12 +834,11 @@ restore_extension_diagnostics (int flags
 {
   cpp_opts->pedantic = pedantic = flags & 1;
   warn_pointer_arith = (flags >> 1) & 1;
   cpp_opts->warn_traditional = warn_traditional = (flags >> 2) & 1;
   flag_iso = (flags >> 3) & 1;
-  warn_long_long = (flags >> 4) & 1;
-  cpp_opts->warn_long_long = (flags >> 5) & 1;
+  cpp_opts->warn_long_long = warn_long_long = (flags >> 4) & 1;
 }
 
 /* Possibly kinds of declarator to parse.  */
 typedef enum c_dtr_syn {
   /* A normal declarator with an identifier.  */
Index: libcpp/init.c
===================================================================
--- libcpp/init.c	(revision 145853)
+++ libcpp/init.c	(working copy)
@@ -158,11 +158,11 @@ cpp_create_reader (enum c_lang lang, has
   CPP_OPTION (pfile, tabstop) = 8;
   CPP_OPTION (pfile, operator_names) = 1;
   CPP_OPTION (pfile, warn_trigraphs) = 2;
   CPP_OPTION (pfile, warn_endif_labels) = 1;
   CPP_OPTION (pfile, warn_deprecated) = 1;
-  CPP_OPTION (pfile, warn_long_long) = !CPP_OPTION (pfile, c99);
+  CPP_OPTION (pfile, warn_long_long) = 0;
   CPP_OPTION (pfile, dollars_in_ident) = 1;
   CPP_OPTION (pfile, warn_dollars) = 1;
   CPP_OPTION (pfile, warn_variadic_macros) = 1;
   CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1;
   CPP_OPTION (pfile, warn_normalize) = normalized_C;
Index: libcpp/expr.c
===================================================================
--- libcpp/expr.c	(revision 145853)
+++ libcpp/expr.c	(working copy)
@@ -417,14 +417,16 @@ cpp_classify_number (cpp_reader *pfile, 
 		       "traditional C rejects the \"%.*s\" suffix",
 		       (int) (limit - str), str);
 	}
 
       if ((result & CPP_N_WIDTH) == CPP_N_LARGE
-	  && ! CPP_OPTION (pfile, c99)
 	  && CPP_OPTION (pfile, warn_long_long))
-	cpp_error (pfile, CPP_DL_PEDWARN,
-		   "use of C99 long long integer constant");
+	cpp_error (pfile, 
+		   CPP_OPTION (pfile, c99) ? CPP_DL_WARNING : CPP_DL_PEDWARN,
+		   CPP_OPTION (pfile, cplusplus) 
+		   ? "use of C++0x long long integer constant"
+		   : "use of C99 long long integer constant");
 
       result |= CPP_N_INTEGER;
     }
 
  syntax_ok:

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-10 19:12       ` Manuel López-Ibáñez
@ 2009-04-10 21:41         ` Joseph S. Myers
  2009-04-19 11:16           ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Joseph S. Myers @ 2009-04-10 21:41 UTC (permalink / raw)
  To: Manuel López-Ibáñez; +Cc: Gcc Patch List

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 463 bytes --]

On Fri, 10 Apr 2009, Manuel López-Ibáñez wrote:

> PING
> 
> Updated to a recent revision. Bootstrapped and regression tested on
> x86_64-unknown-linux-gnu with
> --enable-languages=all,obj-c++ --enable-decimal-float and
> --target_board=\{-m32,-m64\}
> 
> OK for trunk?

As far as I can tell this is essentially a C++ patch (to code mostly 
shared between C and C++) and so should be reviewed as such.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-10 21:41         ` Joseph S. Myers
@ 2009-04-19 11:16           ` Manuel López-Ibáñez
  2009-04-20  3:34             ` Mark Mitchell
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2009-04-19 11:16 UTC (permalink / raw)
  To: Gcc Patch List; +Cc: Jason Merrill, Nathan Sidwell, Mark Mitchell

PING2: http://gcc.gnu.org/ml/gcc-patches/2009-04/msg00842.html

2009/4/10 Joseph S. Myers <joseph@codesourcery.com>:
> On Fri, 10 Apr 2009, Manuel López-Ibáñez wrote:
>
>> PING
>>
>> Updated to a recent revision. Bootstrapped and regression tested on
>> x86_64-unknown-linux-gnu with
>> --enable-languages=all,obj-c++ --enable-decimal-float and
>> --target_board=\{-m32,-m64\}
>>
>> OK for trunk?
>
> As far as I can tell this is essentially a C++ patch (to code mostly
> shared between C and C++) and so should be reviewed as such.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-19 11:16           ` Manuel López-Ibáñez
@ 2009-04-20  3:34             ` Mark Mitchell
  2009-04-20  8:38               ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Mitchell @ 2009-04-20  3:34 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Gcc Patch List, Jason Merrill, Nathan Sidwell

Manuel López-Ibáñez wrote:
> PING2: http://gcc.gnu.org/ml/gcc-patches/2009-04/msg00842.html

> +/* Issue an ISO C++98 pedantic warning MSGID.  This function is supposed to
> +   be used for matters that are allowed in ISO C++0x but not supported in
> +   ISO C++98, thus we explicitly don't pedwarn when ISO c++0x is specified.  */

This comment needs work.  The parameter is GMSGID.  There are two other
parameters not mentioned.  The ", thus" is a run-on sentence.  "is
supposed to be" is inappropriate in an interface specification.  Please
replace the last two sentences with:

  Use this function to report diagnostics for constructs that are
invalid C++98, but valid C++0x.

and document the other parameters.

OK with that change.

It's in no way material to this patch, but I wish you would not put so
much of the error text into test-cases.  I realize that you feel it's
important to test the exact message, but I feel that this just creates a
disincentive to make small changes to improve the text of a message.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20  3:34             ` Mark Mitchell
@ 2009-04-20  8:38               ` Manuel López-Ibáñez
  2009-04-20 15:14                 ` Mark Mitchell
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2009-04-20  8:38 UTC (permalink / raw)
  To: Mark Mitchell
  Cc: Gcc Patch List, Jason Merrill, Nathan Sidwell, Janis Johnson

2009/4/20 Mark Mitchell <mark@codesourcery.com>:
>
> It's in no way material to this patch, but I wish you would not put so
> much of the error text into test-cases.  I realize that you feel it's
> important to test the exact message, but I feel that this just creates a
> disincentive to make small changes to improve the text of a message.
>

After fixing the C testsuite to distinguish between errors and
warnings, I have the completely opposite opinion. If one wants to
change an error text, it is better to have as much text as possible to
do a find + sed and replace everything in one go. If one does not have
any text, a logic change can start producing nonsense and no one will
notice.

We already have testcases (particularly in g++.old-deja) that do not
match what they were supposed to match. Ask Janis how many C++ tests
she had to guess or just ignore what it was being tested when she
fixed the C++ testsuite to distinguish between errors and warnings.

The largest disincentive to make small changes to improve the text of
a message is the waiting and the slow discussion about the merits and
demerits of the change. People do not have enough patience to follow
up such small changes (c.f. the circular dependency patch+thread which
is still unresolved). On the other hand, spelling changes are quickly
reviewed and committed.

Nonetheless, you are the maintainer, so if you want me to cut the test
pattern, tell me what I should match and I will update the patch.

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20  8:38               ` Manuel López-Ibáñez
@ 2009-04-20 15:14                 ` Mark Mitchell
  2009-04-20 16:17                   ` Manuel López-Ibáñez
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Mitchell @ 2009-04-20 15:14 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Gcc Patch List, Jason Merrill, Nathan Sidwell, Janis Johnson

Manuel López-Ibáñez wrote:

> After fixing the C testsuite to distinguish between errors and
> warnings, I have the completely opposite opinion. If one wants to
> change an error text, it is better to have as much text as possible to
> do a find + sed and replace everything in one go. If one does not have
> any text, a logic change can start producing nonsense and no one will
> notice.

There is an intermediate position between just:

  // { dg-error "" }

which matches anything, and which I agree is sloppy (though I added
plenty such tests over the years) and putting the entire text there.
It's generally possible to pick out a few key words from the error
message ("invalid overload" or "does not match" or "private" or some
such) that are the important part of the error message.

> Nonetheless, you are the maintainer, so if you want me to cut the test
> pattern, tell me what I should match and I will update the patch.

I'm not going to make you change a perfectly good patch just for this.
But, unless Jason/Nathan indicate otherwise, I'd appreciate it if for
future patches you take a more circumspect approach.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20 15:14                 ` Mark Mitchell
@ 2009-04-20 16:17                   ` Manuel López-Ibáñez
  2009-04-20 16:26                     ` Mark Mitchell
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel López-Ibáñez @ 2009-04-20 16:17 UTC (permalink / raw)
  To: Mark Mitchell
  Cc: Gcc Patch List, Jason Merrill, Nathan Sidwell, Janis Johnson

2009/4/20 Mark Mitchell <mark@codesourcery.com>:
> Manuel López-Ibáñez wrote:
>
> which matches anything, and which I agree is sloppy (though I added
> plenty such tests over the years) and putting the entire text there.
> It's generally possible to pick out a few key words from the error
> message ("invalid overload" or "does not match" or "private" or some
> such) that are the important part of the error message.
>

If one changes a message and there are not testsuite changes, then one
cannot know whether this message is actually ever tested. If no
testcase is added, then the feature may go untested. If one adds
another testcase, then we may have two equivalent testcases for the
same thing. The latter happens already in the g++.dg and g++.old-deja
dirs.

If there is one important keyword only, your proposal is reasonable
but if there are several important keywords, then the choices are to
use "\[^\n\]*", which is ugly to read and awful to search/replace, or
use "|", which matches more than we want to match.

For example, if you look at this pattern:

-  i = 18446744073709551615; /* { dg-warning "decimal
constant|unsigned" "decimal constant" } */

you cannot tell which error message is matched. In fact, it turns out
that there are two messages for this line:

+  i = 18446744073709551615; /* { dg-warning "integer constant is so
large that it is unsigned" "decimal constant" } */
+  /* { dg-warning "this decimal constant would be unsigned in ISO
C90" "decimal constant" { target *-*-* } 28 } */

One mentions C90 (but it will still pass if it mentioned C++ or C++0x
or pascal) and the other not. In this case, probably the first message
is redundant but if it were important, we can lose it and it will
never be noticed. We could also lose the second message and the test
will still pass. On the other hand, perhaps the intention was to match
only the second message and the redundant one may have appeared
without anyone noticing.

Moreover, I just noticed that in the g++ testcase I am using:

+  x1 = 0x1b27da572ef3cd86LL; // { dg-warning "long long" }

which is not actually testing that CPP is printing:

     warning: use of C++0x long long integer constant

instead of:

    warning: use of C99 long long integer constant

So this feature may regress in the future without anyone noticing.

>> Nonetheless, you are the maintainer, so if you want me to cut the test
>> pattern, tell me what I should match and I will update the patch.
>
> I'm not going to make you change a perfectly good patch just for this.
> But, unless Jason/Nathan indicate otherwise, I'd appreciate it if for
> future patches you take a more circumspect approach.

OK, I'll try, less work for me. But if I err on the side of
specificity rather than generality, feel free to ask for changes.

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20 16:17                   ` Manuel López-Ibáñez
@ 2009-04-20 16:26                     ` Mark Mitchell
  2009-04-20 17:38                       ` Jason Merrill
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Mitchell @ 2009-04-20 16:26 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Gcc Patch List, Jason Merrill, Nathan Sidwell, Janis Johnson

Manuel López-Ibáñez wrote:

> If one changes a message and there are not testsuite changes, then one
> cannot know whether this message is actually ever tested.

That's clearly true, at some level, but I just think it's overly
pedantic.  It's not like messages change by themselves. :-)  In other
words, I think there's something qualitatively different between a
language feature (where a change to some other part of the compiler
might accidentally break it) and a change to an error message, which is
generally being done very consciously.

I just feel that the benefits of avoiding the potential error message
breakage are outweighed by the costs.  Since it's a tradeoff, reasonable
people can certainly disagree.  I'd like to see us aim for a middle
ground where we have enough text to capture key concepts, but not the
entire message.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20 16:26                     ` Mark Mitchell
@ 2009-04-20 17:38                       ` Jason Merrill
  2009-04-23  9:15                         ` Dave Korn
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Merrill @ 2009-04-20 17:38 UTC (permalink / raw)
  To: Mark Mitchell
  Cc: Manuel López-Ibáñez, Gcc Patch List,
	Nathan Sidwell, Janis Johnson

Mark Mitchell wrote:
>  I'd like to see us aim for a middle
> ground where we have enough text to capture key concepts, but not the
> entire message.

I agree.

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-20 17:38                       ` Jason Merrill
@ 2009-04-23  9:15                         ` Dave Korn
  2009-04-23 23:46                           ` Mark Mitchell
  0 siblings, 1 reply; 16+ messages in thread
From: Dave Korn @ 2009-04-23  9:15 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Mark Mitchell, Manuel López-Ibáñez,
	Gcc Patch List, Nathan Sidwell, Janis Johnson

Jason Merrill wrote:
> Mark Mitchell wrote:
>>  I'd like to see us aim for a middle
>> ground where we have enough text to capture key concepts, but not the
>> entire message.
> 
> I agree.
> 
> Jason

    Hi Mark and Jason,

  I intend to submit a testcase that generates (and verifies) the warning:

/tmp/string-null-ctor.C:9: warning: null argument where non-null required
(argument 1)

  Would either of you care to comment on what would be a suitable pattern to test?

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-23  9:15                         ` Dave Korn
@ 2009-04-23 23:46                           ` Mark Mitchell
  2009-04-24 13:39                             ` Dave Korn
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Mitchell @ 2009-04-23 23:46 UTC (permalink / raw)
  To: Dave Korn
  Cc: Jason Merrill, Manuel López-Ibáñez,
	Gcc Patch List, Nathan Sidwell, Janis Johnson

Dave Korn wrote:

> /tmp/string-null-ctor.C:9: warning: null argument where non-null required
> (argument 1)
> 
>   Would either of you care to comment on what would be a suitable pattern to test?

I'd just say "null".  Or maybe "null pointer" (see below).

As a nit-pick on the message, and without having much context, I suggest
something like

  warning: argument is the null pointer

That's because (a) I like using terms of the standard, and "the null
pointer" is a term from the standard, where as just plain "null" isn't,
(b) once you point out it's null, I don't think you add much by saying
that what you want is something non-null.

As for the test question, to me "null" is the key word here.  Any good
warning message for this situation will say "null", and any message
that's talking about "null" is probably saying something useful.

My two cents,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [C/C++] PR 13358 long long and C++ do not mix well
  2009-04-23 23:46                           ` Mark Mitchell
@ 2009-04-24 13:39                             ` Dave Korn
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Korn @ 2009-04-24 13:39 UTC (permalink / raw)
  To: Mark Mitchell
  Cc: Dave Korn, Jason Merrill, Manuel López-Ibáñez,
	Gcc Patch List, Nathan Sidwell, Janis Johnson

Mark Mitchell wrote:
> Dave Korn wrote:
> 
>> /tmp/string-null-ctor.C:9: warning: null argument where non-null required
>> (argument 1)
>>
>>   Would either of you care to comment on what would be a suitable pattern to test?
> 
> I'd just say "null".  Or maybe "null pointer" (see below).
> 
> As a nit-pick on the message, and without having much context, 

  Ah, if I'd given you more context it would have been clear that I'm testing
an existing warning, not writing a new one!

> As for the test question, to me "null" is the key word here.  Any good
> warning message for this situation will say "null", and any message
> that's talking about "null" is probably saying something useful.
> 
> My two cents,

  Your reasoning makes perfect sense to me.  I was originally thinking that it
would also be worth testing the "1" to see that it got the argument number
right, but that would be more sensible in a test /of/ the warning, rather than
a test /for/ the warning.

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-04-24 13:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-30  1:11 [C/C++] PR 13358 long long and C++ do not mix well Manuel López-Ibáñez
2008-08-30  2:15 ` Joseph S. Myers
2008-08-30  9:52   ` Manuel López-Ibáñez
2008-10-23  2:42     ` Manuel López-Ibáñez
2009-04-10 19:12       ` Manuel López-Ibáñez
2009-04-10 21:41         ` Joseph S. Myers
2009-04-19 11:16           ` Manuel López-Ibáñez
2009-04-20  3:34             ` Mark Mitchell
2009-04-20  8:38               ` Manuel López-Ibáñez
2009-04-20 15:14                 ` Mark Mitchell
2009-04-20 16:17                   ` Manuel López-Ibáñez
2009-04-20 16:26                     ` Mark Mitchell
2009-04-20 17:38                       ` Jason Merrill
2009-04-23  9:15                         ` Dave Korn
2009-04-23 23:46                           ` Mark Mitchell
2009-04-24 13:39                             ` Dave Korn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).