[C++ Patch] PR 35321

[C++ Patch] PR 35321

[Paolo Carlini]

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

Hi,

this is a small follow-up to c++/28573, fixed by Paolo (Bonzini), seems 
just matter of dealing with TARGET_EXPR like with CALL_EXPR. Tested 
x86_64-linux.

Ok for mainline?

Paolo.

////////////////

[-- Attachment #2: CL_35321 --]
[-- Type: text/plain, Size: 252 bytes --]

2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/35321
	* c-common.c (fold_offsetof_1): Handle TARGET_EXPR like CALL_EXPR.

/testsuite
2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/35321
	* g++.dg/parse/offsetof8.C: New.

[-- Attachment #3: patch_35321 --]
[-- Type: text/plain, Size: 899 bytes --]

Index: testsuite/g++.dg/parse/offsetof8.C
===================================================================
*** testsuite/g++.dg/parse/offsetof8.C	(revision 0)
--- testsuite/g++.dg/parse/offsetof8.C	(revision 0)
***************
*** 0 ****
--- 1,16 ----
+ // PR c++/35321
+ 
+ struct A
+ {
+   A operator[] (int);
+ };
+ 
+ struct B
+ {
+   A a;
+ };
+ 
+ void foo()
+ {
+   __builtin_offsetof(B, a[0]); /* { dg-error "error: cannot apply.*offsetof" } */
+ }
Index: c-common.c
===================================================================
*** c-common.c	(revision 139178)
--- c-common.c	(working copy)
*************** fold_offsetof_1 (tree expr, tree stop_re
*** 7660,7665 ****
--- 7660,7666 ----
        return error_mark_node;
  
      case CALL_EXPR:
+     case TARGET_EXPR:
        error ("cannot apply %<offsetof%> when %<operator[]%> is overloaded");
        return error_mark_node;
  

Re: [C++ Patch] PR 35321

[Manuel López-Ibáñez]

2008/8/17 Paolo Carlini <paolo.carlini@oracle.com>:
> +   __builtin_offsetof(B, a[0]); /* { dg-error "error: cannot
> apply.*offsetof" } */

I am sorry for being so picky but I would suggest to not use ".*" in
matches That will match anything including newlines. I guess that in
this case it would be very strange to generate by error multiple
messages that start with "cannot apply" and finish with  offsetof.
However, it is better to be consistent.

In this case you just need "error: cannot apply .offsetof" but I think
it would be better if you use:

"error: cannot apply .offsetof. when .operator[]. is overloaded"

If such message changes in the future we can do a "find + sed" for all
instances. However, matching a partial message is more error prone
(less future proof).

Cheers,

Manuel.

PS: To really match anything but not newlines you should use \[^\n\]*.
I wish there was a way to tell DejaGNU to not match newlines with '.'

Re: [C++ Patch] PR 35321

[Paolo Carlini]

Manuel López-Ibáñez wrote:
> I am sorry for being so picky...
Just wanted to clearly explain that whereas I have nothing in principle 
against your various forms of "picky-ness" - assuming of course they 
make sense and can be followed reasonably easily, as in this case, I 
think - I consider rather strange that nobody speak up before, you 
included. That is confusing. In the case at issue, for example, I wanted 
to remain consistent with the *existing* two other testcases in this area.

Again, if you, and the other C++ maintainers, have special new 
directions about the form of the dg-error strings, please state the 
requirements clearly, once and for all, and add it at least to wiki, or, 
better, to the html pages for contributors.

Paolo.

Re: [C++ Patch] PR 35321

[Manuel López-Ibáñez]

2008/8/17 Paolo Carlini <paolo.carlini@oracle.com>:
> Manuel López-Ibáñez wrote:
>>
>> I am sorry for being so picky...
>
> Just wanted to clearly explain that whereas I have nothing in principle
> against your various forms of "picky-ness" - assuming of course they make
> sense and can be followed reasonably easily, as in this case, I think - I
> consider rather strange that nobody speak up before, you included. That is
> confusing. In the case at issue, for example, I wanted to remain consistent
> with the *existing* two other testcases in this area.

I noticed all these issues when trying to fix PR25241 and PR30612.
When you look at the way dg-error works, you notice the problems with
the current testcases. Also, when fixing diagnostics PRs I noticed
that updating testcases is always less error prone if the message is
as complete as possible.

> Again, if you, and the other C++ maintainers, have special new directions
> about the form of the dg-error strings, please state the requirements
> clearly, once and for all, and add it at least to wiki, or, better, to the
> html pages for contributors.

I am not a C++ maintainer, so I am not sure whether they are aware of
the current limitations (namely, the above PRs) of the C++ testsuite.
I think most of the tests assume that '.' does not match a newline,
dg-error matches an error and dg-warning matches a warning.
Unfortunately, this has never been the case.

Cheers,

Manuel.

Re: [C++ Patch] PR 35321

[Paolo Carlini]

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

.... this is the updated patch, just finished regtesting.

Thanks,
Paolo.

/////////////


[-- Attachment #2: CL_35321 --]
[-- Type: text/plain, Size: 252 bytes --]

2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/35321
	* c-common.c (fold_offsetof_1): Handle TARGET_EXPR like CALL_EXPR.

/testsuite
2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/35321
	* g++.dg/parse/offsetof8.C: New.

[-- Attachment #3: patch_35321a --]
[-- Type: text/plain, Size: 938 bytes --]

Index: testsuite/g++.dg/parse/offsetof8.C
===================================================================
*** testsuite/g++.dg/parse/offsetof8.C	(revision 0)
--- testsuite/g++.dg/parse/offsetof8.C	(revision 0)
***************
*** 0 ****
--- 1,16 ----
+ // PR c++/35321
+ 
+ struct A
+ {
+   A operator[] (int);
+ };
+ 
+ struct B
+ {
+   A a;
+ };
+ 
+ void foo()
+ {
+   __builtin_offsetof(B, a[0]); /* { dg-error "error: cannot apply 'offsetof' when 'operator\\\[\\\]' is overloaded" } */
+ }
Index: c-common.c
===================================================================
*** c-common.c	(revision 139178)
--- c-common.c	(working copy)
*************** fold_offsetof_1 (tree expr, tree stop_re
*** 7660,7665 ****
--- 7660,7666 ----
        return error_mark_node;
  
      case CALL_EXPR:
+     case TARGET_EXPR:
        error ("cannot apply %<offsetof%> when %<operator[]%> is overloaded");
        return error_mark_node;
  

Re: [C++ Patch] PR 35321

[Mark Mitchell]

Paolo Carlini wrote:
> .... this is the updated patch, just finished regtesting.

> 
> 2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>
> 
> 	PR c++/35321
> 	* c-common.c (fold_offsetof_1): Handle TARGET_EXPR like CALL_EXPR.
> 
> /testsuite
> 2008-08-17  Paolo Carlini  <paolo.carlini@oracle.com>
> 
> 	PR c++/35321
> 	* g++.dg/parse/offsetof8.C: New.


OK.


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