[[clang::always_destroy]] attribute while emitting exit-time-destructors diagnostic

[ycdtosa]

Compiling this code with the -Wexit-time-desctuctors diagnostic enabled will raise the warning, for good reason.

static Foo ResourceHandler;

here is the output

[build] /somefile.cpp:X:14: warning: declaration requires an exit-time destructor [-Wexit-time-destructors]
[build]   static Foo ResourceHandler;
[build]              ^

However, being happy with the destructor call on exit time, not only do I want to silence the warning but to flag the variable accordingly. For that I add the [[clang::always_destroy]] attribute to the static variable.

[[clang::always_destroy]]
static Foo ResourceHandler;

This is meant to tell the compiler that I am aware of the exit time destructor and willing to pay the price, and properly release the resource.

The [[always_destroy]] attibute documentation states that it

specifies that a variable with static or thread storage duration should have its exit-time destructor run. This attribute is the default unless clang was invoked with -fno-c++-static-destructors.

I believe adding explicitly the attribute should be enough to stop the compiler from emitting the diagnostic, but it is not the case.

[shafik]

I think we just need to add a check for hasAttr<AlwaysDestroyAttr>() to here:

llvm-project/clang/lib/Sema/SemaDeclCXX.cpp

Lines 16119 to 16121 in feea5db

	// Emit warning for non-trivial dtor in global scope (a real global,
	// class-static, function-static).
	Diag(VD->getLocation(), diag::warn_exit_time_destructor);

wdyt @AaronBallman

[AaronBallman]

Yes, I think that should do it, however, it's worth noting that there's some confusion here with our diagnostics: https://godbolt.org/z/nsMhGrxGq

I think that attribute should silence both diagnostics, but... why do we have two diagnostics to tell the user the same information?

[ycdtosa]

@AaronBallman. There is not much about [global constructors] (https://clang.llvm.org/docs/DiagnosticsReference.html#wglobal-constructors) diagnostics in the Diagnostics Reference at the moment.

[ycdtosa]

Also clang/test/SemaCXX/no_destroy.cpp should be updated.

[AaronBallman]

@AaronBallman. There is not much about [global constructors] (https://clang.llvm.org/docs/DiagnosticsReference.html#wglobal-constructors) diagnostics in the Diagnostics Reference at the moment.

Yeah, it seems this feature was added long ago: 47e40931
And the exit-time destructor was added not long after: 98766db

and there's no review or explanation as to why we wanted to issue the same diagnostic twice here. :-/