On Tue, 24 Aug 2010 20:45:54 -0600, Alex Rousskov <[email protected]> wrote: > On 08/24/2010 08:27 PM, Amos Jeffries wrote: >> On Tue, 24 Aug 2010 19:56:54 -0600, Alex Rousskov >> <[email protected]> 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? >
I mean implement. It's polish after all. Amos
