Bug 24536

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:

Version:

528+ (Nightly build)

Hardware:

S60 Hardware

OS:

S60 3rd edition

Bug Depends on:

Bug Blocks:

27065

Attachments:

Description	Flags
Proposed fix for bug 24536	mrowe: review-
Cleaned-up patch for 24536	mrowe: review-
Updated patch for bug 24536	darin: review+

Norbert Leser

Reported 2009-03-11 19:17:16 PDT

Symbian compilers cannot resolve certain missing include files in first compile pass. The specific header files missing can only indirectly be resolved. The proposed fix is to explicitly declare the header files in the respective files where they are required. I propose to add these unconditionally, since it is unambiguous for any other compilers as well. See attached patch.

Attachments
Proposed fix for bug 24536 (1.27 KB, patch) 2009-03-11 19:18 PDT, Norbert Leser	mrowe: review-	Details Formatted Diff Diff
Cleaned-up patch for 24536 (1.14 KB, patch) 2009-03-13 10:25 PDT, Norbert Leser	mrowe: review-	Details Formatted Diff Diff
Updated patch for bug 24536 (963 bytes, patch) 2009-05-08 15:34 PDT, Norbert Leser	darin: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Norbert Leser

Comment 1 2009-03-11 19:18:03 PDT

Created attachment 28512 [details] Proposed fix for bug 24536

Mark Rowe (bdash)

Comment 2 2009-03-11 19:30:30 PDT

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.

Mark Rowe (bdash)

Comment 3 2009-03-11 19:36:33 PDT

Your changelog should also have a name rather than Nokia User.

Norbert Leser

Comment 4 2009-03-13 10:25:54 PDT

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.

Mark Rowe (bdash)

Comment 5 2009-03-13 11:41:02 PDT

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.

Darin Adler

Comment 6 2009-03-13 12:18:41 PDT

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.

Norbert Leser

Comment 7 2009-03-13 12:32:38 PDT

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

Norbert Leser

Comment 8 2009-05-08 15:34:56 PDT

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

Holger Freyther

Comment 9 2009-05-11 09:23:25 PDT

In the future please add the bug number to the changelog. Landed in r43495.

Note You need to log in before you can comment on or make changes to this bug.