Clang permits `int x = x`

[cjdb]

The following program should issue a diagnostic:

int main()
{
  int x = x;
}

I think this is a serious enough problem to be an error by default, even if it's well-formed per C and C++. I am not sure if the following is an exception, but it's at least weird?

int main()
{
  void* ptr = &ptr;
}

Clang Tidy catches this, but Clang really ought to as well.

Repro: https://godbolt.org/z/81andM3jb

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[cjdb]

Thanks to @zeroxia for asking about void* ptr = &ptr; on the #include <C++> Discord.

[cpplearner]

I believe that int x = x; is intentionally permitted because some projects use this pattern to mark an uninitialized variable, where int x; would produce a warning and is more likely to be a mistake.

[DimitryAndric]

It used to be that the <windows.h> header (or one that it transitively includes) defined the macro UNREFERENCED_PARAMETER(p) effectively as (p) = (p), but I see that is no longer the case in recent Windows SDKs. But indeed, int x = x; as a declaration might be suspect, since it is not really defined what the 'previous' value of x was, before the assignment? :)

[shafik]

I would note this example from basic.scope.pdecl:

[Example 1: unsigned char x = 12; { unsigned char x = x; } Here, the initialization of the second x has undefined behavior, because the initializer accesses the second x outside its lifetime ([basic.life]). — end example]

and note this is hard error if we use constexpr since it is undefined behavior:

int main() {
  constexpr int x = x;
}

godbolt: https://godbolt.org/z/rq47z3rh5

[AaronBallman]

I believe that int x = x; is intentionally permitted because some projects use this pattern to mark an uninitialized variable, where int x; would produce a warning and is more likely to be a mistake.

No comment on "intentionally permitted" but this a pattern I've run into before, especially in older C code bases. Once upon a time, not all compilers supported (void)var; as a way to silence the warning. However, I'm more used to seeing it with parameters, as in:

void func(int param) {
  param = param;
}

instead of with a variable, because C before C2x doesn't allow you to elide the parameter name in a function definition. So it may be possible this could be an on-by-default warning, but it'd be good to try this out over a large corpus of older code (like trying to build all the packages in a distro).

[shafik]

I knew I saw this before: https://github.com/llvm/llvm-project/issues/19252#issuecomment-980928829

So @zygoloid said this was deliberate but perhaps we might want to reconsider that.

[zygoloid]

The choice to do this precedes my involvement with Clang, and I can't speak to how important it is to special-case this pattern, but it is definitely intentional that we currently don't warn on it. See for example https://github.com/llvm/llvm-project/blob/c1e252417e399557157e977d8bfb3033bd567f46/clang/lib/Analysis/UninitializedValues.cpp#L906

[cjdb]

The choice to do this precedes my involvement with Clang, and I can't speak to how important it is to special-case this pattern, but it is definitely intentional that we currently don't warn on it. See for example

https://github.com/llvm/llvm-project/blob/c1e252417e399557157e977d8bfb3033bd567f46/clang/lib/Analysis/UninitializedValues.cpp#L906

I figured that adding a new warning would be straightforward; I didn't expect it to be that easy though. I suspect that there are some more esoteric use-cases like

int x = f(x);

that won't be caught here.

[shafik]

For further reference: https://lists.isocpp.org/core/2019/12/8020.php

and also see: https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2256

and: https://github.com/cplusplus/draft/issues/3578

[nickdesaulniers]

Strange, if a macro expansion is involved, we seem to warn about this:

#define foo(x) ({ int __old = x; __old; })

int bar (void) {
    int __old = 42;
    return foo(__old);
}

<source>:5:16: warning: variable '__old' is uninitialized when used within its own initialization [-Wuninitialized]
    5 |     return foo(__old);
      |            ~~~~^~~~~~
<source>:1:31: note: expanded from macro 'foo'
    1 | #define foo(x) ({ int __old = x; __old; })
      |                       ~~~~~   ^

[zygoloid]

@nickdesaulniers The difference isn't due to the macro, it's due to the read of __old at the end of the statement-expression. We don't warn on int x = x; but we do warn on int x = x; use(x);.