On 11/19/2015 03:54 PM, Anthony Brandon wrote:
Hi,
I'm not that familiar with GCCs trees structures and how to check for
these kinds of cases.
If you have any hints for me for where to look for examples, that
would be appreciated.
I usually look for examples of how similar problems are handled
elsewhere in the front end. But it seems to me that you're on
the right track and might just need to tweak the code a bit.
Some good old fashioned debugging (and lots of time and patience)
should be all you need.
For the second case I'm guessing I'd have to somehow checked that the
constructor is actually taking the value and not the address,
but for the first case I'm not sure I can think of a way to check for
that in the general case.
The first case is an instance of C anonymous structs and unions
whose members need to be treated as if they were members of the
containing struct. Your patch skips over them in the conditional
below but does the right thing when the conditional is removed.
/* Only look at the top most COMPONENT_REF to ignore members
of parent classes or structs. */
if (!(TREE_OPERAND (*tp, 0) == current_class_ref))
Below is a slightly different example that the patch also misses
because of the conditional. Since removing the conditional causes
problems elsewhere, it looks to me like all you need to is find
a way to distinguish those two classes of problems and handle each
appropriately.
struct C { int a; };
struct D: C {
int b;
D ():
b (a)
{ }
};
FWIW, as I think Manuel already suggested, creating a test and
coming up with a bunch of test cases will help you find the
shortcomings in your solution. Debugging them should then help
you arrive at a more robust patch.
Martin
On Sat, Nov 14, 2015 at 6:12 PM, Martin Sebor <msebor@xxxxxxxxx> wrote:
On 11/14/2015 03:00 AM, Anthony Brandon wrote:
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?
Sounds good.
By the way, while testing your patch I found a couple of corner cases
that gcc doesn't handle quite right. This one where the uninitialized
member b is used to initialize a isn't diagnosed:
struct B {
union { int a; int b; };
B ():
a (b)
{ }
};
This one initializes a reference so it doesn't actually access the
uninitialized member but is diagnosed with the patched gcc:
struct A {
int &r;
int a;
A ():
r (a),
a ()
{ }
};
EXPR_LOC_OR_LOC() is a macro just like EXPR_LOCATION() except that it
takes a second argument, LOCATION, which is used when the expression
doesn't have a location associated with it. The macro is defined in
tree.h.
Martin
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.
