On 03/16/2016 11:33 AM, Kinkie wrote: >>> Will it be initialized at all? I'd expect that fromHexTable, which is >>> const and POD be simply laid out in the data segment and not require >>> initialization at all. >> >> Are you implying that >> >> (a) fromHexTable is a C++11 constexpr _and_ >> (b) constexpr cannot suffer from initialization order problems? >> >> If yes, then you should declare fromHexTable as constexpr so that >> somebody does not accidentally change it to be something else. This will >> make (a) a strong explicit statement rather than an implied and fragile >> implication.
> both are list of constants. No calls to any code of any kind. Yes, today, they are, but there is no indication that it is an important property or invariant. Somebody can replace those initialization with a function call tomorrow and nobody may notice that mistake until bug reports start coming in. > toHexTable can't be declared constexpr: g++ complains that the standard > doesn't allow that for strings. Which, to me, is an indication that we should not _assume_ anything about its initialization timing! We have to be careful here. BTW, "constexpr const char *toHexTable" appears to compiler fine for me, but perhaps my g++ is too old or this is the wrong way to add constexpr to toHexTable? There is a somewhat related discussion to this at http://stackoverflow.com/questions/30561104/const-constexpr-char-vs-constexpr-char >> However, I cannot find any C++ rule that guarantees the behavior you >> expect -- your fromHexTable (even if you add constexpr to it) does not >> seem to match any of the three items at: >> >> http://en.cppreference.com/w/cpp/language/constant_initialization > To me they seem both cases of (quote) > 3) Static or thread-local object (not necessarily of class type), That rule did not seem to match for me because the tables in the v2 patch I was reviewing where not marked as "static". You have changed their declarations now so the above does seem to apply. Thank you. Please note that I am _not_ saying that rule #3 did not apply before your change. I am saying that it did not seem to apply. Writing correct code is only a part of what we have to do; we have to make our code appear to be correct to others. > This stems from my understading of the meaning of the ".data" section > of ELF files, which may be partial or incomplete. ... and, more importantly, has nothing to do with C++. The .data section may store all our constants, but that does not mean those constants cannot be copied to some of our objects "dynamically", at some semi-random time after the program starts. > Unfortunately initialization rules are quite hard for me to understand yet. They are hard for everybody! I am surprised you volunteered to fix an initialization problem, but since you have done so, it is not fair to avoid the rules that are required to fix it (including proving that your fix is correct). >>> +const int16_t fromHexTable[256] = { >> >> AFAICT, this needs to be "static" and should be "constexpr" to (a) >> guarantee constant initialization and (b) minimize the chances of >> somebody changing it to something that will not be initialized at >> "constant initialization" time. Please correct me if I am wrong. > Aren't variables not declared extern marked static by default? Good question [that we should not be asking IMHO]. AFAICT[1], by default, variables have external linkage (although C++ complicates that further) and static storage duration. Does the "Static or thread-local object" phrase in the C++ constant initialization rule #3 at [2] talk about storage duration or linkage? I think it is about storage duration. [1] http://stackoverflow.com/questions/3281925/what-is-default-storage-class-for-global-variables [2] http://en.cppreference.com/w/cpp/language/constant_initialization > Sure can do but it should be redundant. It may be redundant for the compiler, but if you think "static" is redundant for us, then either you have no respect for the hours wasted discussing three versions of your patches or you think I was missing something that would be obvious to everybody else. I still do not understand why the tables should not go inside the functions that use them, eliminating all questions. However, I think patch v3 does not suffer from table initialization order problems. What about CharacterSet globals like Rfc1738::Unsafe? Do you have reasons to believe they will be initialized before any possible first use? For example, if some Foo.cc contains the following global, will it always be initialized correctly? #include "anyp/Rfc3986.h" const CharacterSet FooSet = Rfc3986::Reserved + Rfc1738::Ctrls; I do not think so because none of the three constant initialization rules[2] appear to apply to Rfc3986::Reserved (for example). Thank you, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
