Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3
Hi Joseph, Many thanks for giving us your feedback. On Tue, Aug 20, 2019 at 3:57 AM Joseph Myers wrote: > On Tue, 30 Jul 2019, Martin Sebor wrote: > > > On 7/30/19 1:13 AM, Akshat Garg wrote: > > > Hi, > > > This patch includes C front-end code for a type qualifier > _Dependent_ptr. > > > > Just some very high-level comments/questions. I only followed > > the _Dependent_ptr discussion from a distance and I'm likely > > missing some context so the first thing I looked for in this > > patch is documentation of the new qualifier. Unless it's > > The first question for any new thing that is syntactically a qualifier is: > is it intended generally to be counted as a qualifier where the standard > refers to qualified type, the unqualified version of a type, etc.? Or is > it, like _Atomic, a qualifier only syntactically and generally excluded > from references to qualifiers? > Can you help me in understanding why the _Atomic is excluded from the standard references. I referred to the C standard draft N2310 ( http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf) but I couldn't understand how it is excluded? I want to know what properties should a qualifier have to be in standard qualifiers list? Thanks, Akshat > > For the _Atomic implementation I had to go through all the references to > qualifiers or TYPE_MAIN_VARIANT in the front end and consider in each case > whether it handled _Atomic correctly, given that _Atomic is not counted as > a qualifier in the standard (so the unqualified version of const _Atomic > int is _Atomic int not int, and so can't be derived simply by using > TYPE_MAIN_VARIANT, for example). Some cases didn't need changing because > the handling (e.g. diagnostic for different types) was still appropriate > for _Atomic even though not formally a qualifier, but plenty did need > changing and associated tests added. > > Such a check of front end code is probably unavoidable (before a change is > ready for trunk, not necessarily for an initial rough RFC patch) for any > new qualifier, whether it counts as a qualifier in standard terms or not > (and the patch reviewer will need to do their own check of references to > qualifiers or TYPE_MAIN_VARIANT that didn't get changed by the patch), but > the answer to that question helps indicate whether the default is to > expect code to need changing for the new qualifier or not. > > > you point to it? (In that case, or if a proposal is planned, > > the feature should probably either only be available with > > -std=c2x and -std=gnu2x or a pedantic warning should be issued > > There should not be any -std=c2x (flag_isoc2x) conditionals simply based > on "a proposal is planned". flag_isoc2x conditionals (pedwarn_c11 calls, > etc.) should be for cases where a feature is *accepted and committed into > the C2x branch of Jens's git repository for the C standard*, not for > something that might be proposed, or is proposed, but doesn't yet have > specific text integrated into the text of the standard. > > If something is simply proposed *and we've concluded it's a good feature > to have as an extension in any case* then you have a normal > pedwarn-if-pedantic (no condition on standard version) as for any GNU > extension (and flag_isoc2x conditions / changes to use pedwarn_c11 instead > can be added later if the extension is added to the standard). > > -- > Joseph S. Myers > jos...@codesourcery.com >
Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3
at issuing
> a pedantic warning and having the name in the implementation namepace
> is enough but others (the C FE maintainers) will know better.)
>
> Other than that, I would also expect to see more extensive test
> coverage, at a minimum to exercise error detection (invalid uses,
> conversions, etc.). For example, I see the code that rejects it
> but no tests for declaring, say, a _Dependent_ptr-qualified integer.
> What I don't think I see is code that rejects _Dependent_ptr-qualified
> function pointers (AFAIK, POINTER_TYPE_P evaluates to nonzero for both).
> Is that intended? (And if so, what does it mean?)I also see that
> the new tests look for warnings but it's not clear to me that that's
> their intent or that the dg-warning directives are being used to
> "suppress" warnings for issues in the tests. For instance, this
> is common:
>
>rcu_assign_pointer (gp,p); /* { dg-warning
> "\\\[-Wincompatible-pointer-types]" } */
>
> but neither gp nor p is a _Dependent_ptr. (I may be missing
> the purpose of the compile-only tests. E.g., the only thing
> p0190r4_fig10.c seems to be exercising besides that the _Dependent_ptr
> qualifier is accepted is a couple of warnings, one of which is the one
> above.) I would suggest to look at tests for other qualifiers for
> examples and model the new ones after those.
>
> I'm also wondering how the new qualifier interacts with others like
> const. Should all combinations of qualifiers be accepted and do
> they affect the semantics in any interesting way? This is probably
> something to cover in the proposal.
>
> Martin
>
> >
> > Thanx, Paul
> >
> > On Tue, Jul 30, 2019 at 12:46 PM Martin Sebor wrote:
> >>
> >> On 7/30/19 1:13 AM, Akshat Garg wrote:
> >>> Hi,
> >>> This patch includes C front-end code for a type qualifier
> _Dependent_ptr.
> >>
> >> Just some very high-level comments/questions. I only followed
> >> the _Dependent_ptr discussion from a distance and I'm likely
> >> missing some context so the first thing I looked for in this
> >> patch is documentation of the new qualifier. Unless it's
> >> a proposed C2x feature that I missed I would expect to find it
> >> in section 6 - Extensions to the C Language Family of the manual.
> >> I saw the references to WG21's p0190r4 in the tests but the paper
> >> doesn't mention _Dependent_ptr, and I found no references to a C
> >> paper that does. If it has been proposed for C2X as well can
> >> you point to it? (In that case, or if a proposal is planned,
> >> the feature should probably either only be available with
> >> -std=c2x and -std=gnu2x or a pedantic warning should be issued
> >> for its use in earlier modes similarly to how uses of _Atomic
> >> are diagnosed in pre-C11 modes.)
> >>
> >> Martin
> >>
> >>> The patch has been tested using the following
> >>> make bootstrap -j 4
> >>> make -k check -j 4
> >>>
> >>> on x86_64-linux-gnu.
> >>>
> >>> Thanks,
> >>> Akshat
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> 2019-07-30 Akshat Garg
> >>>
> >>> * c-family/c-common.c (struct c_common_resword
> c_common_reswords):
> >>> Add "_Dependent_ptr".
> >>> (c_apply_type_quals_to_decl): Set the flag for
> _Dependent_ptr if
> >>> qualified.
> >>> (keyword_is_type_qualifier): Add RID_DEPENDENT_PTR.
> >>> * c-family/c-common.h (enum rid): Add RID_DEPENDENT_PTR.
> >>> * c-family/c-format.c (check_format_types): Add dependent
> pointer
> >>> as a qualifier check.
> >>> * c-family/c-pretty-print.c (pp_c_cv_qualifiers): Handle
> dependent
> >>> pointer qualifier.
> >>> * c/c-aux-info.c (gen_type): Handle dependent pointer
> qualifier.
> >>> (gen_decl): Handle dependent pointer qualifier.
> >>> * c/c-decl.c (merge_decls): Set old declaration as having
> dependent
> >>> pointer qualification if new declaration has one.
> >>> (shadow_tag_warned): Add dependent_ptr_p to declspecs check.
> >>> (quals_from_declspecs): Add dependent_ptr_p to declspecs
> check.
> >>> (grokdeclarator): Add checks for dependent pointer qualifier
> and
> >>> warn of duplicate or errors. Allow dependent pointer for pointer types
> on
Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3
On Fri, Aug 2, 2019 at 10:14 AM Akshat Garg wrote: > > > On Thu, Aug 1, 2019 at 8:57 PM Paul McKenney wrote: > >> Excellent point, this discussion needs to be made official. >> Please see attached for an initial draft of a working paper. >> >> Thoughts? >> >> Thanx, Paul >> > Please, find the attachment here ( > https://drive.google.com/open?id=0B9Q3hzI3TofcZ3o2aXVMd2V1Ujl4VUZnT3MtXzZpV1I2OHFv > ). > >> >> On Tue, Jul 30, 2019 at 12:46 PM Martin Sebor wrote: >> > >> > On 7/30/19 1:13 AM, Akshat Garg wrote: >> > > Hi, >> > > This patch includes C front-end code for a type qualifier >> _Dependent_ptr. >> > >> > Just some very high-level comments/questions. I only followed >> > the _Dependent_ptr discussion from a distance and I'm likely >> > missing some context so the first thing I looked for in this >> > patch is documentation of the new qualifier. Unless it's >> > a proposed C2x feature that I missed I would expect to find it >> > in section 6 - Extensions to the C Language Family of the manual. >> > I saw the references to WG21's p0190r4 in the tests but the paper >> > doesn't mention _Dependent_ptr, and I found no references to a C >> > paper that does. If it has been proposed for C2X as well can >> > you point to it? (In that case, or if a proposal is planned, >> > the feature should probably either only be available with >> > -std=c2x and -std=gnu2x or a pedantic warning should be issued >> > for its use in earlier modes similarly to how uses of _Atomic >> > are diagnosed in pre-C11 modes.) >> > >> > Martin >> > I have tried this patch which raises pedantic warning with pre-gnu2x modes. Let us know what you think. diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 9e80bfee7d4..4d61a38c5b9 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2870,6 +2870,12 @@ c_parser_declspecs (c_parser *parser, struct c_declspecs *specs, declspecs_add_qual (loc, specs, value); break; case RID_DEPENDENT_PTR: + if (flag_isoc11) + pedwarn_c11 (loc, OPT_Wpedantic, +"ISO C11 does not support the %<_Dependent_ptr%> qualifier"); + else + pedwarn_c99 (loc, OPT_Wpedantic, +"ISO C90 does not support the %<_Dependent_ptr%> qualifier"); attrs_ok = true; declspecs_add_qual (loc, specs, c_parser_peek_token (parser)->value); c_parser_consume_token (parser); > > >> > > The patch has been tested using the following >> > > make bootstrap -j 4 >> > > make -k check -j 4 >> > > >> > > on x86_64-linux-gnu. >> > > >> > > Thanks, >> > > Akshat >> > > >> > > gcc/ChangeLog: >> > > >> > > 2019-07-30 Akshat Garg >> > > >> > > * c-family/c-common.c (struct c_common_resword >> c_common_reswords): >> > > Add "_Dependent_ptr". >> > > (c_apply_type_quals_to_decl): Set the flag for >> _Dependent_ptr if >> > > qualified. >> > > (keyword_is_type_qualifier): Add RID_DEPENDENT_PTR. >> > > * c-family/c-common.h (enum rid): Add RID_DEPENDENT_PTR. >> > > * c-family/c-format.c (check_format_types): Add dependent >> pointer >> > > as a qualifier check. >> > > * c-family/c-pretty-print.c (pp_c_cv_qualifiers): Handle >> dependent >> > > pointer qualifier. >> > > * c/c-aux-info.c (gen_type): Handle dependent pointer >> qualifier. >> > > (gen_decl): Handle dependent pointer qualifier. >> > > * c/c-decl.c (merge_decls): Set old declaration as having >> dependent >> > > pointer qualification if new declaration has one. >> > > (shadow_tag_warned): Add dependent_ptr_p to declspecs check. >> > > (quals_from_declspecs): Add dependent_ptr_p to declspecs >> check. >> > > (grokdeclarator): Add checks for dependent pointer qualifier >> and >> > > warn of duplicate or errors. Allow dependent pointer for pointer >> types only. >> > > * c/c-parser.c (c_keyword_starts_typename, >> c_token_is_qualifier, >> > > c_token_starts_declspecs): Add RID_DEPENDENT_PTR. >> > > (c_parser_static_assert_declaration_no_semi): Add >> _Dependent_ptr >>
Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3
On Thu, Aug 1, 2019 at 8:57 PM Paul McKenney wrote: > Excellent point, this discussion needs to be made official. > Please see attached for an initial draft of a working paper. > > Thoughts? > > Thanx, Paul > Please, find the attachment here ( https://drive.google.com/open?id=0B9Q3hzI3TofcZ3o2aXVMd2V1Ujl4VUZnT3MtXzZpV1I2OHFv ). > > On Tue, Jul 30, 2019 at 12:46 PM Martin Sebor wrote: > > > > On 7/30/19 1:13 AM, Akshat Garg wrote: > > > Hi, > > > This patch includes C front-end code for a type qualifier > _Dependent_ptr. > > > > Just some very high-level comments/questions. I only followed > > the _Dependent_ptr discussion from a distance and I'm likely > > missing some context so the first thing I looked for in this > > patch is documentation of the new qualifier. Unless it's > > a proposed C2x feature that I missed I would expect to find it > > in section 6 - Extensions to the C Language Family of the manual. > > I saw the references to WG21's p0190r4 in the tests but the paper > > doesn't mention _Dependent_ptr, and I found no references to a C > > paper that does. If it has been proposed for C2X as well can > > you point to it? (In that case, or if a proposal is planned, > > the feature should probably either only be available with > > -std=c2x and -std=gnu2x or a pedantic warning should be issued > > for its use in earlier modes similarly to how uses of _Atomic > > are diagnosed in pre-C11 modes.) > > > > Martin > > > > > The patch has been tested using the following > > > make bootstrap -j 4 > > > make -k check -j 4 > > > > > > on x86_64-linux-gnu. > > > > > > Thanks, > > > Akshat > > > > > > gcc/ChangeLog: > > > > > > 2019-07-30 Akshat Garg > > > > > > * c-family/c-common.c (struct c_common_resword > c_common_reswords): > > > Add "_Dependent_ptr". > > > (c_apply_type_quals_to_decl): Set the flag for _Dependent_ptr > if > > > qualified. > > > (keyword_is_type_qualifier): Add RID_DEPENDENT_PTR. > > > * c-family/c-common.h (enum rid): Add RID_DEPENDENT_PTR. > > > * c-family/c-format.c (check_format_types): Add dependent > pointer > > > as a qualifier check. > > > * c-family/c-pretty-print.c (pp_c_cv_qualifiers): Handle > dependent > > > pointer qualifier. > > > * c/c-aux-info.c (gen_type): Handle dependent pointer > qualifier. > > > (gen_decl): Handle dependent pointer qualifier. > > > * c/c-decl.c (merge_decls): Set old declaration as having > dependent > > > pointer qualification if new declaration has one. > > > (shadow_tag_warned): Add dependent_ptr_p to declspecs check. > > > (quals_from_declspecs): Add dependent_ptr_p to declspecs > check. > > > (grokdeclarator): Add checks for dependent pointer qualifier > and > > > warn of duplicate or errors. Allow dependent pointer for pointer types > only. > > > * c/c-parser.c (c_keyword_starts_typename, > c_token_is_qualifier, > > > c_token_starts_declspecs): Add RID_DEPENDENT_PTR. > > > (c_parser_static_assert_declaration_no_semi): Add > _Dependent_ptr > > > qualifier in comments. > > > (c_parser_declspecs, c_parser_attribute_any_word): Add > > > RID_DEPENDENT_PTR. > > > * c/c-tree.h (C_TYPE_FIELDS_DEPENDENT_PTR): New macro to mark > > > dependent pointer. > > > (enum c_declspec_word): Add cdw_dependent_ptr. > > > (struct c_declspecs): Add dependent_ptr_p field. > > > * print-tree.c (print_node): Print dependent_ptr qualifier. > > > * tree-core.hi (enum cv_qualifier): Add > TYPE_QUAL_DEPENDENT_PTR. > > > (enum tree_index): Add TI_DEPENDENT_PTR_TYPE. > > > (struct tree_base): Add dependent_ptr_flag field. > > > * tree-pretty-print.c (dump_generic_node): Print dependent > pointer > > > type qualifier. > > > * tree.c (fld_type_variant, set_type_quals): Set > TYPE_DEPENDENT_PTR. > > > * tree.h (TREE_THIS_DEPENDENT_PTR): New macro. To access > > > dependent_ptr_flag field in tree_base. > > > (TYPE_DEPENDENT_PTR): New accessor macro. > > > (TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add > TYPE_QUAL_DEPENDENT_PTR. > > > (dependent_ptrTI_type_node): Add new type node. >
[PATCH] Adding _Dependent_ptr type qualifier in C part 1/3
Hi,
This patch includes C front-end code for a type qualifier _Dependent_ptr.
The patch has been tested using the following
make bootstrap -j 4
make -k check -j 4
on x86_64-linux-gnu.
Thanks,
Akshat
gcc/ChangeLog:
2019-07-30 Akshat Garg
* c-family/c-common.c (struct c_common_resword c_common_reswords):
Add "_Dependent_ptr".
(c_apply_type_quals_to_decl): Set the flag for _Dependent_ptr if
qualified.
(keyword_is_type_qualifier): Add RID_DEPENDENT_PTR.
* c-family/c-common.h (enum rid): Add RID_DEPENDENT_PTR.
* c-family/c-format.c (check_format_types): Add dependent pointer
as a qualifier check.
* c-family/c-pretty-print.c (pp_c_cv_qualifiers): Handle dependent
pointer qualifier.
* c/c-aux-info.c (gen_type): Handle dependent pointer qualifier.
(gen_decl): Handle dependent pointer qualifier.
* c/c-decl.c (merge_decls): Set old declaration as having dependent
pointer qualification if new declaration has one.
(shadow_tag_warned): Add dependent_ptr_p to declspecs check.
(quals_from_declspecs): Add dependent_ptr_p to declspecs check.
(grokdeclarator): Add checks for dependent pointer qualifier and
warn of duplicate or errors. Allow dependent pointer for pointer types only.
* c/c-parser.c (c_keyword_starts_typename, c_token_is_qualifier,
c_token_starts_declspecs): Add RID_DEPENDENT_PTR.
(c_parser_static_assert_declaration_no_semi): Add _Dependent_ptr
qualifier in comments.
(c_parser_declspecs, c_parser_attribute_any_word): Add
RID_DEPENDENT_PTR.
* c/c-tree.h (C_TYPE_FIELDS_DEPENDENT_PTR): New macro to mark
dependent pointer.
(enum c_declspec_word): Add cdw_dependent_ptr.
(struct c_declspecs): Add dependent_ptr_p field.
* print-tree.c (print_node): Print dependent_ptr qualifier.
* tree-core.hi (enum cv_qualifier): Add TYPE_QUAL_DEPENDENT_PTR.
(enum tree_index): Add TI_DEPENDENT_PTR_TYPE.
(struct tree_base): Add dependent_ptr_flag field.
* tree-pretty-print.c (dump_generic_node): Print dependent pointer
type qualifier.
* tree.c (fld_type_variant, set_type_quals): Set TYPE_DEPENDENT_PTR.
* tree.h (TREE_THIS_DEPENDENT_PTR): New macro. To access
dependent_ptr_flag field in tree_base.
(TYPE_DEPENDENT_PTR): New accessor macro.
(TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add TYPE_QUAL_DEPENDENT_PTR.
(dependent_ptrTI_type_node): Add new type node.
gcc/testsuite/ChangeLog:
2019-07-30 Akshat Garg
* gcc.dg/c11-dependent_ptr-test-1.c: New test for _Dependent_ptr
qualifier.
* gcc.dg/{p0190r4_fig8, p0190r4_fig9, p0190r4_fig10, p0190r4_fig11,
p0190r4_fig12, p0190r4_fig13}.c: New tests from document P0190R4 for
_Dependent_ptr qualifier.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index cb92710..4f09037 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -345,6 +345,7 @@ const struct c_common_resword c_common_reswords[] =
{ "_Alignas", RID_ALIGNAS, D_CONLY },
{ "_Alignof", RID_ALIGNOF, D_CONLY },
{ "_Atomic", RID_ATOMIC,D_CONLY },
+ { "_Dependent_ptr", RID_DEPENDENT_PTR, 0 },
{ "_Bool", RID_BOOL, D_CONLY },
{ "_Complex", RID_COMPLEX, 0 },
{ "_Imaginary", RID_IMAGINARY, D_CONLY },
@@ -3546,6 +3547,11 @@ c_apply_type_quals_to_decl (int type_quals, tree
decl)
TREE_SIDE_EFFECTS (decl) = 1;
TREE_THIS_VOLATILE (decl) = 1;
}
+ if (type_quals & TYPE_QUAL_DEPENDENT_PTR)
+{
+ TREE_SIDE_EFFECTS (decl) = 1;
+ TREE_THIS_DEPENDENT_PTR (decl) = 1;
+}
if (type_quals & TYPE_QUAL_RESTRICT)
{
while (type && TREE_CODE (type) == ARRAY_TYPE)
@@ -7851,6 +7857,7 @@ keyword_is_type_qualifier (enum rid keyword)
case RID_VOLATILE:
case RID_RESTRICT:
case RID_ATOMIC:
+case RID_DEPENDENT_PTR:
return true;
default:
return false;
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 117d729..ab55882 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -68,7 +68,7 @@ enum rid
RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN,
RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE,
RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT,
- RID_NORETURN, RID_ATOMIC,
+ RID_NORETURN, RID_ATOMIC, RID_DEPENDENT_PTR,
/* C extensions */
RID_COMPLEX, RID_THREAD, RID_SAT,
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index d134116..00769bb 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -4172,6 +4172,7 @@ check_format_types (const substring_loc _loc,
&& (TYPE_READONLY (cur_type)
|| TYPE_VOLATILE (cur_type)
|| TYPE_ATOMIC (cur_type)
+ || TYPE_DEPENDENT_PTR (cur_type)
|| TYPE_RESTRICT (cur_type)))
warning (OPT_Wformat_, "extra type qualifiers in format &
