_NODISCARD should appear before __declspec

[StephanTLavavej]

@JonCavesMSFT reported:

My recent PR to add a warning for uses of [[nodiscard]] that are ignored, C5240, has shown up an issue with the self-build:

The header file <memory_resource> contains the following line:

STL/stl/inc/memory_resource

Line 47 in 52b7b32

extern "C" _CRT_SATELLITE_1 _NODISCARD memory_resource* __cdecl null_memory_resource() noexcept;

Which causes the compiler to emit the new warning: and if you currently call the function as follows:

void f()
{
    null_memory_resource();
}

Then no warning about discarding the return value is emitted.

If I change the declaration of null_memory_resource to:

extern "C" _NODISCARD _CRT_SATELLITE_1 memory_resource* __cdecl null_memory_resource_2() noexcept;

Then the new warning is not emitted and the call to the function does emit, "C4834: discarding return value of function with 'nodiscard' attribute".

To unblock the self-build I will, for now, make this warning off-by-default.

_CRT_SATELLITE_1 is defined to be __declspec(dllexport) or nothing in crtdefs.h; the issue is with an attribute like [[nodiscard]] occuring after a __declspec.

I observe that this also affects a lot of Special Math machinery.

[StephanTLavavej]

Note that this isn't an issue for users, because _CRT_SATELLITE_1 is defined to be nothing (after our overhaul to avoid __declspec(dllimport) for functions - it remains only for classes and data).

I found 2 occurrences of __declspec before [[deprecated]] on classes:

STL/stl/inc/xlocale

Lines 847 to 849 in 52b7b32

	template <>
	class _CRTIMP2_PURE_IMPORT_UNLESS_CODECVT_ID_SATELLITE _CXX20_DEPRECATE_CODECVT_FACETS
	codecvt<char16_t, char, mbstate_t> : public codecvt_base {

It appears that the attribute takes effect either way here, but I think we should make this consistent.

[JonCavesMSFT]

Classes are different: there is no decl-specifier-seq for the attribute to appertain to (the problem with functions is that the greedy decl-specifier-seq grabs the attribute instead of leaving it for the complete declaration). So for a class the order doesn't matter but I agree being consistent is good.

[StephanTLavavej]

Ah, thanks for the explanation. I had to revert that change because of Clang; we'll need to report a bug if we want to clean that up.