Wrong ChangeLog entries

Wrong ChangeLog entries

[Giovanni Bajo]

Mark,

- with your commit for PR 20142, you also committed a hunk to name-lookup.c
which is not described in the ChangeLog. It is also unclear whether it is
effectively need for that PR or not, but nonetheless it went in, so the entry
should probably be fixed.

- It is unclear which patch fixed PR 19063, which patch fixed PR 13268, or if
the same patch fixed both.
  * Both bugs were closed at the same time, but the ChangeLog entry does not
mention PR 19063. (To add confusion, the original ChangeLog entry mentioned a
totally unrelated bug (17413) instead of PR13268, but that typo was later fixed
in CVS).
  * In the testsuite, I cannot find a testcase for PR13268, I can only find a
testcase for PR 19063 (g++.dg/template/crash31.C), and the testcase is also
wrong (contains a typo, so it does not probably test what it was supposed to
test).
  * The patch in the mail in gcc-patches
(http://gcc.gnu.org/ml/gcc-atches/2004-12/msg01697.html) adds even more
confusion because it contains the diff for crash31.C but with a different
heading comment (PR c++/13268 instead of PR c++/19063).

If those bugs were duplicates, it should have probably be mentioned in both the
ChangeLog and the bug description; also one should be closed as duplicate of
the other, instead of simply marked as "fixed".

Giovanni Bajo

Re: Wrong ChangeLog entries

[Mark Mitchell]

Giovanni Bajo wrote:
> Mark,
> 
> - with your commit for PR 20142, you also committed a hunk to name-lookup.c
> which is not described in the ChangeLog. It is also unclear whether it is
> effectively need for that PR or not, but nonetheless it went in, so the entry
> should probably be fixed

I wanted you to know I'm not ignoring this email.  I've not gotten to it 
yet, but I will soon.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: Wrong ChangeLog entries

[Mark Mitchell]

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

Giovanni Bajo wrote:
> Mark,
> 
> - with your commit for PR 20142, you also committed a hunk to name-lookup.c
> which is not described in the ChangeLog. It is also unclear whether it is
> effectively need for that PR or not, but nonetheless it went in, so the entry
> should probably be fixed.

Thanks for spotting that.  It was a harmless, but, as far as I can tell, 
spurious check-in.  I tested that removing it caused no regression on 
x86_64-linux-gnu, so I'm going to remove it.  Patch below.

> - It is unclear which patch fixed PR 19063, which patch fixed PR 13268, or if
> the same patch fixed both.

The same patch fixed both.  Part of the confusion here comes from the 
fact that PR19063 contains references to a source file called PR13628. 
I think that's part of how I got tangled.

>   * Both bugs were closed at the same time, but the ChangeLog entry does not
> mention PR 19063.

That's confusing; I'll fix testsuite/ChangeLog to say 19063.

>   * In the testsuite, I cannot find a testcase for PR13268, I can only find a
> testcase for PR 19063 (g++.dg/template/crash31.C), and the testcase is also
> wrong (contains a typo, so it does not probably test what it was supposed to
> test).

That was an intentional change.  The test case in the PR:

   template<operator> struct A {};

is confusing, since it looks like "operator" is a template argument.  In 
fact, the parser (correctly) sees that as "operator>" and there is no 
closing ">" symbol.

> If those bugs were duplicates, it should have probably be mentioned in both the
> ChangeLog and the bug description; also one should be closed as duplicate of
> the other, instead of simply marked as "fixed".

I don't think it makes sense to put duplicate information in ChangeLogs; 
I find the fact that we put PR#s in ChangeLogs and testcases to be 
redundant enough. :-)  However, I do very much agree that I made a hash 
of this, and that they should be marked as duplicates in Bugzilla.

So, to some up:

1. I reverted the name-lookup.c patch.

2. I changed testsuite/ChangeLog to say 19063.

3. I marked 13268 as a duplicate of 19063 in bugzilla.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

[-- Attachment #2: nl.patch --]
[-- Type: text/plain, Size: 1252 bytes --]

2005-08-05  Mark Mitchell  <mark@codesourcery.com>

	* name-lookup.c (pushtag): Remove accidental commit from:
	2004-12-21  Mark Mitchell  <mark@codesourcery.com>
	PR c++/19063
	* decl.c (grokdeclarator): Return error_mark_node, not
	void_type_node, to indicate errors.
	* parser.c (cp_parser_template_parameter_list): Robustify.
	(cp_parser_template_parameter): Likewise.

Index: cp/name-lookup.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/name-lookup.c,v
retrieving revision 1.133
diff -c -5 -p -r1.133 name-lookup.c
*** cp/name-lookup.c	1 Aug 2005 04:02:26 -0000	1.133
--- cp/name-lookup.c	5 Aug 2005 20:59:31 -0000
*************** pushtag (tree name, tree type, tag_scope
*** 4711,4723 ****
  		pushdecl_class_level (d);
  	    }
  	  else if (b->kind != sk_template_parms)
  	    d = pushdecl_with_scope (d, b);
  
- 	  if (d == error_mark_node)
- 	    POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, error_mark_node);
- 
  	  TYPE_CONTEXT (type) = DECL_CONTEXT (d);
  
  	  /* If this is a local class, keep track of it.  We need this
  	     information for name-mangling, and so that it is possible to find
  	     all function definitions in a translation unit in a convenient
--- 4711,4720 ----