Bug 24536 - Cannot resolve include file
Summary: Cannot resolve include file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Hardware S60 3rd edition
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 27065
  Show dependency treegraph
 
Reported: 2009-03-11 19:17 PDT by Norbert Leser
Modified: 2009-07-13 11:35 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Norbert Leser 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.
Comment 1 Norbert Leser 2009-03-11 19:18:03 PDT
Created attachment 28512 [details]
Proposed fix for bug 24536
Comment 2 Mark Rowe (bdash) 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.
Comment 3 Mark Rowe (bdash) 2009-03-11 19:36:33 PDT
Your changelog should also have a name rather than Nokia User.
Comment 4 Norbert Leser 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.
Comment 5 Mark Rowe (bdash) 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.
Comment 6 Darin Adler 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.
Comment 7 Norbert Leser 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.
Comment 8 Norbert Leser 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
Comment 9 Holger Freyther 2009-05-11 09:23:25 PDT
In the future please add the bug number to the changelog. Landed in r43495.