| Message ID | 68b46c41-a333-7273-fe12-ae395d642ccb@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | [C] field_decl_cmp | expand |
On Tue, 12 Sep 2017, Nathan Sidwell wrote: > Joseph, > in moving field_decl_cmp to the C FE, I noticed it checks for NULL DECL_NAMES. > Those don't occur. To be clear: they don't occur in the case where field_decl_cmp is used; they can occur in other cases. > This patch removes that checking, and also asserts that when we see > identically named decls, exactly one is a TYPE_DECL. When do you get TYPE_DECLs here, for C? I wouldn't expect them to be possible.
On 09/12/2017 12:06 PM, Joseph Myers wrote: > On Tue, 12 Sep 2017, Nathan Sidwell wrote: >> This patch removes that checking, and also asserts that when we see >> identically named decls, exactly one is a TYPE_DECL. > > When do you get TYPE_DECLs here, for C? I wouldn't expect them to be > possible. > oh, of course (C is so primitive!). This patch survives a bootstrap, ok? nathan
On Tue, 12 Sep 2017, Nathan Sidwell wrote: > On 09/12/2017 12:06 PM, Joseph Myers wrote: > > On Tue, 12 Sep 2017, Nathan Sidwell wrote: > > > > This patch removes that checking, and also asserts that when we see > > > identically named decls, exactly one is a TYPE_DECL. > > > > When do you get TYPE_DECLs here, for C? I wouldn't expect them to be > > possible. > > > > oh, of course (C is so primitive!). This patch survives a bootstrap, ok? I'd be concerned about the possibility of a qsort implementation that calls the comparison function with two pointers to the same object (as far as I can tell, it's valid for qsort to do that). That is, I think you need to check for the two DECLs being the same DECL, before asserting their names are different.
On 09/12/2017 12:48 PM, Joseph Myers wrote: > I'd be concerned about the possibility of a qsort implementation that > calls the comparison function with two pointers to the same object (as far > as I can tell, it's valid for qsort to do that). That is, I think you > need to check for the two DECLs being the same DECL, before asserting > their names are different. I suppose we can drop the assert. That does leave it returning +1 in the case you're concerned about, but I don't really see the need to tell such a stupid qsort that the things are unordered. nathan
On Fri, 15 Sep 2017, Nathan Sidwell wrote: > On 09/12/2017 12:48 PM, Joseph Myers wrote: > > > I'd be concerned about the possibility of a qsort implementation that > > calls the comparison function with two pointers to the same object (as far > > as I can tell, it's valid for qsort to do that). That is, I think you > > need to check for the two DECLs being the same DECL, before asserting > > their names are different. > > I suppose we can drop the assert. That does leave it returning +1 in the case > you're concerned about, but I don't really see the need to tell such a stupid > qsort that the things are unordered. I don't know what such a qsort would do if such a case returned 1; my presumption is that all our comparison functions ought to return 0 when two objects are equal, even if that can only be if they are the same object. It's OK with a return of 0 if x == y (or if DECL_NAME (x) == DECL_NAME (y), whichever you think appropriate).
2017-09-12 Nathan Sidwell <nathan@acm.org> * c-decl.c (field_decl_cmp): Don't handle NULL names. Refactor. Index: c-decl.c =================================================================== --- c-decl.c (revision 252023) +++ c-decl.c (working copy) @@ -7845,19 +7845,17 @@ warn_cxx_compat_finish_struct (tree fiel static int field_decl_cmp (const void *x_p, const void *y_p) { - const tree *const x = (const tree *) x_p; - const tree *const y = (const tree *) y_p; + const tree x = *(const tree *) x_p; + const tree y = *(const tree *) y_p; - if (DECL_NAME (*x) == DECL_NAME (*y)) - /* A nontype is "greater" than a type. */ - return (TREE_CODE (*y) == TYPE_DECL) - (TREE_CODE (*x) == TYPE_DECL); - if (DECL_NAME (*x) == NULL_TREE) - return -1; - if (DECL_NAME (*y) == NULL_TREE) - return 1; - if (DECL_NAME (*x) < DECL_NAME (*y)) - return -1; - return 1; + if (DECL_NAME (x) != DECL_NAME (y)) + return DECL_NAME (x) < DECL_NAME (y) ? -1 : +1; + + /* If the names are the same, exactly one must be a TYPE_DECL, and + that one is less than (before) the other one. */ + gcc_checking_assert ((TREE_CODE (x) == TYPE_DECL) + != (TREE_CODE (y) == TYPE_DECL)); + return TREE_CODE (x) == TYPE_DECL ? -1 : +1; } /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T.