[cfe-commits] [patch] Warn in string literal + integral

Nico Weber thakis at chromium.org
Mon Mar 5 07:56:47 PST 2012


Thanks, Jeff!

Did you build all of Firefox, or just SpiderMonkey?

There was another similar false positive in clang itself that looked
like |bool HasSpace = ...; use(" /*"+HasSpace)|. Maybe the warning
shouldn't fire if the argument is a bool? (But that wouldn't have
helped in your case.)

Nico

On Fri, Mar 2, 2012 at 8:12 PM, Jeff Walden <jwalden+clang at mit.edu> wrote:
> For what it's worth, this warning triggers twice when building the SpiderMonkey JavaScript compiler, in two bits of near-adjacent code that look like this:
>
>>     if (attrs) {
>>         int first = 1;
>>         fputs("(", fp);
>> #define DUMP_ATTR(name, display) if (attrs & JSPROP_##name) fputs(" " #display + first, fp), first = 0
>>         DUMP_ATTR(ENUMERATE, enumerate);
>>         DUMP_ATTR(READONLY, readonly);
>>         DUMP_ATTR(PERMANENT, permanent);
>>         DUMP_ATTR(GETTER, getter);
>>         DUMP_ATTR(SETTER, setter);
>>         DUMP_ATTR(SHARED, shared);
>> #undef  DUMP_ATTR
>>         fputs(") ", fp);
>>     }
>
> (the idea being to print "(enumerate)", "(enumerate readonly)", or whatever depending on attrs; the other instance does likewise for a flags field)
>
> I'm not inclined to defend this technique, although it's not too bad as tricks go.  I think the new warning is a good thing, and I'm perfectly happy to switch those two instances to use &()[] instead.  But it seems worth mentioning here as another data point.
>
> Jeff
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list