Bug 85827 - false positive for -Wunused-but-set-variable because of constexpr-if
Summary: false positive for -Wunused-but-set-variable because of constexpr-if
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: ---
Assignee: Marek Polacek
URL:
Keywords: diagnostic
Depends on:
Blocks: Wunused
  Show dependency treegraph
 
Reported: 2018-05-18 11:32 UTC by Matthias Kretz (Vir)
Modified: 2019-08-17 18:23 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 7.1.0, 7.2.0, 7.3.0, 8.1.0, 9.0
Last reconfirmed: 2018-05-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Kretz (Vir) 2018-05-18 11:32:05 UTC
Testcase `-std=c++17 -Wall` (cf. https://godbolt.org/g/kfgN2V):

template <int N> int f()
{
    constexpr bool _1 = N == 1;
    constexpr bool _2 = N == 2;
    constexpr bool _3 = N == 3;
    
    if constexpr (_1) {
        return 5;
    } else if constexpr (_2) {
        return 1;
    } else if constexpr (_3) {
        return 7;
    }
}

int a() {
    return f<1>();
}
int b() {
    return f<2>();
}
int c() {
    return f<3>();
}

Yes, I see how one can argue that _2 and _3 are unused in f<1>. However, this makes use of constexpr-if cumbersome. E.g. when a variable is required in all but one branch, then it'll warn for specializations that take this one branch. So you'll have to reorder the code such that the variable is only defined inside the else branch where all the other branches are handled. But what if you have two variables and their uses are disjunct? This leads to code duplication, just for silencing the warning.

I believe, -Wunused-but-set-variable needs to consider all constexpr branches to be useful here.
Comment 1 Matthias Kretz (Vir) 2018-05-18 11:39:05 UTC
Same issue for -Wunused-variable
Comment 2 Marc Glisse 2018-05-18 12:29:25 UTC
I think that's going to be hard. The same issue always existed with macros. The whole point of "if constexpr" is not to look at the other branches, as they may not even compile. Sure, some minimal "safe" attempt at looking at those branches would be possible, but I am not sure it is worth the trouble.
Comment 3 Matthias Kretz (Vir) 2018-05-18 12:47:30 UTC
But macros are different. They remove the code before the C++ parser sees it (at least as-if). One great improvement of constexpr-if over macros is that all the other branches are parsed and their syntax checked. E.g. it requires the mentioned names to exist. This doesn't compile (cf. https://godbolt.org/g/iCRPDv):

#ifdef HAVE_FOO
constexpr bool have_foo = true;
void foo();
#else
constexpr bool have_foo = false;
#endif

void f() {
  if constexpr (have_foo) {
    foo();
  }
}

So, the frontend parses all branches anyway. It should be able to see that _2 and _3 are referenced in f<1>().
Comment 4 Marek Polacek 2019-08-16 19:00:56 UTC
This particular problem was fixed in r269433, because the condition is not dependent.  I'll add the test.  For another problem not fixed by this rev see PR81676.
Comment 5 Marek Polacek 2019-08-16 20:41:14 UTC
Author: mpolacek
Date: Fri Aug 16 20:40:36 2019
New Revision: 274587

URL: https://gcc.gnu.org/viewcvs?rev=274587&root=gcc&view=rev
Log:
PR c++/85827
g++.dg/cpp1z/constexpr-if29.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/cpp1z/constexpr-if29.C
Modified:
    trunk/gcc/cp/ChangeLog
Comment 6 Marek Polacek 2019-08-16 20:41:31 UTC
Done.