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. Amos
