If you uncomment the lines to use std::optional instead, you get:

 $ /opt/gcc/bin/g++ optional.cc -g3 -O2 -Wall -std=gnu++17 -c 
 optional.cc: In function ‘void func()’:
 optional.cc:28:15: warning: ‘maybe_a.A::m’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    ~A () { set (m); }
            ~~~~^~~
 optional.cc:41:15: note: ‘maybe_a.A::m’ was declared here
    Optional<A> maybe_a;
                ^~~~~~~

This warns at a different location from the reproducer in the OP.

If you revert back to use the self-contained optional, and change the optional::m_dummy field to be of empty struct type, similarly to
std::optional:

  union
  {
    struct {} m_dummy;
    T m_item;
  };

then you get the exact same warning you get with std::optional.

I suspect that the "maybe_a.A::m" location is a bug of its own and a red herring here, but I can't be sure.

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.)

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.

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<A>, but for some reason, only if
   B/optional<B> 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 <optional>
//#include <experimental/optional>
#include <new>

template<typename T>
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 (&m_item) T ();
    m_instantiated = true; // not set if T() throws
  }

  union
  {
    int m_dummy;
    T m_item;
  };

  bool m_instantiated;
};

template <typename T>
using Optional = optional<T>; // warns
//using Optional = std::experimental::optional<T>; // warns too
//using Optional = std::optional<T>; // 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<A> maybe_a;
  Optional<B> 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<A>::<anonymous>.optional<A>::<unnamed union>::m_dummy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   ~A () { set (m); }
           ~~~~^~~
optional2.cc:59:15: note: ‘maybe_a.optional<A>::<anonymous>.optional<A>::<unnamed union>::m_dummy’ was declared here
   Optional<A> maybe_a;
               ^~~~~~~

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

(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.

That kind of makes sense if you look at optional<T> 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<A> maybe_a;
-  Optional<B> maybe_b; // for some reason, need this here to trigger a
+  Optional<A> 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.
 }

The warning comes from
  _Z3setiD.6701 (maybe_a$D6763$m_dummy_6);
which is protected by
  _9 = VIEW_CONVERT_EXPR<boolD.2220>(maybe_a$4_7);
  if (_9 != 0)
with
  # maybe_a$D6763$m_dummy_6 = PHI <maybe_a$D6763$m_dummy_4(D)(6), _5(4)>
  # 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.

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.

> 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<A>.  Users of optional<T> 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

(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<A>.

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

(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?

(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

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.

(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.

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 <class T>
struct C {
  C (): t (), b () { }
  ~C () { if (b) t.~T (); }

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

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

void g ()
{
  C<A> c1;
  C<B> 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<A> c1;
      |        ^~

(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.

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.

Confirmed.

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::optional<Foo>, std::string> f_res_opt(int var) {
  switch (var) {
  case 0:
    return {std::optional<Foo>{Foo{Foo_new()}}};
  case 1:
    return {std::optional<Foo>{}};
  case 2:
    return {std::string{}};
  default:
    std::abort();
  }
}

```

```
//Foo.hpp
#include <memory>
#include <optional>
#include <variant>

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

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

using Foo = std::unique_ptr<FooOpaque, FooDeleter>;

std::variant<std::optional<Foo>, 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::optional<Foo>>(std::move(res1));

  printf("step 2\n");

  auto res2 = f_res_opt(1);

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

  printf("step 3\n");

  auto res3 = f_res_opt(2);

  auto res3_ok = std::get<std::string>(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::optional<Foo>>(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::optional<Foo>>(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

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 92011 has been marked as a duplicate of this bug. ***

From Bug 92011:

cat > t.cc <<EOF
#include <optional>

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

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

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

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

enum Enum
{};

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

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

Enum transform()
{
  if( auto r = parse2<Enum>( 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: '<anonymous>' may be used uninitialized in this function [-Wmaybe-uninitialized]
   50 |   return std::optional<T>();
      |                           ^

*** Bug 92092 has been marked as a duplicate of this bug. ***

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<Color>; _Functor = main()::<lambda()>; _ArgTypes = {}]':

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

   13 |     return std::optional<Color>();


** Source code **

#include <functional>
#include <optional>

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

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

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

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-bugzilla@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>
> --- Comment #25 from Martin Jambor <jamborm at gcc dot gnu.org> ---
> 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.

(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.

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

(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.

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?

(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.

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.

(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.

(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 <class T>
struct C {
  C (): b(), t()  { }
  ~C () { if (b) t.~T (); }

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

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
<L6>: [LP 2]
  # DEBUG c1$16$iD.2602 => c1$16$i_9
  # DEBUG c1D.2601 => 1
  # DEBUG thisD.2599 => &c1D.2412
  [./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 <bb 15>; [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.

(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> a;
  std::optional<B> b;
  ...
};

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

Regression from GCC 5, because with the GCC 6 -flifetime-dse changes we properly recognize the initial state of the object as uninitialized; with -fno-lifetime-dse we think it starts out zero-initialized.

The problem is that we have two parallel variables that aren't being treated as parallel.  From the .optimized dump of the comment 0 test using std::optional (because the warning is correct for the over-reduced optional):

  <bb 7> [count: 0]:
  # maybe_a$m_51 = PHI <maybe_a$m_38(D)(2), _29(5)> // _M_value
  # maybe_a$4_54 = PHI <0(2), 1(5)> // _M_engaged
...
  _12 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_54);
  if (_12 != 0)
    goto <bb 10>; [0.00%]
  else
    goto <bb 11>; [0.00%]

  <bb 10> [count: 0]:
  set (maybe_a$m_51);

Here maybe_a$m_51 is uninitialized iff maybe_a$4_54 is 0, and in that case we don't use the uninitialized value.  The PHIs are completely parallel.  I'm a bit surprised the optimizer doesn't already handle this sort of situation, where a conditional jump determines which PHI argument we have, and therefore should be able to select the corresponding argument from parallel PHIs in the target block.  So bb 10 should be

  set (_29);

and we shouldn't get the warning.

(In reply to Jason Merrill from comment #37)
>(because the warning is correct for the over-reduced optional):

This is better:

template<typename T>
struct optional
{
  optional () : m_dummy (), live (false) {}
  void emplace () { new (&m_item) T (); live = true; }
  ~optional () { if (live) m_item.~T (); }

  union
  {
    int m_dummy;
    T m_item;
  };
  bool live;
};

*** Bug 92700 has been marked as a duplicate of this bug. ***

*** Bug 92194 has been marked as a duplicate of this bug. ***

(In reply to Jason Merrill from comment #38)
> This is better:

Complete revised testcase:

#ifdef USE_STD

#include <optional>
using std::optional;

#else

using size_t = decltype(sizeof(1));
inline void *operator new (size_t, void *p) { return p; }
template<typename T>
struct optional
{
  optional () : m_dummy (), live (false) {}
  void emplace () { new (&m_item) T (); live = true; }
  ~optional () { if (live) m_item.~T (); }

  union
  {
    struct {} m_dummy;
    T m_item;
  };
  bool live;
};

#endif

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

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

  int m;
};

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

void func ()
{
  optional<A> maybe_a;
  optional<B> maybe_b;

  maybe_a.emplace ();
  maybe_b.emplace ();
}

Jason, thanks for the testcase in c#41.  Here's the blocks of interest:

;;   basic block 5, loop depth 0
;;    pred:       3
;;                2
  # maybe_a$m_6 = PHI <_5(3), maybe_a$m_4(D)(2)>
  # maybe_a$4_7 = PHI <1(3), 0(2)>
<L0>:
  _8 = maybe_b.live;
  if (_8 != 0)
    goto <bb 6>; [0.00%]
  else
    goto <bb 7>; [0.00%]
;;    succ:       6
;;                7

;;   basic block 6, loop depth 0
;;    pred:       5
  B::~B (&maybe_b.D.2512.m_item);
;;    succ:       7

;;   basic block 7, loop depth 0
;;    pred:       5
;;                6
  maybe_b ={v} {CLOBBER};
  resx 3
;;    succ:       8

;;   basic block 8, loop depth 0
;;    pred:       7
<L1>:
  _9 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_7);
  if (_9 != 0)
    goto <bb 9>; [0.00%]
  else
    goto <bb 10>; [0.00%]
;;    succ:       9
;;                10

I believe the complaint arises from the PHI in BB5  which is then used BB9. Intuitively we can see that anytime BB5 is reached directly from BB2 we know that BB8 will transfer control to BB10, avoiding the problematical use in BB9.

However, the form of the CFG is particularly difficult for forward threader to handle.  BB5 is a join point.  We can only have one other block in the threading path with side effects -- and unfortunately we have BB6 and BB7.  We can fudge a bit on BB8 due to some work Alex did a couple years back, but ultimately the forward threader isn't going to handle this.

The backwards threader supports multiple blocks with side effects, but there can only be one path from the start to the destination.  2->5->6->7->8 and 2->5->7->8 makes two paths, so it gets rejected.  This is a limitation we really should lift in the backwards threader.

I suspect the V_C_E  inhibits tree-ssa-uninit.c code to avoid false positives.  If we know the range of the V_C_E operand is narrow enough that it fits into the type of the result of the V_C_E, could we optimize the V_C_E into a NOP_EXPR?   We'd then forward propagate away the NOP_EXPR and the result is something the suppression code in tree-ssa-uninit.c can analyze better.





We can see that the path 2->5->6->7->8->9 is infeasible as is the path 2->5->7->8->9.  ANytime BB5 is reached directly from BB2

(In reply to Jeffrey A. Law from comment #42)
> I suspect the V_C_E  inhibits tree-ssa-uninit.c code to avoid false
> positives.  If we know the range of the V_C_E operand is narrow enough that
> it fits into the type of the result of the V_C_E, could we optimize the
> V_C_E into a NOP_EXPR?   We'd then forward propagate away the NOP_EXPR and
> the result is something the suppression code in tree-ssa-uninit.c can
> analyze better.
 
Looks like the V_C_E comes from sra_modify_assign:

          if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
            {
=>            rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
                                     rhs);

Here rhs (the scalar replacement variable) has unsigned QI type and lhs has type bool.

Changing the type of 'live' to unsigned char avoids the V_C_E and thus indeed the warning.  This also works with the libstdc++ optional.

This seems like an SRA problem with fields of type bool.

I suspect in the context where SRA creates the V_C_E we don't have enough information to know that the range of the input object is constrained enough.  We base the decision solely on the types.

By the time we get into the middle end we can follow the use-def chain to the PHI and realize the source object only has two possible values at runtime and we can optimize the V_C_E into a NOP_EXPR.

I've contacted Eric B. offline on the semantics of V_C_E.  I think the agreement is in this kind of scenario we ought to be able to simplify it into a NOP_EXPR which should be sufficient to eliminate the false positive.

(In reply to Jeffrey A. Law from comment #44)
> I suspect in the context where SRA creates the V_C_E we don't have enough
> information to know that the range of the input object is constrained
> enough.  We base the decision solely on the types.

We SRA a bool field into a QItype variable and then think we need a VCE to get back to bool.  Could the SRA variable have type bool?

(In reply to Jason Merrill from comment #45)
> 
> We SRA a bool field into a QItype variable and then think we need a VCE to
> get back to bool.  Could the SRA variable have type bool?

A semi-wild guess, would the following make SRA not invent the V_C_E (and do what you want)?

diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 5561ea6f655..c3551b469f1 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -2515,7 +2515,7 @@ analyze_access_subtree (struct access *root, struct access *parent,
       /* Always create access replacements that cover the whole access.
          For integral types this means the precision has to match.
         Avoid assumptions based on the integral type kind, too.  */
-      if (INTEGRAL_TYPE_P (root->type)
+      if (false && INTEGRAL_TYPE_P (root->type)
          && (TREE_CODE (root->type) != INTEGER_TYPE
              || TYPE_PRECISION (root->type) != root->size)
          /* But leave bitfield accesses alone.  */

Martin, yea, your patch does prevent creation of the V_C_E.  That in turn allows maybe_a$live_7 to be directly used in the conditional which in turn allows tree-ssa-uninit.c to realize the problematic path isn't runtime feasible and suppresses the warning.

(In reply to Jeffrey A. Law from comment #47)
> Martin, yea, your patch does prevent creation of the V_C_E.  That in turn
> allows maybe_a$live_7 to be directly used in the conditional which in turn
> allows tree-ssa-uninit.c to realize the problematic path isn't runtime
> feasible and suppresses the warning.

This should also work too:
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index ea8594db193..83b1d981439 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -2499,6 +2499,7 @@ analyze_access_subtree (struct access *root, struct access *parent,
          For integral types this means the precision has to match.
         Avoid assumptions based on the integral type kind, too.  */
       if (INTEGRAL_TYPE_P (root->type)
+         && TREE_CODE (root->type) != BOOLEAN_TYPE
          && (TREE_CODE (root->type) != INTEGER_TYPE
              || TYPE_PRECISION (root->type) != root->size)
          /* But leave bitfield accesses alone.  */

---- CUT ----

GCC 8.4.0 has been released, adjusting target milestone.

Reassigning to Martin Jambor since the real fix is to avoid creating the V_C_E in the first place.

(In reply to Andrew Pinski from comment #48)
> This should also work too:
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index ea8594db193..83b1d981439 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -2499,6 +2499,7 @@ analyze_access_subtree (struct access *root, struct
> access *parent,
>           For integral types this means the precision has to match.
>          Avoid assumptions based on the integral type kind, too.  */
>        if (INTEGRAL_TYPE_P (root->type)
> +         && TREE_CODE (root->type) != BOOLEAN_TYPE
>           && (TREE_CODE (root->type) != INTEGER_TYPE
>               || TYPE_PRECISION (root->type) != root->size)
>           /* But leave bitfield accesses alone.  */
> 
> ---- CUT ----

Well, this re-introduces bug PR 52244 and makes the associated
testcase fail.  PR 52244 fix specifically aimed to disallow boolean
replacements.

(In reply to Jeffrey A. Law from comment #50)
> Reassigning to Martin Jambor since the real fix is to avoid creating the
> V_C_E in the first place.
 
I hoped that changing SRA to emit a NOP_EXPR instead of V_C_E would
help, but unfortunately it doesn't.  I've been looking at this for the
whole evening yesterday and ATM I do not see how I could avoid
conversion without reintroducing PR 52244 (in the general case - this
is another consequence of the fact that SRA is not flow sensitive).

ACK.  I guess I'll try to polish up the vr-values hack I was playing with.

For people who are not willing to turn off this warning:

This warning may be suppressed by introducing a volatile member in the union that is used as the storage.

Using Martin Sebor's testcase, this look likes this:

```
  union {
    T t;
    char x[sizeof (T)];
    volatile char dont_use;   // this silences such warnings.
  };
```

(the `sizeof (T)` thing is unnecessary, as the union will always have correct size and alignment w/o it.)

Hope it helps.

I'm not sure if this is the same bug or a different one, but it's in the same family (I think).  Visible in current trunk, 10.2 release, and as far back as 8.1.

I found optimize level -Os is necessary 

https://godbolt.org/z/Ta55jM


#include <optional>
using Opt = std::optional<int>;

Opt f(int x) {               
    Opt ret;                 
    Opt val = (x == 1) ? 999 : (x == 2) ? Opt(2) : std::nullopt;
    if (val.has_value())
        ret = val.value();
    return ret;
}


<source>: In function 'Opt f(int)':
<source>:9:12: error: 'val.std::_Optional_payload_base<int>::_Storage<int, true>::_M_value' may be used uninitialized in this function [-Werror=maybe-uninitialized]

    9 |     return ret;

      |            ^~~

So to summarize some thoughts from Richi from last year.

Converting the V_C_E into a NOP_EXPR is undesirable as the truncation becomes sub-optimal because we end up with an additional masking operation.  So the V_C_E is really the right thing.

What that implies is that other passes need to get smarter about handling a V_C_E.

I think Jeff is correct. While I have no experience with writing compilers, I've been wrestling with this "feature" for about 3 years now.  What's odd is that it doesn't always manifest.  It also seems to be restricted (at least in my cases) to using std::optional with POD or enum types.  Also, the volatile thingy doesn't solve the issue if sizeof(T) is less than 16 bytes. (Again my experience.)

Anyway... some findings which I think support Jeff's comment:  about a year ago I went so far as to memset to zero the memory in std::optional for sizeof(T).  This made the warning go away... defeats the purpose of optional of course, but no more warning.  But on inspecting the assembly generated, there was no zero-ing of the memory (or any other assignment to 0). So the optimizer no longer error'd on maybe-uninitialized, but knew enough to elide away the memset.

So... is this maybe a case of the warning simply being emitted too soon in the passes, i.e., before its completely finished optimizing?  Because even without the memset change, the final assembly generated under -O3 never has a path where the value in the optional is accessed when uninitialized.

Below is a POC for improving the uninit analysis to handle this kind of case.  It's not complete.  In particular it does not ensure that the we have the same result on the RHS and LHS of the V_C_E.  Basically I'm just showing where/how we could look through a V_C_E to determine that the use is properly guarded.

+      /* If FLAG_DEF is a V_C_E, then we can look through it.
+        The trick is to know when the V_C_E doesn't change the
+        value, which isn't validated yet.  */
+      if (is_gimple_assign (*flag_def)
+         && gimple_assign_rhs_code (*flag_def) == VIEW_CONVERT_EXPR)
+       {
+         tree rhs = gimple_assign_rhs1 (*flag_def);
+         rhs = TREE_OPERAND (rhs, 0);
+         if (TREE_CODE (rhs) == SSA_NAME)
+           {
+             if ((*flag_def = SSA_NAME_DEF_STMT (rhs)) == NULL)
+               continue;
+           }
+       }

Jeff's POC also suggests a workaround: changing the type of _Optional_payload_base::_M_engaged from bool to unsigned char avoids the VIEW_CONVERT_EXPR and avoids the warning.  The difference in the uninit IL is below but there's no difference in the assembly.

 {
-  unsigned char maybe_a$4;
+  unsigned char maybe_a$_M_payload$D12141$D11991$_M_engaged;
   int maybe_a$m_val;
   struct optional maybe_b;
   struct optional maybe_a;
-  bool _11;
-  bool _12;
+  unsigned char _11;
   int _29;
-  bool _32;
+  unsigned char _32;
 
   <bb 2> [local count: 1073741824]:
   maybe_b = {};
@@ -53,7 +52,7 @@
 
   <bb 9> [count: 0]:
   # maybe_a$m_val_51 = PHI <_29(6), maybe_a$m_val_38(D)(8)>
-  # maybe_a$4_54 = PHI <1(6), 0(8)>
+  # maybe_a$_M_payload$D12141$D11991$_M_engaged_54 = PHI <1(6), 0(8)>
 <L0>:
   _11 = MEM[(struct _Optional_payload_base *)&maybe_b]._M_engaged;
   if (_11 != 0)
@@ -74,8 +73,7 @@
 
   <bb 12> [count: 0]:
 <L1>:
-  _12 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_54);
-  if (_12 != 0)
+  if (maybe_a$_M_payload$D12141$D11991$_M_engaged_54 != 0)
     goto <bb 13>; [0.00%]
   else
     goto <bb 17>; [0.00%]

So, can't we combine
--- gcc/match.pd.jj	2021-02-18 16:21:01.000000000 +0100
+++ gcc/match.pd	2021-02-23 12:39:32.801064905 +0100
@@ -3316,7 +3316,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (view_convert @0)
   (if ((INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type))
        && (INTEGRAL_TYPE_P (TREE_TYPE (@0)) || POINTER_TYPE_P (TREE_TYPE (@0)))
-       && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)))
+       && (TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0))
+	   || (TREE_CODE (type) == BOOLEAN_TYPE
+	       && TREE_CODE (@0) == SSA_NAME
+	       && TREE_CODE (TREE_TYPE (@0)) != BOOLEAN_TYPE
+	       && TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (@0))
+	       && type_has_mode_precision_p (TREE_TYPE (@0))
+	       && ssa_name_has_boolean_range (@0))))
    (convert @0)))
 
 /* Strip inner integral conversions that do not change precision or size, or
with expander optimization that will optimize away that masking when expanding NOP_EXPR from integral SSA_NAME with ssa_name_has_boolean_range to bool?
Note, on this testcase the NOP_EXPR to bool is optimized away, so the expansion change would make zero difference, so I'll need to test that without the above change and doing the VCE -> NOP optimization during TER or so to see it in action.  Anyway, when I did it earlier today, while expansion emitted the AND 1, later optimizations optimized that away again.

Created attachment 50240 [details]
gcc11-pr80635.patch

Full untested patch.

The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:3cf52b87ff6938e30883b8f8f542a638635d507d

commit r11-7383-g3cf52b87ff6938e30883b8f8f542a638635d507d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Feb 25 10:16:55 2021 +0100

    vrp: Handle VCE in vrp_simplify_cond_using_ranges [PR80635]
    
    > So I wonder what other optimizations are prevented here?
    
    > Why does uninit warn with VCE but not with NOP_EXPR?  Or does the
    > warning disappear because of those other optimizations you mention?
    
    The optimization that it prevents is in this particular case in tree-vrp.c
    (vrp_simplify_cond_using_ranges):
    
          if (!is_gimple_assign (def_stmt)
              || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
            return;
    so it punts on VIEW_CONVERT_EXPR, with NOP_EXPR it optimizes that:
      _9 = (bool) maybe_a$4_7;
      if (_9 != 0)
    into:
      _9 = (bool) maybe_a$4_7;
      if (maybe_a$4_7 != 0)
    
    Now, if I apply my patch but manually disable this
    vrp_simplify_cond_using_ranges optimization, then the uninit warning is
    back, so on the uninit side it is not about VIEW_CONVERT_EXPR vs. NOP_EXPR,
    both are bad there, uninit wants the guarding condition to be
    that SSA_NAME and not some demotion cast thereof.
    We have:
      # maybe_a$m_6 = PHI <_5(4), maybe_a$m_4(D)(6)>
      # maybe_a$4_7 = PHI <1(4), 0(6)>
    ...
    One of:
      _9 = VIEW_CONVERT_EXPR<bool>(maybe_a$4_7);
      if (_9 != 0)
    or:
      _9 = (bool) maybe_a$4_7;
      if (_9 != 0)
    or:
      if (maybe_a$4_7 != 0)
    followed by:
        goto <bb 11>; [0.00%]
      else
        goto <bb 14>; [0.00%]
    ...
      <bb 11> [count: 0]:
      set (maybe_a$m_6);
    and uninit wants to see that maybe_a$m_4(D) is not used if
    bb 11 is encountered.
    
    This patch fixes it by teaching vrp_simplify_cond_using_ranges
    to handle VCE (when from an integral type) in addition to
    NOP_EXPR/CONVERT_EXPR, of course as long as the VCE or demotion
    doesn't change any values, i.e. when the range of the VCE or
    conversion operand fits into the target type.
    
    2021-02-25  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/80635
            * tree-vrp.c (vrp_simplify_cond_using_ranges): Also handle
            VIEW_CONVERT_EXPR if modes are the same, innerop is integral and
            has mode precision.
    
            * g++.dg/warn/pr80635-1.C: New test.
            * g++.dg/warn/pr80635-2.C: New test.

Fixed on the trunk.

GCC 8 branch is being closed.

GCC 9.4 is being released, retargeting bugs to GCC 9.5.

Below is some code that produces unexpected -Wmaybe-uninitialized warnings.  Is this a variant of this bug or a separate bug?

I've tried a few configurations:

Unexpected warnings:
$ g++-10 -Wall -O2 warn.cpp -DBOOST
warn.cpp: In function ‘int main()’:
warn.cpp:19:10: warning: ‘*((void*)& i +1)’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   19 |     m_i(i)
      |          ^
warn.cpp:32:10: note: ‘*((void*)& i +1)’ was declared here
   32 |     optb i;
      |          ^

No warnings (c++17 flag not needed with GCC 11).
$ g++-10 -Wall -O2 warn.cpp -std=c++17 
$ g++-11 -Wall -O2 warn.cpp -DBOOST
$ g++-11 -Wall -O2 warn.cpp 

The constructor takes three optional<std::string> and one optional<bool>.  Adjusting the number and types of parameters makes a difference, but I don't see why.  Perhaps with less parameters, passing by register rather than stack memory affects warning generation.  However 4 x optional<std::string> gives no warning and that would be larger than 3 x optional<std::string> plus 1 x optional<bool>.

It's not clear why std::optional would be free from warnings, yet boost::optional not.  Is adoption of std::optional an effective way of avoiding unnecessary -Wmaybe-uninitialized warnings?

It seems that GCC 11 has better behaviour.  Is this expected or would some other (perhaps larger) collection of parameters trigger the same warning with GCC 11?

If GCC 11 incorporates a specific fix, will that be backported to GCC 10?

Tested with GCC on openSUSE Tumbleweed.  Package versions:
gcc10-c++-10.3.0+git1587-2.1.x86_64
gcc11-c++-11.1.1+git340-1.1.x86_64

The source for warn.cpp:
========================
#include <string>

#ifdef BOOST
#include <boost/optional.hpp>
using opts = boost::optional<std::string>;
using optb = boost::optional<bool>;
#else
#include <optional>
using opts = std::optional<std::string>;
using optb = std::optional<bool>;
#endif

class foo
{
public:
    foo(opts a, opts b, opts c,
        optb i)
    : m_a(a), m_b(b), m_c(c),
      m_i(i)
    {}

private:
    opts m_a;
    opts m_b;
    opts m_c;
    optb m_i;
};

int main()
{
    opts a, b, c;
    optb i;

    foo bar(a, b, c, i);
}

GCC 9 branch is being closed

GCC 10.4 is being released, retargeting bugs to GCC 10.5.

This (or at least a very similar) bug still exists in gcc 11.2.0.

Here's another test case that still emits warnings in latest GCC trunk as available through godbolt: https://godbolt.org/z/cWaca3s5s

Please do not attach new testcases to this BZ.  They should be created at distinct bugs.  While they may generate the same warning, the underlying details are usually different.

Fixed in GCC 11.