Why does msvc compiler issue unused variable when explicitly calling a destructor

Question

With the following code:

struct Foo {};

template<class T>
void Destruct(T *obj)
{
    obj->~T();
}

int main(int /*argc*/, const char * /*argv*/[])
{
    char buffer[sizeof(Foo)];
    Destruct((Foo*)buffer);
    return 0;
}

Visual Studio 2015 will issue a warning for unreferenced parameter:

warning C4100: 'obj': unreferenced formal parameter

Is this a legitimate warning or a bug in the compiler?

Online Repro here: https://godbolt.org/z/xq96GU

Edit: updated the sample to a full example

Edit 2: you need to enable /W4 for this to occur in visual studio 2015, /W3 is not enough; Also confirmed this does not occur in 2017.

Edit 3: For the CNR, here is the output from command line with all arguments used to repro:

>"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\CL.exe" /W4 test.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24215.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cpp
test.cpp(4): warning C4100: 'obj': unreferenced formal parameter
test.cpp(12): note: see reference to function template instantiation 'void Destruct<Foo>(T *)' being compiled
        with
        [
            T=Foo
        ]
Microsoft (R) Incremental Linker Version 14.00.24215.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
test.obj

Edit 4 Added sample reproduction on godbolt.org Edit 5 Actually /W4 is enough to reproduce, /Wall is not necessary

Visual Studio 2015 is pretty old by now. It might be a compiler bug which has been corrected in the meantime. — Jabberwocky, Jan 24 '19 at 13:09
Cannot reproduce with your updated code and `/Wall` under Visual Studio 2017. See my previous comment. BTW what is this rather strange code supposed to do? — Jabberwocky, Jan 24 '19 at 13:17
I acknowledge that the warning is not issued in 2017. This is repro with CL.exe `19.00.24215.1` — GaspardP, Jan 24 '19 at 13:21
@GaspardP it's almost certainly a compiler bug. Consider upgrading to a more recent compiler. Current version of cl is 19.16.27025.1 or even more recent as I don't have the very latest Visual Studio 2017 — Jabberwocky, Jan 24 '19 at 13:25
@Jabberwocky thanks, I was wondering if I was missing something (obvious or not) and your feedback is reassuring. — GaspardP, Jan 24 '19 at 13:28
FWIW the code is UB so anything is good formally. `buffer` is not a `foo` and coercing it to be one violates strict aliasing. — NathanOliver, Jan 24 '19 at 13:36
I'm not going to start an argument over this @NathanOliver but I'd be very curious to know how one does placement new without coercing types in C++. — GaspardP, Jan 24 '19 at 13:52
@GaspardP Using placement new works because it actually creates an object (starts its lifetime) of that type in the storage area. You haven't done that here, you just cast the pointer. That means right now you don't actually have a live `Foo` object to work on. Had you done `char buffer[sizeof(Foo)]; auto fooptr = new (buffer) Foo; Destruct(fooptr);` there would be no UB. — NathanOliver, Jan 24 '19 at 13:56
Obligatory warning _[sic]_ not to bother with `/Wall`: https://stackoverflow.com/a/54162034/560648 — Lightness Races in Orbit, Jan 24 '19 at 14:05
@NathanOliver gotcha, in the code above I skipped construction to focus on the problem and this forced me to cast, but you are right - normally the placement new will do the cast for you in a defined way. — GaspardP, Jan 24 '19 at 14:17
Yep. I figured it isn't actually the issue here but I wanted to point it out. — NathanOliver, Jan 24 '19 at 14:18
@LightnessRacesinOrbit Thanks and I agree - there isn't much that compiles even in the standard headers with `/Wall`. However, I actually realized `/W4` is enough to trigger this. — GaspardP, Jan 24 '19 at 15:34

score 1 · Accepted Answer · answered Jan 24 '19 at 13:57

1

IMHO it is just an interpretation.

In the specialized function void Destruct<Foo>(Foo *obj), the passed object is only used to call an empty destructor. No virtual is involved, no variable change value, no IO occurs. Long story short, nothing directly or indirectly observable can result from that call, so the compiler can optimize it out. So the compiler is correct when it says that the passed object is unused in that specific specialization.

In addition, few diagnostics are required per standard, but AFAIK nothing prevents a compiler to emit warnings for suspect code. This is exactly what happens here: the compiler warns you that with your code the call to the destructor will be a no-op and because of that the code is suspect. But there is no compilation error, and I assume that the program will run with no problem, despite a cast that violates the strict aliasing rule.

So my opinion is that this warning is far from required, and compilers that do not emit it are correct, but as the code is strange, you cannot blame a compiler that kindly warns you by saying programmer, I hope you know what you are doing here because I cannot understand the rationale behind it...

answered Jan 24 '19 at 13:57

Serge Ballesta

143,923
11
122
252

You are correct, actually the warning goes away with a trivial destructor: https://godbolt.org/z/JZmS6N - adding `~Foo() {}` is enough. – GaspardP Jan 24 '19 at 14:01
I don't agree. The warning suggests that the variable isn't used in the source, so can be removed (without changing the program). But if you'd do that, you get an compiler error (`obj: undeclared identifier`). So the warning is, IMO, wrong. Just because it can be optimized away in this specific case, doesn't mean my code doesn't use it! – king_nak Jan 24 '19 at 14:04
As for the strangeness of the code, don't worry, this is part of a larger code base. It was simplified to the minimum working pieces that reproduced the problem. The rationale behind it is managed memory allocations. Allocate a large block of heap memory, placement new a bunch of objects in that block, use them, destroy them explicitly (this code) and free the heap buffer. – GaspardP Jan 24 '19 at 14:07
1

@king_nak: In this code snippet `int i; int j; j=0; i=j; printf("%d", j);`, `i` is indeed unused. And you actually get a compilation error if you remove the line `int i;`. – Serge Ballesta Jan 24 '19 at 14:09
@king_nak I am with you on this - the variable is "used", optimized away because there is no effective destructor. The caller is using the variable, but it has no effect. There is no point for the compiler to tell us that a destructor has no effect, this happens all the time. So I'd say it's a compiler error, but I understand how they interpret that as a unused variable (and I can now work around it) – GaspardP Jan 24 '19 at 14:10
@SergeBallesta good point, didn't think of such a case. Howerver in your case, `i` is only written to, its value is never used. In `obj->`, the value *is* used. – king_nak Jan 24 '19 at 14:41

Why does msvc compiler issue unused variable when explicitly calling a destructor

1 Answers1