diff mbox

[2/2] Better system header location detection for built-in macro tokens

Message ID 556DFC71-868F-404B-B072-478DBC33A5BC@comcast.net
State New
Headers show

Commit Message

Mike Stump June 4, 2012, 6:54 p.m. UTC 
On Jun 4, 2012, at 5:36 AM, Dodji Seketeli wrote:
>>> -  s = s; // { dg-message "synthesized|deleted" }
>>> +  s = s;
>> 
>> Why do we lose this error?
> 
> Good question.
> 
> This is the result of a weird behaviour of DejaGNU, I think.

Yup, that is truly weird.  The problem is that the | operator is binding funny because there are no () around the substituted variables in the matching expressions in the .exp files, and the messages for a single line get the opportunity to match and eat multiple lines.

So, dejagnu is using:

	{[0-9]+: error:[^\n]*const|operator}

instead of:

	{[0-9]+: error:[^\n]*(const|operator)}

and that makes all the difference.

If you want, you can use just const, like so:

template <class T>
struct S {  // { dg-error "const" }
  S();
  T t;
};

The key is not to use |, or if you want to use |, you can (for now, until the underlying bug is fix) use:

template <class T>
struct S {  // { dg-error "(const|operator)" }
  S();
  T t;
}

to fix the underlying problem, we need something like:



which I'll test and see how bad off any other test cases are.  If people on fast machines want to weigh in, that'd be good.

Comments

Jason Merrill June 4, 2012, 7:30 p.m. UTC | #1 
On 06/04/2012 02:54 PM, Mike Stump wrote:
> Yup, that is truly weird.  The problem is that the | operator is binding funny because there are no () around the substituted variables in the matching expressions in the .exp files, and the messages for a single line get the opportunity to match and eat multiple lines.
>
> So, dejagnu is using:
>
> 	{[0-9]+: error:[^\n]*const|operator}
>
> instead of:
>
> 	{[0-9]+: error:[^\n]*(const|operator)}
>
> and that makes all the difference.

That was my guess, too, but when I look at dg.exp it seems that the 
pattern is properly wrapped in parens.

Jason
Mike Stump June 4, 2012, 9:27 p.m. UTC | #2 
On Jun 4, 2012, at 12:30 PM, Jason Merrill wrote:
> On 06/04/2012 02:54 PM, Mike Stump wrote:
>> Yup, that is truly weird.  The problem is that the | operator is binding funny because there are no () around the substituted variables in the matching expressions in the .exp files, and the messages for a single line get the opportunity to match and eat multiple lines.
>> 
>> So, dejagnu is using:
>> 
>> 	{[0-9]+: error:[^\n]*const|operator}
>> 
>> instead of:
>> 
>> 	{[0-9]+: error:[^\n]*(const|operator)}
>> 
>> and that makes all the difference.
> 
> That was my guess, too, but when I look at dg.exp it seems that the pattern is properly wrapped in parens.

That one is, the problem is the work that added error: and warning: processing differentiation puts together a new expression with concatenation and it is that use that _also_ needs protecting.

	error: CONCAT regexp

is wrong, whereas:

	error: CONCAT (regexp)

is correct.  This type of patch:

-       set expmsg "$column: $msgprefix\[^\n\]*$expmsg"
+       set expmsg "$column: $msgprefix\[^\n\]*($expmsg)"

might have been easier to understand, but, that repeats three times and is dependent upon people getting the obscure detail of () correct.  I instead add the () at the top, to ensure it is impossible to ever have this error again, no matter the code rearrangements and to have it appear just once, instead of three times.  Possibly:

-	set expmsg [lindex $newentry 2]
+	set expmsg "([lindex $newentry 2])"

might be better...
Jason Merrill June 4, 2012, 9:34 p.m. UTC | #3 
On 06/04/2012 05:27 PM, Mike Stump wrote:
> That one is, the problem is the work that added error: and warning: processing differentiation puts together a new expression with concatenation and it is that use that _also_ needs protecting.

Aha!  Sneaky gcc-dg.exp.

> -	set expmsg [lindex $newentry 2]
> +	set expmsg "([lindex $newentry 2])"

Looks good to me.

Jason
diff mbox

Patch

Index: gcc-dg.exp
===================================================================
--- gcc-dg.exp	(revision 188120)
+++ gcc-dg.exp	(working copy)
@@ -744,6 +744,12 @@  if { [info procs saved-dg-error] == [lis
 proc process-message { msgproc msgprefix dgargs } {
     upvar dg-messages dg-messages
 
+    # We must enclose the expression in () to ensure any | in the
+    # pattern doesn't escape from this part of the expression.
+    set darg1 [lindex $dgargs 0]
+    set dgargs [lindex $dgargs 1]
+    set dgargs "$darg1 ($dgargs)"
+
     # Process the dg- directive, including adding the regular expression
     # to the new message entry in dg-messages.
     set msgcnt [llength ${dg-messages}]