Hi Martin, Thanks for your helpful comments.
Martin Sebor <mse...@gmail.com> writes:
> As an aside, the bug number should be referenced in the change
> message so that when the patch is committed it's automatically
> reflected in the bug entry. The contrib/mklog.py script should
> do that automatically based on new test files added to verify
> the bug fix/change, provided one such test is added and starts
> with a PR c/95379 comment. Also mentioning it in the Subject
> of the email is helpful.
>
> [...]
>
> I don't think there's any "rule" against it but it's customary
> to add test files for new features, rather than modify existing
> ones (see also my comment about the post-commit hook above).
>
> [...]
>
> ChangeLog changes need not be included in a patch (and shouldn't
> be). They are now automatically extracted from the commit message
> and added to the ChangeLog files by a post-commit hook.
OK, these will be easy to fix. Should my other patch¹ (which implements
-Wuniversal-initializer) also have PR c/95379?
> Rather than creating own linked list I suggest to consider making
> use of one of GCC's data structures.
How would this be done? I read the gccint manual a bit and, from what I
understand, a 'tree' type can be a linked list of other 'tree's. But how
would I make a linked list of generic types ('location_t's)?
> If the argument to free was returned from XNEW it should probably
> be released by XDELETE (even if the two do the same same thing).
In that case, I believe that all other instances of free (which free
something allocated by either XNEW or XRESIZEVEC) should be changed to
XDELETE. The following patch does that (created against the latest
master, without my patches applied).
From 160f696db8e875ab89d6c383ec7f68a772157089 Mon Sep 17 00:00:00 2001
From: Asher Gordon <asd...@posteo.net>
Date: Mon, 8 Jun 2020 20:59:38 -0400
Subject: [PATCH] Replace free with XDELETE.
gcc/c/ChangeLog:
* c-typeck.c (free_all_tagged_tu_seen_up_to): Replace free
with XDELETE.
(finish_init): Likewise.
(pop_init_level): Likewise.
---
gcc/c/c-typeck.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 385bf3a1c7b..d97dcf50374 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1404,7 +1404,7 @@ free_all_tagged_tu_seen_up_to (const struct tagged_tu_seen_cache *tu_til)
const struct tagged_tu_seen_cache *const tu1
= (const struct tagged_tu_seen_cache *) tu;
tu = tu1->next;
- free (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
+ XDELETE (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
}
tagged_tu_seen_base = tu_til;
}
@@ -8268,13 +8268,13 @@ finish_init (void)
{
struct constructor_stack *q = constructor_stack;
constructor_stack = q->next;
- free (q);
+ XDELETE (q);
}
gcc_assert (!constructor_range_stack);
/* Pop back to the data of the outer initializer (if any). */
- free (spelling_base);
+ XDELETE (spelling_base);
constructor_decl = p->decl;
require_constant_value = p->require_constant_value;
@@ -8287,7 +8287,7 @@ finish_init (void)
spelling_size = p->spelling_size;
constructor_top_level = p->top_level;
initializer_stack = p->next;
- free (p);
+ XDELETE (p);
}
/* Call here when we see the initializer is surrounded by braces.
@@ -8818,7 +8818,7 @@ pop_init_level (location_t loc, int implicit,
RESTORE_SPELLING_DEPTH (constructor_depth);
constructor_stack = p->next;
- free (p);
+ XDELETE (p);
if (ret.value == NULL_TREE && constructor_stack == 0)
ret.value = error_mark_node;
--
2.26.2
> I realize the patch doesn't change the text of the message but... > if the name of the field is known at this point, mentioning it in > the message would be a helpful touch. If it isn't, then "a field" > would be grammatically more correct. In addition, pointing to > the definition of the struct in a follow-on note would also be > helpful (and in line with other similar messages). These > improvements are unrelated to the change you're making so could > be made separately, but since you're already making changes here > it's an opportunity to make them now (otherwise they're unlikely > to happen). I will look into this. Thanks, Asher Footnotes: ¹ https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547460.html -- One picture is worth 128K words. -------- I prefer to send and receive mail encrypted. Please send me your public key, and if you do not have my public key, please let me know. Thanks. GPG fingerprint: 38F3 975C D173 4037 B397 8095 D4C9 C4FC 5460 8E68
signature.asc
Description: PGP signature
