Summary: | Cannot resolve include file | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Norbert Leser <norbert.leser> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | hausmann, laszlo.gombos, mrowe | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | S60 Hardware | ||||||||||
OS: | S60 3rd edition | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 27065 | ||||||||||
Attachments: |
|
Description
Norbert Leser
2009-03-11 19:17:16 PDT
Created attachment 28512 [details] Proposed fix for bug 24536 Why are these extra includes necessary? RefPtr.h has no dependency on the contents of PassRefPtr.h, so I'm not sure why your patch adds that include. The change to ProfileGenerator.cpp also appears to be bogus, as it already includes Profile.h. Your changelog should also have a name rather than Nokia User. Created attachment 28582 [details]
Cleaned-up patch for 24536
According to review comments:
- I removed the change for RefPtr.h (we will resolve this with a compiler fix rather than webkit code change).
- I removed the second include Profile.h. This was omitted in the initial patch. The intention was to change the order of header includes, and not to add a second Profile.h. Without this changed order, our compiler cannot resolve WTF::PassRefPtr<JSC::Profile>, as referenced in ProfileGenerator.h.
Comment on attachment 28582 [details]
Cleaned-up patch for 24536
In a .cpp file named Foo our policy is to include config.h first, followed by Foo.h, followed by any remaining headers that the file needs. It seems to me that if your compiler is choking on this code for some reason, it is buggy and should be fixed.
Actually, looking at ProfileGenerator, I don’t understand why this line of code compiles: PassRefPtr<Profile> profile() const { return m_profile; } Without a definition of the class Profile, this should fail. I have no idea why this is working with other compilers. The correct fix for this is to change ProfileGenerator.h to include Profile.h, though, not to reorder includes ProfileGenerator.cpp. (In reply to comment #6) > Actually, looking at ProfileGenerator, I don’t understand why this line of code > compiles: > > PassRefPtr<Profile> profile() const { return m_profile; } > > Without a definition of the class Profile, this should fail. I have no idea why > this is working with other compilers. The correct fix for this is to change > ProfileGenerator.h to include Profile.h, though, not to reorder includes > ProfileGenerator.cpp. > That is fine with me. That would have been my alternate suggestion as well, if you want to keep the order in the implementation file. I believe, the class Profile forward declaration will be redundant then and can be removed. Created attachment 30144 [details] Updated patch for bug 24536 Updated patch according to Darin's suggestion of adding Profile.h to ProfileGenerator.h instead of ProfileGenerator.cpp |