Request to make ACE headers files more compatible with MSVC compiler and high warning level

[vbaderks]

Version

ACE 6.5.1

Host machine and operating system

Windows 10

Target machine and operating system (if different from host)

Windows 10

Compiler name and version (including patch level)

Visual Studio 2019 16.3.6 Microsoft (R) C/C++ Optimizing Compiler Version 19.23.28106.4 for x86

The problem effects:

Compiling our application.

Synopsis

We are compiling our application with the highest warning level enabled and the Microsoft Code Analysis option enabled.

Description

Because we compile with the code analysis option enabled we are getting a warnings that are coming from the ACE include headers. We have already reported this problem to Microsoft and asked for an option to exclude certain directories from their analysis. At the moment this option is not available. This forces us to put the ACE headers between #pragma pop/disable/push warnings constructions. The code below is from our pch.h (precompiler header file).

#pragma warning(push)
#pragma warning(disable : 4548)  // expression before comma has no effect; expected expression with side - effect
#pragma warning(disable : 4619)  // #pragma warning: there is no warning number '4231'
#pragma warning(disable : 4996)  // This function or variable may be unsafe (_CRT_SECURE_NO_WARNINGS).
#pragma warning(disable : 6011)  // Dereferencing NULL pointer
#pragma warning(disable : 6320)  // Exception-filter expression is the constant EXCEPTION_EXECUTE_HANDLER. This might mask exceptions that were not intended to be handled.
#pragma warning(disable : 6387)  // 'file_handle' could be '0' : this does not adhere to the specification for the function 'CreateFileMappingA'.
#pragma warning(disable : 26135) // Missing annotation _Acquires_lock_(*m) at function 'ACE_OS::thread_mutex_xxx
#pragma warning(disable : 26400) // Do not assign the result of an allocation or a function call with an owner<T> return value to a raw pointer, use owner<T> instead(i.11).
#pragma warning(disable : 26401) // Do not delete a raw pointer that is not an owner<T>(i.11).
#pragma warning(disable : 26409) // Avoid calling new and delete explicitly, use std::make_unique<T> instead(r.11).
#pragma warning(disable : 26430) // Symbol is not tested for nullness on all paths(f.23).
#pragma warning(disable : 26433) // Function '' should be marked with 'override'(c .128).
#pragma warning(disable : 26440) // Function '...' can be declared 'noexcept' (f.6).
#pragma warning(disable : 26443) // Overriding destructor should not use explicit 'override' or 'virtual' specifiers(c .128).
#pragma warning(disable : 26447) // The function is declared 'noexcept' but calls function 'xxx' which may throw exceptions (f.6).
#pragma warning(disable : 26460) // The reference argument '.' for function '.' can be marked as const (con.3).
#pragma warning(disable : 26465) // Don't use const_cast to cast away const. const_cast is not required; constness or volatility is not being removed by this conversion (type.3).
#pragma warning(disable : 26471) // Don't use reinterpret_cast. A cast from void* can use static_cast (type.1).
#pragma warning(disable : 26473) // Don't cast between pointer types where the source type and the target type are the same (type.1).
#pragma warning(disable : 26476) // Expression / symbol 'value' uses a naked union 'union semun' with multiple type pointers : Use variant instead(type.7).
#pragma warning(disable : 26477) // Use 'nullptr' rather than 0 or NULL(es.47).
#pragma warning(disable : 26495) // Variable is uninitialized.Always initialize a member variable(type.6)
#include <ace/Future.h>
#include <ace/Signal.h>
#include <ace/Synch.h>
#include <ace/Task.h>
#include <ace/ace.h>
#include <ace/os.h>
#pragma warning(pop)

Sample fix/ workaround

My original intend was to create and provide pull requests for these warnings. I will however stop working on this application and will therefore not be able to do this anymore. In general there are 2 solutions for these warnings:

• disable and enable them in the ACE headers themselves (which is already done for other MSVC warnings)
• solve the root cause of the warnings. Inline functions are causing most of these warnings.

Most of these warnings are caused by the fact that we are using C++17 and follow the C++ core guidelines (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) and the inline ACE functions are compiled in scope of our application compiler settings. The intend of this issue is to communicate this problem, not to offer a solution direction.

[knutpett]

I've also noticed that ACE & TAO produces (a lot of) compiler warnings. What is the policy on this?

[jwillemsen]

@knutpett We prefer warning free code but lack sponsoring to work ourselves on the warnings at this moment. Remedy IT has solved the warnings in the ACE and TAO core during the development of TAOX11 and AXCIOMA. The current builds are visible online at the ACE/TAO, TAOX11, and AXCIOMA scoreboards.

After that initial effort newer compilers/OS releases are done which trigger new warnings. We welcome fixes to the code to resolve warnings, we prefer real fixes, not just suppress warnings globally. Given the huge amount of compilers ACE/TAO supports it can be tricky to solve warnings, a fix on one platform could trigger new ones on others.

When you submit PRs to solve warnings try to make smaller PRs which are easily reviewed. Review of a pull request takes time and is best effort when there is no sponsoring for the work involved at our side.