Bug 146002 - FTL boolify() UntypedUse is wrong in the masquerades-as-undefined case
Summary: FTL boolify() UntypedUse is wrong in the masquerades-as-undefined case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-15 20:27 PDT by Filip Pizlo
Modified: 2015-06-16 12:18 PDT (History)
13 users (show)

See Also:


Attachments
the patch (3.00 KB, patch)
2015-06-15 20:29 PDT, Filip Pizlo
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-06-15 20:27:05 PDT
It makes non-masquerading objects boolify to False instead of True.
Comment 1 Filip Pizlo 2015-06-15 20:29:20 PDT
Created attachment 254927 [details]
the patch
Comment 2 Darin Adler 2015-06-16 10:41:11 PDT
Comment on attachment 254927 [details]
the patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        * ftl/FTLLowerDFGToLLVM.cpp: Put this in an anonymous namespace. We should have done that all along. It makes it easier to add debug code.

Should we be using anonymous namespaces throughout WebKit instead of “static” to make things local to a file? A while back this made debugging harder, which I think is why we didn’t do that at the time.
Comment 3 Mark Lam 2015-06-16 11:32:06 PDT
<rdar://problem/21393673>
Comment 4 Filip Pizlo 2015-06-16 12:07:14 PDT
(In reply to comment #2)
> Comment on attachment 254927 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254927&action=review
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        * ftl/FTLLowerDFGToLLVM.cpp: Put this in an anonymous namespace. We should have done that all along. It makes it easier to add debug code.
> 
> Should we be using anonymous namespaces throughout WebKit instead of
> “static” to make things local to a file? A while back this made debugging
> harder, which I think is why we didn’t do that at the time.

I didn't know about the debugging issue. Compiler debugging usually involves glorified printf()'s.

My preference is to use anonymous namespaces instead of static, because it allows us to create one block in a module that contains all of the module-local stuff - functions, classes, variables, enums, etc. It's very liberating - you can use short names.

We've been steadily converting the DFG/FTL to use anonymous namespaces for the module-local classes. Each compiler phase usually involves at least one module-local class. That's really all I'm doing here.
Comment 5 Filip Pizlo 2015-06-16 12:18:35 PDT
Landed in http://trac.webkit.org/changeset/185600