gmane.os.opendarwin.webkit.devel

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Maciej Stachowiak — 2013-06-19T08:28:37

On Jun 18, 2013, at 10:16 PM, Ryosuke Niwa <rniwa< at >webkit.org> wrote:


I don't think possible lazy creation is a good reason to decorate the name. Functions should be named for what they do, not their presumed efficiency.

I am also not sure we need a style guideline about putting things into variables. If it makes a difference in a hot code path, then sure, but most of the time, the more concise code is better.

 - Maciej

_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Gavin Barraclough — 2013-06-19T07:13:25

I find ‘requireStyleResolver()’ a little confusing.  My first expectation is often that a method is an imperative command on the receiver, so I first read 'requireStyleResolver()’ as mandating that the document now requires a StyleResolver, rather than referring to the need of the caller.

In a previous codebase I’ve worked on we used ‘acquire’ as a term of art for lazy creation (as a synonym for get, with an inherent connotation that doing so may nontrivial).

    StyleResolver& acquireStyleResolver();

cheers,
G.

On Jun 18, 2013, at 7:11 PM, Darin Adler <darin< at >apple.com> wrote:


_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme for fooIfExists/ensureFooclearStyleResolver

Gavin Barraclough — 2013-06-19T06:53:54


OOI Is the lazily-create function not inlined? – I wouldn’t expect to pay two function calls here, since the method is in the header & looks simple enough:

    StyleResolver* ensureStyleResolver()
    { 
        if (!m_styleResolver)
            createStyleResolver();
        return m_styleResolver.get();
    }

I imaging we may perform the branch twice, since it may not be apparent to the compiler that m_styleResolver is non-null after the first call – though I guess this is necessary since theoretically x() could clearStyleResolver.


_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: There is a linking error when build webkit onwindows 7

ビリゴスチン — 2013-06-19T06:28:29

It works!

The Conclusion is :
We must copy all lib files from WebKitLibraries/win/lib to WebKitLibraries/win/lib32 before first build.
It is better to set it default.

Thank you.



Best Regards.
SiqinBilige

Re: For your consideration: Naming scheme forfooIfExists/ensureFoo

Ryosuke Niwa — 2013-06-19T05:16:41


True. But it's important to differentiate a simple inline accessor and a
lazily-create function because it's very easy to write code like:

if (styleResolver().x())
    styleResolver().y();

and incur two function calls when we could have done

StyleResolver& resolver = styleResolver();
if (resolver.x())
    resolver.y();

instead.

On the other hand, I've started to think that maybe we can simply forbid
the former style altogether in the style guide so that we'll never have to
think about whether a given function is inline or not.

- R. Niwa
_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Maciej Stachowiak — 2013-06-19T04:21:34

If the semantic is essentially that of a getter that just happens to lazily create what it gets on demand, then I don't think "require" or "required" is needed. It can just be named as a getter. If the side effect is very important and especially if clients ever call the function only for its side effect, then a verb phrase would be better. I am not sure which applies in this case.

 - Maciej



On Jun 18, 2013, at 7:20 PM, Simon Fraser <simon.fraser< at >apple.com> wrote:

_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: There is a linking error when build webkit on windows 7

Brent Fulgham — 2013-06-19T03:41:31

Hi Siqinbilige:

On Jun 18, 2013, at 8:31 PM, ビリゴ スチン <siqinbilige< at >gmail.com> wrote:


There is a script that runs at the start of the WTFGenerated build (build-generated-files.sh) that checks for the presence of the lib32/libicuuc.lib file.  It creates a file in your build folder ("${CONFIGURATIONBUILDDIR}/include/private/ICUVersion.h"), which governs whether it is looking for versioned (old-stye) or new-style.

Even though you corrected the library paths, this ICUVersion.h file is probably messing up your build.

Try deleting it (or just changing the "#define U_DISABLE_RENAMING 0" to "#define U_DISABLE_RENAMING 1", and then do a full rebuild.

-Brent

Re: There is a linking error when build webkit onwindows 7

ビリゴスチン — 2013-06-19T03:31:39

Hi brent,
Thank you for your response.


Yes, There is a WebKitLibraries/win/lib32 folder and also have WebKitLibraries/win/lib folder.
first, there is one file WebKitSystemInterface.lib.
I copied the all of the files which including libicuuc.lib from WebKitLibraries/win/lib to WebKitLibraries/win/lib32.
was passed.
But failed when linking.

SiqinBilige.

Re: There is a linking error when build webkit on windows 7

Brent Fulgham — 2013-06-19T03:17:07

Hi SiqinBilige:

On Jun 18, 2013, at 8:03 PM, siqinbilige <siqinbilige< at >gmail.com> wrote:


All of that should be fine...


First clue above...


... second clue.

When we build on Windows, we do not expect to see the version suffix on the function symbols (i.e., the "_u_charDirection_46" should be "_u_charDirection").

This means that the build system was unable to find the "libicuuc.lib" library in its path, and instead is building using the older symbols.

This usually happens when the WebKitSupportLibraries are not installed, or are installed in the wrong place.

Do you have a folder called "WebKitLibraries/win/lib32"?

-Brent

_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

There is a linking error when build webkit on windows 7

siqinbilige — 2013-06-19T03:03:35

Hi,

I wan to build webkit on Windows.
My environment is :
  Windows 7 Ultimate (Service Pack1) 64bit (japanese)
  Intel(R) Core(TM) i7 CPU / 12.0GB Memory
  VC++ 2010 Express
  Microsoft Windows SDK for windows 7(7.1)
  Safari 5.1.7

When I run build-webkit, first failed with libs.
  1>LINK : fatal error LNK1181: 入力ファイル 'libicuuc.lib' を開けません。
  1>プロジェクト "V:\WebKit\Source\WTF\WTF.vcxproj\WTF.vcxproj" (build ターゲット)
のビルドが終了しました -- 失敗。
I copy files of  WebKitLibraries\win\lib to WebKitLibraries\win\lib32
and it is passed.

But, failed with link.
            ライブラリ V:\WebKit\WebKitBuild\Release\lib32\WTF.lib とオブジェクト
V:\WebKit\WebKitBuild\Release\lib32\WTF.exp を作成中
     1>Base64.obj : error LNK2019: 未解決の外部シンボル _u_charDirection_46 が関数 "enum
WTF::Unicode::Direction __cdecl WTF::Unicode::direction(int)"
(?direction< at >Unicode< at >WTF< at >< at >YA?AW4Direction< at >12< at >H< at >Z) で参照されました。
     1>StringImpl.obj : error LNK2001: 外部シンボル "_u_charDirection_46" は未解決です。
     1>WTFString.obj : error LNK2001: 外部シンボル "_u_charDirection_46" は未解決です。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_foldCase_46 が関数 "bool
__cdecl WTF::equalIgnoringCase(unsigned char const *,unsigned char const
*,unsigned int)" (?equalIgnoringCase< at >WTF< at >< at >YA_NPBE0I< at >Z) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_strFoldCase_46 が関数
"public: class WTF::PassRefPtr<class WTF::StringImpl> __thiscall
WTF::StringImpl::foldCase(void)" (?foldCase< at >StringImpl< at >WTF< at >
< at >QAE?AV?$PassRefPtr< at >VStringImpl< at >WTF< at >< at >< at >2< at >XZ) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_strToLower_46 が関数
"public: class WTF::PassRefPtr<class WTF::StringImpl> __thiscall
WTF::StringImpl::lower(void)" (?lower< at >StringImpl< at >WTF< at >
< at >QAE?AV?$PassRefPtr< at >VStringImpl< at >WTF< at >< at >< at >2< at >XZ) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_tolower_46 が関数
"public: class WTF::PassRefPtr<class WTF::StringImpl> __thiscall
WTF::StringImpl::lower(void)" (?lower< at >StringImpl< at >WTF< at >
< at >QAE?AV?$PassRefPtr< at >VStringImpl< at >WTF< at >< at >< at >2< at >XZ) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_toupper_46 が関数
"public: class WTF::PassRefPtr<class WTF::StringImpl> __thiscall
WTF::StringImpl::upper(void)" (?upper< at >StringImpl< at >WTF< at >
< at >QAE?AV?$PassRefPtr< at >VStringImpl< at >WTF< at >< at >< at >2< at >XZ) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_strToUpper_46 が関数
"public: class WTF::PassRefPtr<class WTF::StringImpl> __thiscall
WTF::StringImpl::upper(void)" (?upper< at >StringImpl< at >WTF< at >
< at >QAE?AV?$PassRefPtr< at >VStringImpl< at >WTF< at >< at >< at >2< at >XZ) で参照されました。
     1>StringImpl.obj : error LNK2019: 未解決の外部シンボル _u_memcasecmp_46 が関数
"bool __cdecl WTF::equalIgnoringCaseNonNull(class WTF::StringImpl const
*,class WTF::StringImpl const *)" (?equalIgnoringCaseNonNull< at >WTF
< at >< at >YA_NPBVStringImpl< at >1< at >0< at >Z) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_setAttribute_46
が関数 "private: void __thiscall WTF::Collator::createCollator(void)const "
(?createCollator< at >Collator< at >WTF< at >< at >ABEXXZ) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_open_46 が関数
"private: void __thiscall WTF::Collator::createCollator(void)const "
(?createCollator< at >Collator< at >WTF< at >< at >ABEXXZ) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_getAttribute_46
が関数 "private: void __thiscall WTF::Collator::createCollator(void)const "
(?createCollator< at >Collator< at >WTF< at >< at >ABEXXZ) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_getLocaleByType_46
が関数 "private: void __thiscall WTF::Collator::createCollator(void)const "
(?createCollator< at >Collator< at >WTF< at >< at >ABEXXZ) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_close_46 が関数
"private: void __thiscall WTF::Collator::releaseCollator(void)"
(?releaseCollator< at >Collator< at >WTF< at >< at >AAEXXZ) で参照されました。
     1>CollatorICU.obj : error LNK2019: 未解決の外部シンボル _ucol_strcoll_46 が関数
"public: enum WTF::Collator::Result __thiscall
WTF::Collator::collate(wchar_t const *,unsigned int,wchar_t const
*,unsigned int)const " (?collate< at >Collator< at >WTF< at >< at >QBE?AW4Result< at >12< at >PB_WI0I< at >Z)
で参照されました。
     1>V:\WebKit\WebKitBuild\Release\bin32\WTF.dll : fatal error LNK1120:
外部参照 14 が未解決です。
     1>プロジェクト "V:\WebKit\Source\WTF\WTF.vcxproj\WTF.vcxproj" (build ターゲット)
のビルドが終了しました -- 失敗。

What is wrong with me ? is it a problem of 32/64bit ?

Thanks and Regards,
SiqinBilige
_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Simon Fraser — 2013-06-19T02:20:19


Since the fact that it returns a reference implies that it must create something if necessary, the “required” part of the name seems redundant. Why not just
StyleResolver& styleResolver()

requireStyleResolver() sounds like it would return a bool.

Simon

_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Darin Adler — 2013-06-19T02:11:37


I’m warming to this idea. Maybe we can use “require” as a term of art, analogous to the way we use “create”, to mean “create if not already created”.

Re: For your consideration: Naming scheme for fooIfExists/ensureFoo

Darin Adler — 2013-06-19T02:09:37


That is true.


Functions with return values don’t read well when they are verb phrases instead of noun phrases. It doesn’t make grammatical sense to perform an operation on a verb. Other ideas, maybe not so great:

    StyleResolver& mandatoryStyleResolver();
    StyleResolver& neededStyleResolver();

Not loving any of these yet, I guess. They seem to have the same problem as requiredStyleResolver.

Re: For your consideration: Naming scheme forfooIfExists/ensureFoo

Ryosuke Niwa — 2013-06-19T02:05:25


requiredStyleResolver sounds as if it's a special (required) type of a
style resolver as opposed to the caller requiring it.  Why don't we call it
requireStyleResolver() instead?

Note also that I think the requiredStyleResolver function should return a


Sounds like a great idea.

On Tue, Jun 18, 2013 at 7:03 PM, Emil A Eklund <eae< at >chromium.org> wrote:



I think it's important to communicate the runtime cost of ensuring the
existence of the object.

- R. Niwa
_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: For your consideration: Naming scheme forfooIfExists/ensureFoo

Emil A Eklund — 2013-06-19T02:03:45

+1, much clearer and the pointer vs reference makes it even more so.
Perhaps enough so that the required prefix could be dropped:

StyleResolver* optionalStyleResolver();
StyleResolver& styleResolver();

For your consideration: Naming scheme forfooIfExists/ensureFoo

Darin Adler — 2013-06-19T01:38:30

Lets bike shed!

For some time, functions with names like fooIfExists and ensureFoo have been bothering me. I find both names kind of opaque and unpleasant. Here’s an example:

    StyleResolver* styleResolverIfExists();
    StyleResolver* ensureStyleResolver()

What do you think of these names instead?

    StyleResolver* optionalStyleResolver();
    StyleResolver& requiredStyleResolver();

I like them better. Note also that I think the requiredStyleResolver function should return a reference so nobody is tempted to do null checks. It seems like the C++ community likes the name optional for this concept; isn’t there some kind of std::optional template?

Functionality of create_activation andtear_off_activation

Abhishek Bichhawat — 2013-06-18T19:38:31

Hi,

We are working on a research project involving webkit. We are currently using Webkit-r122160 and trying to understand the functionality of two of the opcodes (op_create_activation and op_tear_off_activation), which are a part of the classical interpreter. Although, the classical interpreter has been phased out, we would appreciate if you could provide an insight into the functionality of the two opcodes. We are interested in the low-level details of the opcodes as to how they insert an object in the heap/register/scopechain and delete it from there. 

Thanks and Regards,
Abhishek_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: Adding document.currentScript to WebCore

Maciej Stachowiak — 2013-06-17T21:34:11

Sounds good to me.

 - Maciej

On Jun 17, 2013, at 1:24 AM, Kangil Han <kangil.han< at >samsung.com> wrote:


_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: Any update on bug 114036

Benjamin Poulain — 2013-06-17T19:40:49

Hi Stéphane,

On Mon, Jun 17, 2013 at 6:56 AM, Stephane Cerveau <
scerveau< at >connected-labs.com> wrote:


Please keep the discussion on the bug instead of the mailing list.
Have you tried on the nightly? http://nightly.webkit.org

Benjamin
_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Any update on bug 114036

Stephane Cerveau — 2013-06-17T13:56:21

Dear all,

I just wanted to know if there is a chance to have an update on this bug:
https://bugs.webkit.org/show_bug.cgi?id=114036
I'm fully available to discuss about it.
This bug has been open in april 2013 and can be reproduced on Safari.
Best regards.

Stéphane

Adding document.currentScript to WebCore

Kangil Han — 2013-06-17T08:24:42

Hi webkit-dev!
I wanted to let you know that I plan to add document.currentScript attribute
support to WebKit.
 
At this moment, it's been supported in chrome and firefox.
You can find this attribute details in [1][2][3].
 
You can also find related bug in
https://bugs.webkit.org/show_bug.cgi?id=104221
I am looking forward to your comments.
 
Thanks!
Kangil Han
 
[1]
http://www.w3.org/html/wg/drafts/html/master/dom.html#dom-document-currentsc
ript
[2]
http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#dom-doc
ument-currentscript
[3] https://developer.mozilla.org/en-US/docs/Web/API/document.currentScript
 

_______________________________________________
webkit-dev mailing list
webkit-dev< at >lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev