[clang++] Unnecessary warning "non-void function does not return a value in all control paths"

[mkornaukhov]

I have the following snippet:

int foo(int) {
    bool matched = false;
    do {
      if (matched) {
        break;
      }
        return 42;
    } while(false);
}


int main() {
    foo(42);
}

I compile it with clang++-18:

$ clang++-18 --version
Ubuntu clang version 18.1.8 (++20240731024944+3b5b5c1ec4a3-1~exp1~20240731145000.144)
Target: x86_64-pc-linux-gnu
Thread model: posix
$ clang++-18 -Wreturn-type main.cpp 
main.cpp:9:1: warning: non-void function does not return a value in all control paths [-Wreturn-type]
    9 | }
      | ^
1 warning generated.

Variable matched is effectively constant and there are no paths that "do not return a value".
The expected behavior is such that there should be no such warning.

[chrchr-github]

Reduced:

int foo() {
    /*const*/ bool b = true;
    if (b) 
        return 42;
}

const b prevents the warning.

[mkornaukhov]

Initial snippet was something like that:

int foo(int x) {
    bool matched = false;
    do {
      if (matched) {
        label: return 0;
      }

      if (matched || x == 1) {
        matched = true;
        return 123;
      }
      if (matched || x == 2) {
        matched = true;
        return 456;
      }

      if (matched) {
        break;
      }
      goto label;
    } while(false);
}

Which shouldn't have warning, too. But I reduced it to the snippet that is in description.
In fact even this things:

int foo(int x) {
    bool matched = false;

    if (matched) return 1;
    if (!matched) return 2;
}

emits warning.

[chrchr-github]

Another example:

int f() {
    for (int i = 0; i < 1; ++i)
        return 0;
}

[cor3ntin]

@Sirraide

[Sirraide]

Er, I mean we can probably try harder and check for constant initialisers of non-const variables; I’m not particularly familiar w/ the code around the CFG but I can try taking a look at these.

[Sirraide]

So the thing is I’m not sure there is a good way to deal with cases like this:

int foo(int x) {
    bool matched = false;
    do {
      if (matched) {
        label: return 0;
      }

      if (matched || x == 1) {
        matched = true;
        return 123;
      }
      if (matched || x == 2) {
        matched = true;
        return 456;
      }

      if (matched) {
        break;
      }
      goto label;
    } while(false);
}

Because sure, it’s very easy to reduce this down to return 0—if you’re writing an optimiser, that is; and I’m not sure we’d want to start implementing optimisation passes on the CFG... Other than that I don’t really see a solution for this issue because sure, even if matched is only written to once, we don’t really have that information in the AST. We could do some checking whether the address of matched is ever taken or whether it’s ever written to, but that leads us just back to writing optimisation passes. I’ll keep thinking about this, but it’s not looking promising to me at least...