29106 2009-09-09 14:45:51 -0700 [Qt] make CachedResourceHandle.h compile in winscw Symbian compiler. 2009-10-13 22:24:14 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) S60 Emulator S60 3rd edition RESOLVED FIXED Qt P2 Normal --- 27065 1 yongjun.zhang webkit-unassigned ariya.hidayat hausmann koshuin laszlo.gombos oldest_to_newest 145946 0 yongjun.zhang 2009-09-09 14:45:51 -0700 In Winscw compiler, when CachedResourceHandle<T> is used as a class member, the compiler will aggressively resolve the inheritance of R* when constructor CachedResourceHandle(R* res) is inlined, which means the class definition of T must come before CachedResourceHandle<T> and the header file of class T must be included wherever CachedResourceHandler<T> is defined. To avoid unnecessary inclusion of header files, the right workaround would be removing inline for CacheResourceHandler<T>(R*) in winscw compiler, while keep it inline for other compilers to avoid performance penalty. 146070 1 39320 yongjun.zhang 2009-09-09 21:23:20 -0700 Created attachment 39320 don't inline CachedResourceHandle<T>(R*) for winscw compiler. 146150 2 ariya.hidayat 2009-09-10 03:13:32 -0700 Nitpick: technically [Qt] is not needed, right? 146298 3 39320 eric 2009-09-10 12:45:22 -0700 Comment on attachment 39320 don't inline CachedResourceHandle<T>(R*) for winscw compiler. I'm not sure how it makes sense to ever have a non-inline constructor definition in a header file. :) 146321 4 hausmann 2009-09-10 13:39:29 -0700 From what I understand from Janne the outlining shouldn't be necessary if RefPtr is patched appropriately. Janne, can you elaborate? :) 146327 5 koshuin 2009-09-10 14:05:01 -0700 Right, I'll post my patch first thing in the morning. So we can fight it over tomorrow :) 146441 6 yongjun.zhang 2009-09-10 20:02:52 -0700 (In reply to comment #4) > From what I understand from Janne the outlining shouldn't be necessary if > RefPtr is patched appropriately. Janne, can you elaborate? :) RefPtr is not related here. The root cause is the base constructor requires CachedResource* argument. When CachedResourceHandle<T>(R*) is used, the compiler tries to convert R to CachedResource in the base class's ctor, which fails compiling when class R's header file is not included. Making it not inline can defer the type conversion till it is really needed. 146513 7 koshuin 2009-09-11 03:31:36 -0700 > RefPtr is not related here. > > The root cause is the base constructor requires CachedResource* argument. When > CachedResourceHandle<T>(R*) is used, the compiler tries to convert R to Right you are. I had a fix in CachedImage that I had forgotten. Putting the fix into base class is better. Those who still are sceptical: mwccsym2 tries to resolve inlines immediately and here if the constructor is not taken out would give "illegal use of incomplete struct/union/class" - error as the actual class is not yet been defined within the compilation unit, only forward declared. 147486 8 39320 eric 2009-09-15 23:15:33 -0700 Comment on attachment 39320 don't inline CachedResourceHandle<T>(R*) for winscw compiler. See my comments on your other bug about "inline" for templates functions. Style: + CachedResourceHandle(R* res); We need a comment which references the bug. The compiler bug should be linked to from the ChangeLog too. 151355 9 40399 yongjun.zhang 2009-09-30 14:25:43 -0700 Created attachment 40399 modify patch to include Eric's comments. Added links to winscw compiler bug in ChangeLog and header file. 154330 10 40399 commit-queue 2009-10-13 07:27:40 -0700 Comment on attachment 40399 modify patch to include Eric's comments. Rejecting patch 40399 from commit-queue. Patch https://bugs.webkit.org/attachment.cgi?id=40399 from bug 29106 failed to download and apply. 154489 11 40399 laszlo.gombos 2009-10-13 16:39:53 -0700 Comment on attachment 40399 modify patch to include Eric's comments. Will commit manually; patch does not apply cleanly. 154538 12 laszlo.gombos 2009-10-13 22:24:14 -0700 Landed as http://trac.webkit.org/changeset/49556. - part of it was landed previously as http://trac.webkit.org/changeset/49017. 39320 2009-09-09 21:23:20 -0700 2009-09-30 14:25:43 -0700 don't inline CachedResourceHandle<T>(R*) for winscw compiler. CachedResourceHandle.h.patch text/plain 2867 yongjun.zhang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0ODI0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMDktMDktMDkgIFlvbmdqdW4gWmhhbmcgIDx5b25nanVuLnpoYW5n QG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjkxMDYKKyAgICAg ICAgW1F0XSBtYWtlIENhY2hlZFJlc291cmNlSGFuZGxlLmggY29tcGlsZSBpbiB3aW5zY3cgU3lt YmlhbiBjb21waWxlci4KKworICAgICAgICBEb24ndCBpbmxpbmUgY29uc3RydWN0b3IgQ2FjaGVk UmVzb3VyY2VIYW5kbGU8VD4oUiopIHRvIHN0b3Agd2luc2N3CisgICAgICAgIGNvbXBpbGVyIGFn Z3Jlc3NpdmVseSByZXNvbHZlIGluaGVyaXRhbmNlIG9mIGNsYXNzIFIuCisKKyAgICAgICAgQWRk IHBhcmV0aGVuc2lzIGFyb3VuZCAoQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNlOjoqVW5zcGVjaWZp ZWRCb29sVHlwZSkgdG8KKyAgICAgICAgaGVscCB3aW5zY3cgY29tcGlsZXIgcmVzb2x2ZSBjbGFz cyBxdWFsaWZpZXIuCisKKyAgICAgICAgKiBsb2FkZXIvQ2FjaGVkUmVzb3VyY2VIYW5kbGUuaDoK KyAgICAgICAgKFdlYkNvcmU6Ojo6Q2FjaGVkUmVzb3VyY2VIYW5kbGUpOgorCiAyMDA5LTA5LTA4 ICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkg RXJpYyBTZWlkZWwuCkluZGV4OiBXZWJDb3JlL2xvYWRlci9DYWNoZWRSZXNvdXJjZUhhbmRsZS5o Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFdlYkNvcmUvbG9hZGVyL0NhY2hlZFJlc291cmNlSGFuZGxlLmgJKHJl dmlzaW9uIDQ4MjQwKQorKysgV2ViQ29yZS9sb2FkZXIvQ2FjaGVkUmVzb3VyY2VIYW5kbGUuaAko d29ya2luZyBjb3B5KQpAQCAtMzgsNyArMzgsOCBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAg ICAgIGJvb2wgb3BlcmF0b3IhKCkgY29uc3QgeyByZXR1cm4gIW1fcmVzb3VyY2U7IH0KICAgICAg ICAgCiAgICAgICAgIC8vIFRoaXMgY29udmVyc2lvbiBvcGVyYXRvciBhbGxvd3MgaW1wbGljaXQg Y29udmVyc2lvbiB0byBib29sIGJ1dCBub3QgdG8gb3RoZXIgaW50ZWdlciB0eXBlcy4KLSAgICAg ICAgdHlwZWRlZiBDYWNoZWRSZXNvdXJjZSogQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNlOjoqVW5z cGVjaWZpZWRCb29sVHlwZTsKKyAgICAgICAgLy8gUGFyZW50aGVzaXMgaXMgbmVlZGVkIGZvciB3 aW5zY3cgY29tcGlsZXIgdG8gcmVzb2x2ZSBjbGFzcyBxdWFsaWZpZXIgaW4gdGhpcyBjYXNlLgor ICAgICAgICB0eXBlZGVmIENhY2hlZFJlc291cmNlKiAoQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNl OjoqVW5zcGVjaWZpZWRCb29sVHlwZSk7CiAgICAgICAgIG9wZXJhdG9yIFVuc3BlY2lmaWVkQm9v bFR5cGUoKSBjb25zdCB7IHJldHVybiBtX3Jlc291cmNlID8gJkNhY2hlZFJlc291cmNlSGFuZGxl QmFzZTo6bV9yZXNvdXJjZSA6IDA7IH0KIAogICAgIHByb3RlY3RlZDoKQEAgLTU5LDcgKzYwLDcg QEAgbmFtZXNwYWNlIFdlYkNvcmUgewogICAgIHRlbXBsYXRlIDxjbGFzcyBSPiBjbGFzcyBDYWNo ZWRSZXNvdXJjZUhhbmRsZSA6IHB1YmxpYyBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UgewogICAg IHB1YmxpYzogCiAgICAgICAgIENhY2hlZFJlc291cmNlSGFuZGxlKCkgeyB9Ci0gICAgICAgIENh Y2hlZFJlc291cmNlSGFuZGxlKFIqIHJlcykgOiBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UocmVz KSB7IH0KKyAgICAgICAgQ2FjaGVkUmVzb3VyY2VIYW5kbGUoUiogcmVzKTsKICAgICAgICAgQ2Fj aGVkUmVzb3VyY2VIYW5kbGUoY29uc3QgQ2FjaGVkUmVzb3VyY2VIYW5kbGU8Uj4mIG8pIDogQ2Fj aGVkUmVzb3VyY2VIYW5kbGVCYXNlKG8pIHsgfQogCiAgICAgICAgIFIqIGdldCgpIGNvbnN0IHsg cmV0dXJuIHJlaW50ZXJwcmV0X2Nhc3Q8Uio+KENhY2hlZFJlc291cmNlSGFuZGxlQmFzZTo6Z2V0 KCkpOyB9CkBAIC03MCw2ICs3MSwxNiBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgIGJv b2wgb3BlcmF0b3I9PShjb25zdCBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UmIG8pIGNvbnN0IHsg cmV0dXJuIGdldCgpID09IG8uZ2V0KCk7IH0KICAgICAgICAgYm9vbCBvcGVyYXRvciE9KGNvbnN0 IENhY2hlZFJlc291cmNlSGFuZGxlQmFzZSYgbykgY29uc3QgeyByZXR1cm4gZ2V0KCkgIT0gby5n ZXQoKTsgfQogICAgIH07CisJCisgICAgLy8gRG9uJ3QgaW5saW5lIGZvciB3aW5zY3cgY29tcGls ZXIgdG8gcHJldmVudCB0aGUgY29tcGlsZXIgYWdyZXNzaXZlbHkgcmVzb2x2aW5nIAorICAgIC8v IHRoZSBiYXNlIGNsYXNzIG9mIFIqIHdoZW4gQ2FjaGVkUmVzb3VyY2VIYW5kbGVyPFQ+KFIqKSBp cyBpbmxpbmVkLiAgICAKKyAgICB0ZW1wbGF0ZSA8Y2xhc3MgUj4KKyNpZiAhQ09NUElMRVIoV0lO U0NXKQorICAgIGlubGluZQorI2VuZGlmCisgICAgQ2FjaGVkUmVzb3VyY2VIYW5kbGU8Uj46OkNh Y2hlZFJlc291cmNlSGFuZGxlKFIqIHJlcykgOiBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UocmVz KSAKKyAgICB7CisgICAgfQogICAgIAogICAgIHRlbXBsYXRlIDxjbGFzcyBSLCBjbGFzcyBSUj4g Ym9vbCBvcGVyYXRvcj09KGNvbnN0IENhY2hlZFJlc291cmNlSGFuZGxlPFI+JiBoLCBjb25zdCBS UiogcmVzKSAKICAgICB7IAo= 40399 2009-09-30 14:25:43 -0700 2009-10-13 22:23:18 -0700 modify patch to include Eric's comments. CachedResourceHandle.h.patch text/plain 3220 yongjun.zhang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0ODk0MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjQgQEAKKzIwMDktMDktMzAgIFlvbmdqdW4gWmhhbmcgIDx5b25nanVuLnpoYW5n QG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjkxMDYKKyAgICAg ICAgW1F0XSBtYWtlIENhY2hlZFJlc291cmNlSGFuZGxlLmggY29tcGlsZSBpbiB3aW5zY3cgU3lt YmlhbiBjb21waWxlci4KKworICAgICAgICBBZGQgcGFyZXRoZW5zaXMgYXJvdW5kIChDYWNoZWRS ZXNvdXJjZUhhbmRsZUJhc2U6OipVbnNwZWNpZmllZEJvb2xUeXBlKSB0bworICAgICAgICBoZWxw IHdpbnNjdyBjb21waWxlciByZXNvbHZlIGNsYXNzIHF1YWxpZmllci4KKworICAgICAgICBEb24n dCBpbmxpbmUgY29uc3RydWN0b3IgQ2FjaGVkUmVzb3VyY2VIYW5kbGU8VD4oUiopIHRvIHN0b3Ag d2luc2N3CisgICAgICAgIGNvbXBpbGVyIGFnZ3Jlc3NpdmVseSByZXNvbHZlIGluaGVyaXRhbmNl IG9mIGNsYXNzIFIuICAKKyAgICAgICAgCisgICAgICAgIFRoZSB3aW5zY3cgY29tcGlsZXIgYnVn IGlzIHJlcG9ydGVkIGF0OgorICAgICAgICBodHRwczovL3hkYWJ1ZzAwMS5leHQubm9raWEuY29t L2J1Z3ppbGxhL3Nob3dfYnVnLmNnaT9pZD05ODEyLgorICAgICAgICAKKyAgICAgICAgVGhlIGNo YW5nZSBzaG91bGQgYmUgcmV2ZXJ0ZWQgd2hlbiB0aGUgYWJvdmUgYnVnIGlzIGZpeGVkIGluIHdp bnNjdyBjb21waWxlci4KKworICAgICAgICAqIGxvYWRlci9DYWNoZWRSZXNvdXJjZUhhbmRsZS5o OgorICAgICAgICAoV2ViQ29yZTo6OjpDYWNoZWRSZXNvdXJjZUhhbmRsZSk6CisKIDIwMDktMDkt MzAgIEplcmVteSBPcmxvdyAgPGpvcmxvd0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgQnVpbGQg Zml4IGZvciBRVC4gIERpZG4ndCBrbm93IFdlYkNvcmUucHJvIGV4aXN0ZWQuCkluZGV4OiBXZWJD b3JlL2xvYWRlci9DYWNoZWRSZXNvdXJjZUhhbmRsZS5oCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUv bG9hZGVyL0NhY2hlZFJlc291cmNlSGFuZGxlLmgJKHJldmlzaW9uIDQ4OTQzKQorKysgV2ViQ29y ZS9sb2FkZXIvQ2FjaGVkUmVzb3VyY2VIYW5kbGUuaAkod29ya2luZyBjb3B5KQpAQCAtMzgsNyAr MzgsOCBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgIGJvb2wgb3BlcmF0b3IhKCkgY29u c3QgeyByZXR1cm4gIW1fcmVzb3VyY2U7IH0KICAgICAgICAgCiAgICAgICAgIC8vIFRoaXMgY29u dmVyc2lvbiBvcGVyYXRvciBhbGxvd3MgaW1wbGljaXQgY29udmVyc2lvbiB0byBib29sIGJ1dCBu b3QgdG8gb3RoZXIgaW50ZWdlciB0eXBlcy4KLSAgICAgICAgdHlwZWRlZiBDYWNoZWRSZXNvdXJj ZSogQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNlOjoqVW5zcGVjaWZpZWRCb29sVHlwZTsKKyAgICAg ICAgLy8gUGFyZW50aGVzaXMgaXMgbmVlZGVkIGZvciB3aW5zY3cgY29tcGlsZXIgdG8gcmVzb2x2 ZSBjbGFzcyBxdWFsaWZpZXIgaW4gdGhpcyBjYXNlLgorICAgICAgICB0eXBlZGVmIENhY2hlZFJl c291cmNlKiAoQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNlOjoqVW5zcGVjaWZpZWRCb29sVHlwZSk7 CiAgICAgICAgIG9wZXJhdG9yIFVuc3BlY2lmaWVkQm9vbFR5cGUoKSBjb25zdCB7IHJldHVybiBt X3Jlc291cmNlID8gJkNhY2hlZFJlc291cmNlSGFuZGxlQmFzZTo6bV9yZXNvdXJjZSA6IDA7IH0K IAogICAgIHByb3RlY3RlZDoKQEAgLTU5LDcgKzYwLDcgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewog ICAgIHRlbXBsYXRlIDxjbGFzcyBSPiBjbGFzcyBDYWNoZWRSZXNvdXJjZUhhbmRsZSA6IHB1Ymxp YyBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UgewogICAgIHB1YmxpYzogCiAgICAgICAgIENhY2hl ZFJlc291cmNlSGFuZGxlKCkgeyB9Ci0gICAgICAgIENhY2hlZFJlc291cmNlSGFuZGxlKFIqIHJl cykgOiBDYWNoZWRSZXNvdXJjZUhhbmRsZUJhc2UocmVzKSB7IH0KKyAgICAgICAgQ2FjaGVkUmVz b3VyY2VIYW5kbGUoUiogcmVzKTsKICAgICAgICAgQ2FjaGVkUmVzb3VyY2VIYW5kbGUoY29uc3Qg Q2FjaGVkUmVzb3VyY2VIYW5kbGU8Uj4mIG8pIDogQ2FjaGVkUmVzb3VyY2VIYW5kbGVCYXNlKG8p IHsgfQogCiAgICAgICAgIFIqIGdldCgpIGNvbnN0IHsgcmV0dXJuIHJlaW50ZXJwcmV0X2Nhc3Q8 Uio+KENhY2hlZFJlc291cmNlSGFuZGxlQmFzZTo6Z2V0KCkpOyB9CkBAIC03MCw2ICs3MSwxNyBA QCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgIGJvb2wgb3BlcmF0b3I9PShjb25zdCBDYWNo ZWRSZXNvdXJjZUhhbmRsZUJhc2UmIG8pIGNvbnN0IHsgcmV0dXJuIGdldCgpID09IG8uZ2V0KCk7 IH0KICAgICAgICAgYm9vbCBvcGVyYXRvciE9KGNvbnN0IENhY2hlZFJlc291cmNlSGFuZGxlQmFz ZSYgbykgY29uc3QgeyByZXR1cm4gZ2V0KCkgIT0gby5nZXQoKTsgfQogICAgIH07CisJCisgICAg Ly8gRG9uJ3QgaW5saW5lIGZvciB3aW5zY3cgY29tcGlsZXIgdG8gcHJldmVudCB0aGUgY29tcGls ZXIgYWdyZXNzaXZlbHkgcmVzb2x2aW5nIAorICAgIC8vIHRoZSBiYXNlIGNsYXNzIG9mIFIqIHdo ZW4gQ2FjaGVkUmVzb3VyY2VIYW5kbGVyPFQ+KFIqKSBpcyBpbmxpbmVkLiAgVGhlIGJ1ZyBpcwor ICAgIC8vIHJlcG9ydGVkIGF0OiBodHRwczovL3hkYWJ1ZzAwMS5leHQubm9raWEuY29tL2J1Z3pp bGxhL3Nob3dfYnVnLmNnaT9pZD05ODEyLgorICAgIHRlbXBsYXRlIDxjbGFzcyBSPgorI2lmICFD T01QSUxFUihXSU5TQ1cpCisgICAgaW5saW5lCisjZW5kaWYKKyAgICBDYWNoZWRSZXNvdXJjZUhh bmRsZTxSPjo6Q2FjaGVkUmVzb3VyY2VIYW5kbGUoUiogcmVzKSA6IENhY2hlZFJlc291cmNlSGFu ZGxlQmFzZShyZXMpIAorICAgIHsKKyAgICB9CiAgICAgCiAgICAgdGVtcGxhdGUgPGNsYXNzIFIs IGNsYXNzIFJSPiBib29sIG9wZXJhdG9yPT0oY29uc3QgQ2FjaGVkUmVzb3VyY2VIYW5kbGU8Uj4m IGgsIGNvbnN0IFJSKiByZXMpIAogICAgIHsgCg==