On 08/24/2010 08:27 PM, Amos Jeffries wrote:
On Tue, 24 Aug 2010 19:56:54 -0600, Alex Rousskov
<rouss...@measurement-factory.com> wrote:
On 08/24/2010 07:25 PM, Henrik Nordström wrote:
tis 2010-08-24 klockan 19:15 -0600 skrev Alex Rousskov:
The current header sequence (somewhere) violates the squid-then-sys
rule
and causes the problem. A header sequence that follows the
squid-then-sys rule will not cause the problem. I suspect such
sequence
does not exist (beyond one file scope) because some Squid headers have
to include system headers.
And I say the opposite. The sequence that can fork is sys headers first
then squid headers with #undef. The keywords are used both in the squid
header and in squid code.
Note that I was not arguing for one sequence or the other. I was just
answering Amos' question.
Okay. Thought I was going crazy there for a minute. The sequence is not my
creation. AFAIK, it was designed explicitly to highlight such clashes as
these and ensure the compiler warnings/errors point at the local copy as
the faulty first-define which the dev at least has a hope of fixing easily.
The wiki documents it for .cc but leaves .h unstated, although the
minimal-symbols principal covers it for .h.
I was planning to get someone to make a source format enforcer to check
the sequence was consistent. If we agree its a good or bad idea to keep up
now is a good time to discuss that.
squid heders first then sys headers renders the squid code using these
members directly broken.
In this particular case, with the compilers we know about, the squid
code using these members directly works fine. This is because the system
headers define a macro with a parameter:
#define major(foo) gnu_craft_major_device(foo)
The compilers we tested with do not substitute "major" unless it looks
like a function call:
http_ver.major = 0; // this is fine
this->major< that.major; // this is fine too
HttpVersion(): major(0) // this breaks
HttpVersion(...): major(that.major) // this would break too
This is why the problem is not visible until you try to polish the
HttpVersion class.
However, I would not be surprised if some other compilers behave
differently so, ideally, we should solve the problem for good.
Which *good* fix means renaming however its looked at. What about:
theMajor and theMinor ?
AFAICT from theory they are only needed public by code which converts
to/from a string. Maybe not even that. If that can be avoided major_ and
minor_ private members might be available.
BTW: the polish update should include my earlier comments about the
agnostic naming and SourceLayout position of the class itself.
By "include comments", do you mean implement them or add a source code
// TODO comment?
Cheers,
Alex.
