Bug 24536 – Cannot resolve include file

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Norbert Leser]

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.

[Norbert Leser]

Created attachment 28512 [details]
Proposed fix for bug 24536

[Mark Rowe (bdash)]

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)]

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

[Norbert Leser]

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 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]

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]

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

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]

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