[Webkit-unassigned] [Bug 207119] Enable offlineasm debug annotations for GCC

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=207119

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #395174|review?                     |review+
              Flags|                            |

--- Comment #38 from Darin Adler <darin at apple.com> ---
Comment on attachment 395174
  --> https://bugs.webkit.org/attachment.cgi?id=395174
Patch

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

Looks fine to me; please consider further refinement to the Compiler.h macro

> Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:549
> +#endif /* WTF_COMPILER_GCC */

Code style nitpick:

This isn’t following the coding style of the rest of this file and WebKit. We aren’t commenting every #endif, particularly when a block of code is this short. And we use "//" comments when we do. And we don’t comment with the underlying macro name instead of the actual expression on the #if statement.

Better to not have an #if here at all (see comment in Compiler.h).

> Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:557
> +#endif /* WTF_COMPILER_GCC */

Ditto.

> Source/WTF/wtf/Compiler.h:388
> +#if !defined(MARKER_SYMBOL) && COMPILER(GCC)
> +#define MARKER_SYMBOL(name) \
> +    __attribute__((__no_reorder__)) void name(void) { __asm__(""); }
> +#endif

It’s more consistent with the Compiler.h compiler-independence strategy to define MARKER_SYMBOL everywhere and have it be a no-op on compilers that don’t need it. Might also need a more unambiguous name. The phrase "marker symbol" doesn’t point straight at a compiler capability to me. Seems a bit obscure to claim such a basic name. I might have chosen a longer name like DEBUGGER_ANNOTATION_MARKER(name). If we want to emphasize that it’s GCC-specific can even include GCC in the name.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200401/cd153726/attachment.htm>