[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-18 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #36 from Pedro Alves  ---
(In reply to Jason Merrill from comment #33)
> (In reply to Pedro Alves from comment #32)
> > Usually maybe-uninit warnings point to false positives involving scalars,
> > and initializing them is practically free.  But here the size of T may be
> > significant, which could lead to e.g., calling memset in loops.
> 
> Using optional with a large T sounds like a strange choice to me, since it
> means you have to allocate the space even if you don't have a value.

I see it as a perfectly viable approach to avoiding the heap, or avoiding heap
fragmentation and have better cache locality.  For example, instead of:

struct F
{
  A *a = nullptr;
  B* b = nullptr;
};

and then allocating a / b on the heap as separate allocations, with pointer
nullness indicating whether you have the field, you can have:

struct F
{
  ...
  std::optional a;
  std::optional b;
  ...
};

and allocate the whole thing as one block.  gdb's lookup_name_info object uses
this pattern, for example.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread manu at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #35 from Manuel López-Ibáñez  ---
In any case, looking at the uninit dump with -fdump-tree-all-all-lineno it
appears that GCC knows the block where the warning is triggered is never
executed:

;;   basic block 13, loop depth 0, count 0 (precise), probably never executed
;;prev block 12, next block 14, flags: (NEW, REACHABLE, VISITED)
;;pred:   12 [never]  count:0 (precise) (EH,EXECUTABLE)
;;   starting at line -1
: [LP 2]
  # DEBUG c1$16$iD.2602 => c1$16$i_9
  # DEBUG c1D.2601 => 1
  # DEBUG thisD.2599 => 
  [./example.cpp:20:23] # DEBUG D#2ptD.0 => [./example.cpp:20:23]
&[./example.cpp:20:18] [./example.cpp:20:18] c1D.2412.D.2424.tD.2426
  [./example.cpp:20:23] # DEBUG thisD.2600 => D#2ptD.0
  [./example.cpp:9:11] # DEBUG BEGIN_STMT
  [./example.cpp:9:13] # .MEM_27 = VDEF <.MEM_15>
  xD.2332 = c1$16$i_9;
  [./example.cpp:9:18] goto ; [0.00%]
;;succ:   15 [never]  count:0 (precise) (FALLTHRU,EXECUTABLE)

I remember seeing other bugs where the block info could be used to avoid
warning.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread manu at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #34 from Manuel López-Ibáñez  ---
(In reply to Martin Sebor from comment #15)
> I think the following smaller test case independent of libstdc++ captures
> the same issue as the bigger test case in comment #4.  Again, declaring f()
> noexcept avoids the warning (but it's not a solution in general).  Zero
> initializing A::i first and then setting it to the result of f() also avoids
> the warning and seems like more viable solution/workaround until GCC gets
> smarter about exceptions.


In this example, if you add a useless pointer to t, then GCC doesn't warn:

template 
struct C {
  C (): b(), t()  { }
  ~C () { if (b) t.~T (); }

  void f () {
b = true;
new () T ();
pt = 
  }
private:
  bool b;
  T * pt;
public:
  union {
T t;
char x[sizeof (T)];
  };
 };

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread jason at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #33 from Jason Merrill  ---
(In reply to Pedro Alves from comment #32)
> Usually maybe-uninit warnings point to false positives involving scalars,
> and initializing them is practically free.  But here the size of T may be
> significant, which could lead to e.g., calling memset in loops.

Using optional with a large T sounds like a strange choice to me, since it
means you have to allocate the space even if you don't have a value.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #32 from Pedro Alves  ---
Right, the potentially-sensitive aspect is what I mean to stress here.  Usually
maybe-uninit warnings point to false positives involving scalars, and
initializing them is practically free.  But here the size of T may be
significant, which could lead to e.g., calling memset in loops.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread jason at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #31 from Jason Merrill  ---
(In reply to Pedro Alves from comment #30)
> I assume so, but do we really want to zero-initialize the buffer?  T might
> be large, and I'd think that pessimization to quiet a warning isn't the
> right way to go?

The usual way to quiet a maybe-uninit warning is to add redundant
initialization; this doesn't seem that different, apart from it being in a
library that could potentially be used in performance-sensitive code.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #30 from Pedro Alves  ---
I assume so, but do we really want to zero-initialize the buffer?  T might be
large, and I'd think that pessimization to quiet a warning isn't the right way
to go?

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-12-17 Thread jason at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Jason Merrill  changed:

   What|Removed |Added

 CC||jason at gcc dot gnu.org

--- Comment #29 from Jason Merrill  ---
(In reply to Pedro Alves from comment #0)
>union
>{
>  int m_dummy;
>  T m_item;

Here, changing m_dummy to be unsigned char[sizeof(T)] (or std::byte instead of
unsigned char), so that the buffer starts fully zero-initialized, avoids the
warning.  Similarly, in msebor's comment #15 testcase, having the C constructor
initialize x instead of t avoids the warning.  For some reason, this isn't
working for me with actual std::optional.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-11-18 Thread jamborm at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #28 from Martin Jambor  ---
The RFC did not receive any real negative feedback so I proposed to commit an
updated patch:

https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01494.html

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-11-10 Thread jamborm at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #27 from Martin Jambor  ---
(In reply to Manuel López-Ibáñez from comment #26)
> Hi Martin,
> 
> Wouldn't it be better if the testcase tested that no warning is given for a
> true case? Otherwise if the bug is fixed, no warning will be given, no
> matter the option. Or have a testcase that the warning is given with the
> option and not without .

I had to change a few testcases to use the new option so it is tested
to exist and work in that way.  I definitely do not want to add a test
ensuring we produce a false-positive warning to the testcase and a
test that a true positive is not given with the weaker option seems of
little value for me too.  But if people think it is useful, I guess I
could add one.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-11-08 Thread lopezibanez at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #26 from Manuel López-Ibáñez  ---
Hi Martin,

Wouldn't it be better if the testcase tested that no warning is given for a
true case? Otherwise if the bug is fixed, no warning will be given, no
matter the option. Or have a testcase that the warning is given with the
option and not without .

On Fri, 8 Nov 2019, 12:42 jamborm at gcc dot gnu.org, <
gcc-bugzi...@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>
> --- Comment #25 from Martin Jambor  ---
> I have posted an RFC patch alleviating the situation somewhat to the
> mailing
> list:
>
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00614.html
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-11-08 Thread jamborm at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #25 from Martin Jambor  ---
I have posted an RFC patch alleviating the situation somewhat to the mailing
list:

https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00614.html

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-10-14 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #24 from Jonathan Wakely  ---
From Bug 92092:

The program below gets the following warning message. I think the program is
well-formed (Clang 9.0.0 accepts it without warning).

** Compiler Flags **

-O2 -std=c++17 -Wall 

** Version **

gcc 9.2.0, tested online with Compiler Explorer ( https://gcc.godbolt.org/ )
but the warning happens on my Ubuntu machine as well (that version is gcc
8.3.0)

** Warning **

source>: In static member function 'static _Res
std::_Function_handler<_Res(_ArgTypes ...), _Functor>::_M_invoke(const
std::_Any_data&, _ArgTypes&& ...) [with _Res = std::optional; _Functor =
main()::; _ArgTypes = {}]':

:13:33: warning: '' may be used uninitialized in this
function [-Wmaybe-uninitialized]

   13 | return std::optional();


** Source code **

#include 
#include 

enum class Color { Red, Green, Blue };
size_t load(size_t);

int main() {
  size_t currentValue = load(0);
  auto ready = [currentValue]() -> std::optional {
if (load(1) != currentValue) {
  return Color::Red;
}
return std::optional();
  };
  std::function()> temp(ready);
  (void)temp;
}

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-10-14 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Jonathan Wakely  changed:

   What|Removed |Added

 CC||gnu at kosak dot com

--- Comment #23 from Jonathan Wakely  ---
*** Bug 92092 has been marked as a duplicate of this bug. ***

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-10-14 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #22 from Jonathan Wakely  ---
From Bug 92011:

cat > t.cc <

struct Bar
{
int size_;
Bar( int size )
  : size_( size )
{}
};

template
Bar get( T const& val )
{
  return Bar( __builtin_strlen(val) );
}

class Foo
{
int size2_;
  public:
Foo()
{}

template
Foo( T const& t )
  : size2_( get( t ).size_ )
{}
};

enum Enum
{};

bool parseImpl2( Foo s, Enum* val )
{
  *val = Enum();
  for(;;)
  {
s = "aa";
if( true )
  return false;
return true;
  }
}

template std::optional parse2( Foo str )
{
  T res = T();
  if( parseImpl2( str,  ) )
return res;
  return std::optional();
}

Enum transform()
{
  if( auto r = parse2( Foo() ) )
return *r;
  return {};
}

EOF

gcc -std=c++17 -c -o t.cc.o t.cc -Wall -O1


Gives:
t.cc: In function 'Enum transform()':
t.cc:50:27: warning: '' may be used uninitialized in this function
[-Wmaybe-uninitialized]
   50 |   return std::optional();
  |   ^

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-10-14 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Jonathan Wakely  changed:

   What|Removed |Added

 CC||joerg.rich...@pdv-fs.de

--- Comment #21 from Jonathan Wakely  ---
*** Bug 92011 has been marked as a duplicate of this bug. ***

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-07-03 Thread dushistov at mail dot ru
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #20 from Evgeniy Dushistov  ---
Also if add one line to code `printf("test\n");`

```
struct FooDeleter {
  void operator()(FooOpaque *p) {
printf("test\n");
Foo_free(p);
  }
};
```

gcc don't report any warning,
and valgrind also can not find any errors.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-07-03 Thread dushistov at mail dot ru
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Evgeniy Dushistov  changed:

   What|Removed |Added

 CC||dushistov at mail dot ru

--- Comment #19 from Evgeniy Dushistov  ---
I saw problem similar `std::optional` may be unitialized case as desribed here
with boost::variant/boost::optional (c++11) and std::variant/std::optional
(c++17),
but in my case I have not only gcc warning, but valgrind also reports problem,
is it the same problem or gcc code generation bug?


```
//Foo.cpp
#include "Foo.hpp"

struct FooOpaque {};

FooOpaque *Foo_new() {
  auto p = new FooOpaque;
  return p;
}

void Foo_free(FooOpaque *p) { delete p; }

std::variant, std::string> f_res_opt(int var) {
  switch (var) {
  case 0:
return {std::optional{Foo{Foo_new()}}};
  case 1:
return {std::optional{}};
  case 2:
return {std::string{}};
  default:
std::abort();
  }
}

```

```
//Foo.hpp
#include 
#include 
#include 

struct FooOpaque;
FooOpaque *Foo_new();
void Foo_free(FooOpaque *);

struct FooDeleter {
  void operator()(FooOpaque *p) { Foo_free(p); }
};

using Foo = std::unique_ptr;

std::variant, std::string> f_res_opt(int var);
```

```
//main.cpp
#include "Foo.hpp"

int main() {

  auto res1 = f_res_opt(0);
  auto res1_ok = std::get>(std::move(res1));

  printf("step 2\n");

  auto res2 = f_res_opt(1);

  auto res2_ok = std::get>(std::move(res2));

  printf("step 3\n");

  auto res3 = f_res_opt(2);

  auto res3_ok = std::get(std::move(res3));
}
```

gcc reports:
```
g++ -ggdb  -Ofast -Wall -Wextra -std=c++17 -pedantic  main.cpp Foo.cpp
In file included from main.cpp:1:
Foo.hpp: In function 'int main()':
Foo.hpp:10:43: warning: 'res2_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' may be used uninitialized in this function
[-Wmaybe-uninitialized]
   10 |   void operator()(FooOpaque *p) { Foo_free(p); }
  |   ^~~
main.cpp:12:8: note: 'res2_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' was declared here
   12 |   auto res2_ok = std::get>(std::move(res2));
  |^~~
In file included from main.cpp:1:
Foo.hpp:10:43: warning: 'res1_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' may be used uninitialized in this function
[-Wmaybe-uninitialized]
   10 |   void operator()(FooOpaque *p) { Foo_free(p); }
  |   ^~~
main.cpp:6:8: note: 'res1_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' was declared here
6 |   auto res1_ok = std::get>(std::move(res1));
  |^~~
```

but valgrind also reports:

valgrind -v ./a.out

```
==7858== Conditional jump or move depends on uninitialised value(s)
==7858==at 0x109374: ~unique_ptr (unique_ptr.h:288)
==7858==by 0x109374: _M_destroy (optional:257)
==7858==by 0x109374: _M_reset (optional:277)
==7858==by 0x109374: ~_Optional_payload (optional:398)
==7858==by 0x109374: ~_Optional_base (optional:471)
==7858==by 0x109374: ~optional (optional:656)
==7858==by 0x109374: main (main.cpp:12)
```

gcc 9.1.0 and valgrind 3.15.0.GIT

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-02-23 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Martin Sebor  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2019-02-23
 Ever confirmed|0   |1

--- Comment #18 from Martin Sebor  ---
Confirmed.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-02-19 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #17 from Martin Sebor  ---
The only way to avoid the warning in C (or std::optional) that I can think of
is to somehow obscure the access to the boolean flag.  Using volatile works but
looks like it has a cost that users wouldn't be happy about.  Using a relaxed
__atomic_store/_load also works and the optimized code doesn't look as bad, at
least not for the test case.  Obviously, neither is pretty but might be worth
looking into some more as a workaround.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-02-19 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #16 from Pedro Alves  ---
(In reply to Martin Sebor from comment #15)
> Zero
> initializing A::i first and then setting it to the result of f() also avoids
> the warning and seems like more viable solution/workaround until GCC gets
> smarter about exceptions.

Recall that this C in this case is an std::optional.  Basically any random type
wrapped in a std::optional can trigger issues like these.  A workaround for C
in A may be quite tricky -- A may not be default constructible, and the problem
may appear in some of A's subobjects recursively.  Or A may a class that isn't
under the user's control.  A workaround in C would be much better.  Memsetting
the buffer in C's ctor works, but the downside is that that likely has a run
time cost, which seems undesirable for std::optional.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-02-19 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #15 from Martin Sebor  ---
I think the following smaller test case independent of libstdc++ captures the
same issue as the bigger test case in comment #4.  Again, declaring f()
noexcept avoids the warning (but it's not a solution in general).  Zero
initializing A::i first and then setting it to the result of f() also avoids
the warning and seems like more viable solution/workaround until GCC gets
smarter about exceptions.

$ cat pr80635.C && gcc -O2 -S -Wall pr80635.C
void* operator new (__SIZE_TYPE__, void *p) { return p; }

int f ();
int x;

struct A {
  int i;
  A (): i (f ()) { }
  ~A () { x = i; }
};

struct B {
  B ();
  ~B ();
};


template 
struct C {
  C (): t (), b () { }
  ~C () { if (b) t.~T (); }

  void f () {
new () T ();
b = true;
  }

  union {
T t;
char x[sizeof (T)];
  };
  bool b;
};

void g ()
{
  C c1;
  C c2;

  c1.f ();
  c2.f ();
}
pr80635.C: In function ‘void g()’:
pr80635.C:9:13: warning: ‘c1.A::i’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
9 |   ~A () { x = i; }
  |   ~~^~~
pr80635.C:37:8: note: ‘c1.A::i’ was declared here
   37 |   C c1;
  |^~

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2019-01-15 Thread tromey at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #14 from Tom Tromey  ---
(In reply to Jonathan Wakely from comment #8)
> Something like __builtin_unreachable() to say "trust me" would be nice, but
> I can't think how to do it.

How about an attribute that can be attached to the member?
Then tree-ssa-uninit could check for this and suppress the warning.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2018-09-18 Thread manu at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Manuel López-Ibáñez  changed:

   What|Removed |Added

 CC||manu at gcc dot gnu.org,
   ||msebor at gcc dot gnu.org

--- Comment #13 from Manuel López-Ibáñez  ---
This may be another case where it is worth it to print the inline stack AND
silence a warning whose inline stack is within pragma GCC diagnostics.

However, there seems to be another kind of missed optimization going on here.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2018-09-17 Thread egallager at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #12 from Eric Gallager  ---
(In reply to Marc Glisse from comment #11)
> (In reply to Jonathan Wakely from comment #10)
> > Sadly I have no better suggestion than -Wno-error=maybe-uninitialized
> 
> Move -Wmaybe-uninitialized from -Wall to -Wextra?

That sounds like a decent suggestion

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #11 from Marc Glisse  ---
(In reply to Jonathan Wakely from comment #8)
> Something like __builtin_unreachable() to say "trust me" would be nice, but
> I can't think how to do it.

Some __builtin_unreachable() in _M_get might (?) be useful even if it doesn't
help with the destructor issue. Or some assertion for debug mode, since the
comment above says "The _M_get operations have _M_engaged as a precondition"...

(In reply to Jonathan Wakely from comment #10)
> Sadly I have no better suggestion than -Wno-error=maybe-uninitialized

Move -Wmaybe-uninitialized from -Wall to -Wextra?

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #10 from Jonathan Wakely  ---
(In reply to Pedro Alves from comment #9)
> I had tried that last night, but unfortunately it couldn't get it to work,
> because the warning triggers in A, not optional.

Bah! When we want the warning location to be in our headers it's in user code
(like this case) and when we want it in user code it's in our headers (and so
suppressed, like Bug 58876).

Sadly I have no better suggestion than -Wno-error=maybe-uninitialized

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #9 from Pedro Alves  ---
> So maybe we just want to use a #pragma around the std::optional destructor to 
> suppress this warning.

I had tried that last night, but unfortunately it couldn't get it to work,
because the warning triggers in A, not optional.  Users of optional have
to put the #pragma around their the Ts (in this case A::~A()).  I.e., this
would work:

 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"

 struct A
 {
   A () : m (get ()) {}
   ~A () { set (m); }  // warns here

   int m;
 };

 #pragma GCC diagnostic pop

I think as we'll use gdb/std::optional more and more, that would become too
unwildy/ugly.  My current workaround in gdb is -Wno-error=maybe-uninitialized:

[1] - https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #8 from Jonathan Wakely  ---
Something like __builtin_unreachable() to say "trust me" would be nice, but I
can't think how to do it. So maybe we just want to use a #pragma around the
std::optional destructor to suppress this warning.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #7 from Marc Glisse  ---
The warning comes from
  _Z3setiD.6701 (maybe_a$D6763$m_dummy_6);
which is protected by
  _9 = VIEW_CONVERT_EXPR(maybe_a$4_7);
  if (_9 != 0)
with
  # maybe_a$D6763$m_dummy_6 = PHI 
  # maybe_a$4_7 = PHI <0(6), 1(4)>

In this case, more aggressive threading would kill the possibility to call set
on something undefined (I believe Jeff was already looking into it for other
Wmaybe-uninitialized testcases). The warning is unstable because it depends on
fragile optimization results.

This isn't solvable in general anyway, Wmaybe-uninitialized has "maybe" for a
good reason.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #6 from Pedro Alves  ---
That kind of makes sense if you look at optional in isolation, but why does
it _not_ warn if you remove anything related to B and leave only A?  That's
what's truly mystifying to me.

Even this change makes the warning go away:

 void func ()
 {
   Optional maybe_a;
-  Optional maybe_b; // for some reason, need this here to trigger a
+  Optional maybe_b; // for some reason, need this here to trigger a
   // warning in _A_.

   maybe_a.emplace ();
   maybe_b.emplace (); // comment out, and the warning disappears.
 }

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #5 from Jonathan Wakely  ---
(In reply to Pedro Alves from comment #4)
> Looks like I over reduced in the minimal reproducer.  std::optional has a
> boolean field to track whether the contained object had been fully
> initialized, which is checked in the desctructor, but I removed it because
> its presence doesn't affect whether the warning is emitted.  Of course,
> std::optional has that field, but still, it warns.

I think the problem is that GCC isn't smart enough to infer the invariant that
the truthiness of the bool corresponds to the initialization of the member. So
the value of the bool is treated as unrelated to the (un)initialized state. By
inspecting all the accesses to the bool we can tell that's true, but the
compiler apparently can't. I don't know how we could state the invariant in
code.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #4 from Pedro Alves  ---
Hi Marc, thanks much for taking a look.

Looks like I over reduced in the minimal reproducer.  std::optional has a
boolean field to track whether the contained object had been fully initialized,
which is checked in the desctructor, but I removed it because its presence
doesn't affect whether the warning is emitted.  Of course, std::optional has
that field, but still, it warns.

A couple of things that look suspiciously odd to me, even in the
original testcase:

 - the warning is about A::m_dummy, while optional::~optional calls the 
   m_item/T's destructor, not m_dummy's.

 - the warning triggers in A/optional, but for some reason, only if
   B/optional exist, as well as the maybe_b variable, which are all
   completely unrelated to A.  This one makes me wonder if there's some
   miscompilation related to aliasing or or object lifetimes going on,
   not just a warning.

Here's the corrected testcase:

~~
$ cat optional2.cc
//#include 
//#include 
#include 

template
struct optional
{
  optional ()
: m_dummy (),
  m_instantiated (false)
  {}

  ~optional ()
  {
if (m_instantiated)
  m_item.~T (); // won't run unless T is fully constructed.
  }

  void emplace ()
  {
new (_item) T ();
m_instantiated = true; // not set if T() throws
  }

  union
  {
int m_dummy;
T m_item;
  };

  bool m_instantiated;
};

template 
using Optional = optional; // warns
//using Optional = std::experimental::optional; // warns too
//using Optional = std::optional; // warns too

extern int get ();
extern void set (int);

struct A
{
  A () : m (get ()) {} // warns here
  ~A () { set (m); }

  int m;
};

// for some reason, need B to trigger the warning.
struct B
{
  B (); // remove or make noexcept, and the warning disappears
  ~B (); // remove, and the warning disappears
};

void func ()
{
  Optional maybe_a;
  Optional maybe_b; // for some reason, need this here to trigger a
   // warning in _A_.

  maybe_a.emplace ();
  maybe_b.emplace (); // comment out, and the warning disappears.
}

$ /opt/gcc/bin/g++ optional2.cc -O2 -Wall -c 
optional2.cc: In function ‘void func()’:
optional2.cc:45:15: warning:
‘maybe_a.optional::.optionalm_dummy’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
   ~A () { set (m); }
   ^~~
optional2.cc:59:15: note:
‘maybe_a.optional::.optionalm_dummy’ was
declared here
   Optional maybe_a;
   ^~~



Do you see anything invalid in this version of the test?

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-05 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #3 from Marc Glisse  ---
If you mark "get" as noexcept, the warning disappears. If get throws an
exception, you can very well end up running the destructor without having
initialized the members. The warning seems correct to me.

[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning

2017-05-04 Thread palves at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

--- Comment #2 from Pedro Alves  ---
Looks like a regression at some point:

There are no warnings with g++ 5.3.1, either reduced testcase, or with the
obvious change to use std::experimental::optional instead of std::optional.

Also no warnings with g++ 8.0.0 20170428 + -fno-lifetime-dse or
-flifetime-dse=1, perhaps unsurprisingly.

The original bug supposedly triggers with g++ 6.3.1 too, though I haven't
confirmed with the reduced testcase there.  (It's an s390 gdb buildbot, I don't
have access to it.)