| Message ID | 4C4664DB.6040902@oracle.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 21 Jul 2010, Shujing Zhao wrote: > Index: c-decl.c > =================================================================== > --- c-decl.c (revision 162361) > +++ c-decl.c (working copy) > @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara > return tem; > } > > +/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE. > + DECL is the original declaration. */ This comment is unclear. What you seem to mean is: TYPE is the type of an uninitialized object DECL. If that object has a const member, diagnose this. Why do you need to recurse down among fields? Why isn't checking C_TYPE_FIELDS_READONLY enough? What if the original uninitialized object is not a structure but an array of structures? Will you diagnose things in that case? You don't appear to have any testcases where the recursion is needed for diagnosis, or where the member in question is an array, only tests directly involving a const scalar element of the structure or union.
On 07/27/2010 09:58 PM, Joseph S. Myers wrote: > On Wed, 21 Jul 2010, Shujing Zhao wrote: > >> Index: c-decl.c >> =================================================================== >> --- c-decl.c (revision 162361) >> +++ c-decl.c (working copy) >> @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara >> return tem; >> } >> >> +/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE. >> + DECL is the original declaration. */ > > This comment is unclear. What you seem to mean is: TYPE is the type of an > uninitialized object DECL. If that object has a const member, diagnose > this. > Thanks. > Why do you need to recurse down among fields? Why isn't checking > C_TYPE_FIELDS_READONLY enough? > It used to notice which field should be initialized, just like the diagnostics at c++. If the notice is not very necessary, C_TYPE_FIELDS_READONLY is enough. > What if the original uninitialized object is not a structure but an array > of structures? Will you diagnose things in that case? C++ would not error an uninitialized array of structure that has a const member. So I think it doesn't need warn at C? > > You don't appear to have any testcases where the recursion is needed for > diagnosis, or where the member in question is an array, only tests > directly involving a const scalar element of the structure or union. > Yes, they should be added. Pearly
On Wed, 28 Jul 2010, Shujing Zhao wrote: > > Why do you need to recurse down among fields? Why isn't checking > > C_TYPE_FIELDS_READONLY enough? > > > It used to notice which field should be initialized, just like the diagnostics > at c++. If the notice is not very necessary, C_TYPE_FIELDS_READONLY is enough. You could always check C_TYPE_FIELDS_READONLY before recursing to find the field in question. > > What if the original uninitialized object is not a structure but an array of > > structures? Will you diagnose things in that case? > > C++ would not error an uninitialized array of structure that has a const > member. So I think it doesn't need warn at C? Could you please give references to the relevant sections of the C++ standard that explain exactly what is or is not permitted in this regard?
2010/7/28 Shujing Zhao <pearly.zhao@oracle.com>: > On 07/27/2010 09:58 PM, Joseph S. Myers wrote: >> >> On Wed, 21 Jul 2010, Shujing Zhao wrote: >> >>> Index: c-decl.c >>> =================================================================== >>> --- c-decl.c (revision 162361) >>> +++ c-decl.c (working copy) >>> @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara >>> return tem; >>> } >>> +/* Subroutine of finish_decl. Diagnose uninitialized const member of >>> type TYPE. >>> + DECL is the original declaration. */ >> >> This comment is unclear. What you seem to mean is: TYPE is the type of an >> uninitialized object DECL. If that object has a const member, diagnose >> this. >> > Thanks. >> >> Why do you need to recurse down among fields? Why isn't checking >> C_TYPE_FIELDS_READONLY enough? >> > It used to notice which field should be initialized, just like the > diagnostics at c++. If the notice is not very necessary, > C_TYPE_FIELDS_READONLY is enough. > >> What if the original uninitialized object is not a structure but an array >> of structures? Will you diagnose things in that case? > > C++ would not error an uninitialized array of structure that has a const > member. So I think it doesn't need warn at C? It seems to be PR c++/43719, which should be fixed on mainline.
Index: c-decl.c =================================================================== --- c-decl.c (revision 162361) +++ c-decl.c (working copy) @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara return tem; } +/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE. + DECL is the original declaration. */ + +static void +diagnose_uninitialized_cst_member (tree decl, tree type) +{ + tree field; + for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field)) + { + tree field_type; + if (TREE_CODE (field) != FIELD_DECL) + continue; + field_type = strip_array_types (TREE_TYPE (field)); + + if (TYPE_QUALS (field_type) & TYPE_QUAL_CONST) + { + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat, + "uninitialized const member in %qT is invalid in C++", + TREE_TYPE (decl)); + inform (DECL_SOURCE_LOCATION (field), "%qD should be initialized", field); + } + + if (TREE_CODE (field_type) == RECORD_TYPE + || TREE_CODE (field_type) == UNION_TYPE) + diagnose_uninitialized_cst_member (decl, field_type); + } +} + + /* Finish processing of a declaration; install its initial value. If ORIGTYPE is not NULL_TREE, it is the original type of INIT. @@ -4420,11 +4449,16 @@ finish_decl (tree decl, location_t init_ if (warn_cxx_compat && TREE_CODE (decl) == VAR_DECL - && TREE_READONLY (decl) && !DECL_EXTERNAL (decl) && DECL_INITIAL (decl) == NULL_TREE) - warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat, - "uninitialized const %qD is invalid in C++", decl); + { + if (TREE_READONLY (decl)) + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat, + "uninitialized const %qD is invalid in C++", decl); + else if (TREE_CODE (type) == RECORD_TYPE + || TREE_CODE (type) == UNION_TYPE) + diagnose_uninitialized_cst_member (decl, type); + } } /* Given a parsed parameter declaration, decode it into a PARM_DECL. */ Index: testsuite/gcc.dg/Wcxx-compat-20.c =================================================================== --- testsuite/gcc.dg/Wcxx-compat-20.c (revision 0) +++ testsuite/gcc.dg/Wcxx-compat-20.c (revision 0) @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-Wc++-compat" } */ +typedef struct s { const int i; } s; /* { dg-message "should be initialized" } */ +void foo () { s v; } /* { dg-warning "uninitialized const member in" } */ + +union u {const int a; double b;}; /* { dg-message "should be initialized" } */ +void bar () { union u f; } /* { dg-warning "uninitialized const member in" } */