Bug 150773 – Rename op_put_getter_setter to op_put_getter_setter_by_id

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Rename op_put_getter_setter to op_put_getter_setter_by_id

[Yusuke Suzuki]

Created attachment 264514 [details]
Patch

[Mark Lam]

Comment on attachment 264514 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264514&action=review

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        Renaming op_put_getter_setter to op_put_getter_setter_by_id makes this op name consistent to

"consistent to" ==> "consistent with"

[Yusuke Suzuki]

Comment on attachment 264514 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264514&action=review

Thank you for your review :D

>> Source/JavaScriptCore/ChangeLog:8
>> +        Renaming op_put_getter_setter to op_put_getter_setter_by_id makes this op name consistent to
> 
> "consistent to" ==> "consistent with"

Thanks! Fixed.

[Yusuke Suzuki]

Committed r191858: <http://trac.webkit.org/changeset/191858>

[Alexey Proskuryakov]

This broke 32-bit build:

https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/13856/steps/compile-webkit/logs/stdio

Undefined symbols for architecture i386:
  "_llint_slow_path_put_getter_setter", referenced from:
      _llint_entry in LowLevelInterpreter.o
     (maybe you meant: _llint_slow_path_put_getter_setter_by_id)

[WebKit Commit Bot]

Re-opened since this is blocked by bug 150780

[Alexey Proskuryakov]

Rolling out. Even if this just needs a can rebuild, the right fix is to fix the dependencies.

[Yusuke Suzuki]

Hm,

```
offlineasm: Parsing /Volumes/Data/slave/mavericks-32bitJSC-debug/build/Source/JavaScriptCore/llint/LowLevelInterpreter.asm and creating offset extractor /Volumes/Data/slave/mavericks-32bitJSC-debug/build/WebKitBuild/Debug/LLIntOffsets/LLIntDesiredOffsets.h.
offlineasm: Nothing changed.
```

```
offlineasm: Parsing JavaScriptCore/llint/LowLevelInterpreter.asm and /Volumes/Data/slave/mavericks-32bitJSC-debug/build/WebKitBuild/Debug/JSCLLIntOffsetsExtractor and creating assembly file LLIntAssembly.h.
offlineasm: Nothing changed.
```

LowLevelInterpreter.asm is updated in the patch, but the output seems not updated.

[Yusuke Suzuki]

Alexey, I think the patch itself is correct.
It changes all the existing getter_setter signature to getter_setter_by_id.

Seeing the log,
https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/13853/steps/compile-webkit/logs/stdio
LowLevelInterpreter.o is not recompiled regardless of change of LLIntAssembly.h.
Do you know how XCode handles header change?
I'm not sure that Xcode correctly handles changes of headers.

One possible workaround is, adding white space line to LowLevelInterpreter.cpp.

[Yusuke Suzuki]

(In reply to comment #9)
> One possible workaround is, adding white space line to
> LowLevelInterpreter.cpp.

But if do so, just issuing a clean build is preferable I think.

[Yusuke Suzuki]

After investigating a little bit, I think adding LLIntAssembly.h into Xcode project file may solve this issue.

[Yusuke Suzuki]

Discussed with Alexey on IRC.
We will include LLIntAssembly.h into Xcode project file, land the patch and watch on buildbot!

[Yusuke Suzuki]

Committed r191897: <http://trac.webkit.org/changeset/191897>

[Yusuke Suzuki]

Yay!
https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/13879