[prev in list] [next in list] [prev in thread] [next in thread]
List: webkit-dev
Subject: Re: [webkit-dev] renaming ASSERT macro
From: Paul Pedriana <ppedriana () gmail ! com>
Date: 2008-07-01 17:39:25
Message-ID: 486A6BCD.7030807 () gmail ! com
[Download RAW message or body]
[Attachment #2 (text/html)]
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
As Darin suggests, there seem to be two questions:<br>
<br>
1 whether it's useful to have a different name for ASSERT.<br>
2 whether it's worthwhile to make a large code change to reflect
the different name.<br>
<br>
With regard to question 1, IMO in an ideal world ASSERT would have a
unique prefix (e.g. WTF_) in order to guarantee collision avoidance as
WebKit is ported to different platforms and different usage. I can see
a number of other places in WebKit where this practice is already done
(e.g. WEBKIT_VIDEO_SINK, KJS_FAST_CALL, etc.). This is of course
similar to the reason we use C++ namespaces to help avoid code
collisions. <br>
<br>
But (2) it may or may not be trivial to do a replace of ASSERT
project-wide to deal with a name change. Some might argue for a gradual
approach, but I don't know enough about WebKit development conventions
to comment on that. <br>
<br>
Even if people don't want to revise the name of ASSERT, I do think that
it would be a good idea to consider prefixes for future macros. In
addition to avoiding collisions, they help clarify what subsystem they
are associated with.<br>
<br>
On a related note, I would like to propose (possibly in a separate
email) that the CRASH macro in Assertions.h that ASSERT uses be
augmented to the following for improved debugging and portability
across most platforms:<br>
<br>
#if !defined(WTF_DEBUG_BREAK)<br>
#if defined(__GNUC__) && (defined(_X86) || defined(__i386)
> > defined(__i386__) || defined(i386))<br>
#ifndef __L4ENV__<br>
#define \
WTF_DEBUG_BREAK() __asm__ __volatile__ ("int3\n\tnop")<br>
#else<br>
#define \
WTF_DEBUG_BREAK() __asm__ __volatile__ ("int3; jmp 1f; 1:")<br>
#endif<br>
<br>
#elif defined(__GNUC__) && (defined(__powerpc__) ||
defined(__ppc__))<br>
#define WTF_DEBUG_BREAK() __asm__ \
__volatile__ ("tw 31,1,1")<br> <br>
#elif defined(_MSC_VER) && (_MSC_VER >= 1300)<br>
#define WTF_DEBUG_BREAK() \
__debugbreak()<br> <br>
#else<br>
#define WTF_DEBUG_BREAK() (*(int \
*)(uintptr_t)0xffffffff = 0)<br> #endif<br>
<br>
Paul<br>
<br>
<br>
<br>
<blockquote cite="mid:B9455365-D825-4956-B62D-3359E62DA2F1@apple.com"
type="cite">
<pre wrap="">On Jul 1, 2008, at 1:45 AM, Jörg Bornemann wrote:
</pre>
<blockquote type="cite">
<pre wrap="">This solution is easy to do, leads to the smallest source diff but
is a very dirty hack, which will lead to problems on WinCE, because
we will include windows.h in public headers.
</pre>
</blockquote>
<pre wrap=""><!---->
Adding <windows.h> to Assertions.h will not cause it to be included in
public headers. Assertions.h is not designed to be used in public
headers; it's for internal use inside the WebKit project.
And "is a very dirty hack" is not a technical argument.
</pre>
<blockquote type="cite">
<pre wrap="">So what's your argument against the clean solution (renaming)?
</pre>
</blockquote>
<pre wrap=""><!---->
For one thing, I don't like the other names you suggested.
We've used ASSERT for the lifetime of the WebKit project, many years.
It appears in thousands of lines of code. I don't want to make a
global change in that name because of a WinCE-specific issue unless
there's no other solution.
There are numerous examples where internal WebKit things conflict with
platform headers or macros and we've been able to resolve them without
renaming the WebKit things. To give one small example, we use "id" in
WebKit even though that's a special reserved word on Mac OS X in
Objective-C. We also manage to use min and max despite the definitions
in <windef.h>. We work around these bugs in platform header design in
ways that don't require us to change the bulk of the WebKit code.
If your argument was that ASSERT is not a good name and you were
making a case for a better name on the basis of clarity and coding
style, I'd be happy to consider and debate that.
Lets do the local solution in Assertions.h. Then we will have code
that compiles and works on WinCE, and then we can debate the concrete
merits of other solutions at our leisure.
-- Darin
_______________________________________________
webkit-dev mailing list
<a class="moz-txt-link-abbreviated" \
href="mailto:webkit-dev@lists.webkit.org">webkit-dev@lists.webkit.org</a> <a \
class="moz-txt-link-freetext" \
href="http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev">http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev</a>
</pre>
</blockquote>
<br>
</body>
</html>
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic