Hi Martin,
I don't know how close my patch is to being finished.
But it seems like unless we need a separate warning for Winit_self, we
should probably just combine that code with mine and just give a
" 'i' is uninitialized " warning, rather than " 'i' is initialized
with itself ".
I've attached my patch here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19808
If we need to detect multiple uninitialized values in an initializer
we need something other than walk_tree I think, since it just returns
the first match.
I'll submit it to gcc-patches for comments later.
What is EXPR_LOC_OR_LOC?
On Fri, Nov 13, 2015 at 4:12 PM, Martin Sebor <msebor@xxxxxxxxx> wrote:
> On 11/13/2015 04:30 AM, Manuel López-Ibáñez wrote:
>>
>> Hi Anthony,
>>
>> Would you mind attaching your draft patch to the PR? You could also
>> submit it to gcc-patches with a "[RFC, C++]" note in the subject to get
>> some early feedback on it.
>>
>> I think Martin Sebor has recently fixed the i(i) case (or improved it).
>
>
> I just posted a patch adjusting the location to point at the member
> being initialized:
> https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01420.html
> The patch hasn't been committed or even approved yet. It looks like
> Anthony's fixed it in his own patch so I might as well withdraw mine
> if you're close to being done. For reference, I also opened bug 68301
> for an outstanding problem in this area.
>
> For the case below, even though it might look like it will lead to
> a lot of duplicitous output, I would think that simply diagnosing
> every instance of using an uninitialized member would be fine in
> practice. That's what Clang does:
>
> u.cpp:5:18: warning: field 'j' is uninitialized when used here
> [-Wuninitialized]
> S() : i(j), j(1) {}
> ^
> u.cpp:11:18: warning: field 'j' is uninitialized when used here
> [-Wuninitialized]
> B() : i(j+i), j(j+1) {}
> ^
> u.cpp:11:20: warning: field 'i' is uninitialized when used here
> [-Wuninitialized]
> B() : i(j+i), j(j+1) {}
> ^
> u.cpp:11:26: warning: field 'j' is uninitialized when used here
> [-Wuninitialized]
> B() : i(j+i), j(j+1) {}
> ^
> u.cpp:17:18: warning: field 'i' is uninitialized when used here
> [-Wuninitialized]
> C() : i(i) {}
> ^
>
> Martin
>
>
>>
>> Cheers,
>>
>> Manuel.
>>
>> On 11/12/2015 10:12 PM, Anthony Brandon wrote:
>>>
>>> Hi,
>>>
>>> I found the code from when I worked on 19808.
>>> With this input:
>>>
>>> struct S
>>> {
>>> int i, j;
>>> S() : i(j), j(1) {}
>>> };
>>>
>>> struct B
>>> {
>>> int i, j;
>>> B() : i(j+i), j(j+1) {}
>>> };
>>>
>>> struct C
>>> {
>>> int i, j;
>>> C() : i(i) {}
>>> };
>>>
>>> I get this output:
>>>
>>> test.C:4:10: warning: ‘S::i’ is initialized with uninitialized field
>>> ‘S::j’ [-Wuninitialized]
>>> S() : i(j), j(1) {}
>>> ^
>>>
>>> test.C:10:10: warning: ‘B::i’ is initialized with uninitialized field
>>> ‘B::j’ [-Wuninitialized]
>>> B() : i(j+i), j(j+1) {}
>>> ^
>>>
>>> test.C:16:10: warning: ‘C::i’ is initialized with itself [-Winit-self]
>>> C() : i(i), j(1) {}
>>> ^
>>> The main questions I have are what to do in cases like
>>> i(i+j) and the like, or where multiple uninitialized values are used,
>>> or i(i+1) for that matter.
>>>
>>>
>>> On Tue, Nov 10, 2015 at 12:40 AM, Manuel López-Ibáñez
>>> <manuel.lopez-ibanez@xxxxxxxxxxxxxxxx> wrote:
>>>>
>>>> On 09/11/15 20:41, Zygmunt Ptak wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Is there any param in the gcc which will warn about not initialized
>>>>> class member from the initialization list?
>>>>
>>>>
>>>>
>>>> Unfortunately, no. We do not even warn for:
>>>>
>>>> struct S
>>>> {
>>>> int i, j;
>>>> S() : i(j), j(1) {}
>>>> }
>>>>
>>>> This is https://gcc.gnu.org/PR19808 and it should be not too
>>>> difficult to
>>>> fix, it just needs someone with enough time and perseverance to fix it.
>>>> Anthony Brandon started working on it, but I'm not sure what is the
>>>> status
>>>> now. Of course, anyone is more than welcome to pick it up.
>>>>
>>>> There is also https://gcc.gnu.org/PR2972, which is probably closer to
>>>> what
>>>> you want. The current patch
>>>> (https://gcc.gnu.org/ml/gcc-patches/2011-11/msg01068.html) will warn
>>>> even if
>>>> the member is initialized within the constructor. But if this is what
>>>> you
>>>> want, you could try updating the patch to the latest trunk, complete
>>>> it and
>>>> submit it for approval.
>>>>
>>>> Cheers,
>>>>
>>>> Manuel.
>>>
>>>
>>>
>>>
>>
>
--
Anthony
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 2e11acb..680ca83 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -38,7 +38,7 @@ static tree finish_init_stmts (bool, tree, tree);
static void construct_virtual_base (tree, tree);
static void expand_aggr_init_1 (tree, tree, tree, tree, int, tsubst_flags_t);
static void expand_default_init (tree, tree, tree, tree, int, tsubst_flags_t);
-static void perform_member_init (tree, tree);
+static void perform_member_init (tree, tree, vec<tree> *);
static int member_init_ok_or_else (tree, tree, tree);
static void expand_virtual_init (tree, tree);
static tree sort_mem_initializers (tree, tree);
@@ -596,12 +596,46 @@ get_nsdmi (tree member, bool in_ctor)
return init;
}
+/* Search for a usage of an uninitialized member of the current class.
+ For use in perform_member_init. */
+
+static tree
+find_uninitialized_member (tree *tp, int *walk_subtrees, void *data)
+{
+ vec<tree> *inited = (vec<tree> *)data;
+ if (TREE_CODE (*tp) == COMPONENT_REF)
+ {
+ /* Only look at the top most COMPONENT_REF to ignore members of parent
+ classes or structs. */
+ if (!(TREE_OPERAND (*tp, 0) == current_class_ref))
+ {
+ *walk_subtrees = 0;
+ return NULL_TREE;
+ }
+ /* Search the vector of previously seen members to determine if one is
+ being used before being initialized. */
+ tree ptr;
+ bool found = false;
+ for (int ix = 0; inited->iterate (ix, &ptr); ix++)
+ {
+ if (ptr == TREE_OPERAND(*tp,1))
+ {
+ found = true;
+ break;
+ }
+ }
+ if (!found)
+ return *tp;
+ }
+ return NULL_TREE;
+}
+
/* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
arguments. If TREE_LIST is void_type_node, an empty initializer
list was given; if NULL_TREE no initializer was given. */
static void
-perform_member_init (tree member, tree init)
+perform_member_init (tree member, tree init, vec<tree> *inited)
{
tree decl;
tree type = TREE_TYPE (member);
@@ -638,10 +672,25 @@ perform_member_init (tree member, tree init)
val = TREE_OPERAND (val, 0);
if (TREE_CODE (val) == COMPONENT_REF && TREE_OPERAND (val, 1) == member
&& TREE_OPERAND (val, 0) == current_class_ref)
- warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+ warning_at (EXPR_LOCATION (val),
OPT_Winit_self, "%qD is initialized with itself",
member);
}
+ if (warn_uninitialized && init && TREE_CODE (init) == TREE_LIST
+ && TREE_CHAIN (init) == NULL_TREE)
+ {
+ tree res = walk_tree_without_duplicates(&init, find_uninitialized_member,
+ inited);
+ if (res != NULL_TREE && !(TREE_CODE (res) == COMPONENT_REF &&
+ TREE_OPERAND (res, 1) == member &&
+ TREE_OPERAND (res, 0) == current_class_ref))
+ {
+ warning_at (EXPR_LOCATION(res),
+ OPT_Wuninitialized,
+ "%qD is initialized with uninitialized field %qD",
+ member, TREE_OPERAND(res,1));
+ }
+ }
if (init == void_type_node)
{
@@ -1170,12 +1219,16 @@ emit_mem_initializers (tree mem_inits)
initialize_vtbl_ptrs (current_class_ptr);
/* Initialize the data members. */
+ vec<tree> initialized;
+ initialized.create(0);
while (mem_inits)
{
perform_member_init (TREE_PURPOSE (mem_inits),
- TREE_VALUE (mem_inits));
+ TREE_VALUE (mem_inits), &initialized);
+ initialized.safe_push(TREE_PURPOSE(mem_inits));
mem_inits = TREE_CHAIN (mem_inits);
}
+ initialized.release();
}
/* Returns the address of the vtable (i.e., the value that should be
