Re: [webkit-dev] Style proposal for branch style for macros
On Apr 20, 2014, at 1:19 PM, Mark Rowe mr...@apple.com wrote:
Won’t compile:
if (!MAYBE_DEFINED) {
…
}
if (SOMETHING) {
void doSomething() { }
}
if (SOMETHING) {
#include “Something.h
}
Will result in unintended behavior:
if (!FOO) {
…
#define BAR
…
}
We clearly can’t use if() to replace #if in general. Let’s assume that we
reject a universal proposal of that sort.
However, it seems like the above examples will generally not apply in the case
of the switch for assertions being enabled or disabled. Or at least I would not
expect it - do you know of such cases in the code?
One worry about Phil’s proposal is that we’d be making asserts enabled/disabled
different from handling of other conditional preprocessor macros. One possible
way to avoid this is to make asserts enabled/disabled a compile-time constant
held in a const global variable instead. That would enforce this style and make
it natural.
I am not sure offhand if this is actually viable, but I see no deep reason that
assertion control has to be just like the conditional macros we use to avoid
submitting code to the compiler that won’t actually compile.
Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Style proposal for branch style for macros
On Apr 20, 2014, at 1:32 PM, Mark Rowe mr...@apple.com wrote:
It’s also not possible to determine from context alone whether #if or if
should be used. When #if is used to remove members or functions you’re then
forced to use #if, rather than if, around all uses of those members or
functions since the alternative won’t compile.
An example lifted from Vector.h:
#if !ASSERT_DISABLED
templatetypename T struct ValueCheckVectorT {
typedef VectorT TraitType;
static void checkConsistency(const VectorT v)
{
v.checkConsistency();
}
};
#endif
templatetypename T, size_t inlineCapacity, typename OverflowHandler
inline void VectorT, inlineCapacity, OverflowHandler::checkConsistency()
{
if (!ASSERT_DISABLED) {
for (size_t i = 0; i size(); ++i)
ValueCheckT::checkConsistency(at(i));
}
}
This won’t compile with assertions disabled because
ValueCheck::checkConsistency doesn’t exist.
Oops, I should have read this email before sending my previous reply. It does
seem like using if() exclusively to control assertions won’t work.
On Apr 20, 2014, at 2:02 PM, Filip Pizlo fpi...@apple.com wrote:
The #if means that the code being guarded isn't indented. I believe that
indenting control flow is a good idea.
This is the main reason I thought the proposal was promising. Note though that
we could change our style guide to call for some indentation of #if’s if we
wanted to. It is valid C/C++ to do so. The main is that not all text editors
have good support for this.
Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Adding flag to optionally run check-webkit-style as part of prepare-Changelog and svn-create-patch
Thanks to Brendan, Alexey, Zoltan, and Daniel for the feedback on this, which I've addressed. Any more comments? Thanks. r? On Apr 4, 2014, at 12:14 AM, James Craig jcr...@apple.com wrote: I sometimes forget to run the check-webkit-style script before uploading patches, so I put a patch up for review that adds a --style and --no-style flags to prepare-Changelog and svn-create-patch. Alexey suggested I make it the default, which I've done for prepare-Changelog, but not in svn-create-patch due to cross-references that cause a loop when it't the default. (I just added the --style flag to my local bash alias.) Any concerns or suggestions? Thanks. http://webkit.org/b/131115 ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
