On 5/31/19 10:12 AM, Sean Gillespie wrote:
On Thu, May 30, 2019 at 4:28 PM Martin Sebor <mse...@gmail.com> wrote:
On 5/28/19 3:30 PM, Sean Gillespie wrote:
This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. This new warning fires
whenever a thread_local or global variable is declared whose type has a
non-trivial constructor or destructor. When faced with both a constructor
and a destructor, the error message mentions the destructor and is only
fired once.
This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.
Bootstrapped and regression-tested on x86-64 linux, new tests passing.
I can't tell from the Clang online manual:
Is the warning meant to trigger just for globals, or for all
objects with static storage duration regardless of scope (i.e.,
including namespace-scope objects and static locals and static
class members)?
"Requires a constructor to initialize" doesn't seem very clear
to me. Is the warning intended to trigger for objects that
require dynamic initialization? If so, then what about dynamic
intialization of objects of trivial types, such as this:
static int i = std::string ("foo").length ();
or even
static int j = strlen (getenv ("TMP"));
If these aren't meant to be diagnosed then the description should
make it clear (the first one involves a ctor, the second one does
not). But I would think that if the goal is to find sources of
dynamic initialization then diagnosing the above would be useful.
If so, the description should make it clear and tests verifying
that it works should be added.
Martin
The answer to both of those is yes. Clang warns on both of these with
-Wglobal-constructors because the goal of -Wglobal-constructors is to
catch and warn against potential life-before-main behavior.
Based on Marek's feedback I'm about to push another patch that also
warns on your above two examples and adds tests for them. The idea
is that any C++-level dynamically initialized globals get warned. Constexpr
constructors or things that otherwise can be statically initialized
are not warned.
That makes sense.
Regarding the doc description, how about something like this:
"Warns whenever an object with static storage duration requires
dynamic initialiation,
through global constructors or otherwise."
This makes it clearer -- i.e, global constructors refers to the ELF
concept rather than to the C++ one. What does the otherwise part
refer to? Some non-ELF mechanism?
PS Dynamic initialization can be optimized into static
initialization, even when it involves a user-defined constructor.
If the purpose of the warning is to find objects that are
dynamically initialized in the sense of the C++ language then
implementing it in the front-end is sufficient. But if the goal
is to detect constructors that will actually run at runtime (i.e.,
have a startup cost and may have dependencies on one another) then
it needs to be implemented much later.
Clang sticks to the C++ language definition of dynamic initialization for the
purposes of this warning and I think we should as well. Without runtime checks
this is a best-effort warning, but it is capable of catching a bunch
of the common
ways that you can get constructors running on startup, which is the goal.
It might be worth mentioning that in the manual, perhaps along with
an example showing an instance of the warning that will most likely
be a true positive vs a false positive. E.g., something like this:
const char* const tmp = getenv ("TMP");
vs
const int n = strlen ("123"); // initialized statically
Martin
