Bug 160195 – check-for-exit-time-destructors should be usable outside Xcode

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Created attachment 284583 [details]
WIP Patch

It would be nice if check-for-exit-time-destructors was generally usable outside an Xcode build phase script.

[David Kilzer (:ddkilzer)]

Created attachment 284597 [details]
Patch v1

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> Created attachment 284597 [details]
> Patch v1

Same as "WIP Patch", but with ChangeLog.

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> (In reply to comment #1)
> > Created attachment 284597 [details]
> > Patch v1
> 
> Same as "WIP Patch", but with ChangeLog.

https://bugs.webkit.org/attachment.cgi?oldid=284583&action=interdiff&newid=284597&headers=1

[Darin Adler]

Comment on attachment 284597 [details]
Patch v1

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

> Tools/ChangeLog:13
> +        * Scripts/check-for-exit-time-destructors: Update to parse
> +        -h|--help switch, or to take one argument to a binary to check
> +        for exit time destructors on the command-line.  The clang
> +        compiler will find these at compile-time with the
> +        -Wexit-time-destructors switch, but this script will check for
> +        them after-the-fact.

Given that -Wexit-time-destructors now exists, do we even need this script at all any more?

> Tools/Scripts/check-for-exit-time-destructors:65
> +    # Running from Xcode build phase script with no arguments.

Maybe this behavior should be triggered by an explicit command line argument instead of by "no arguments".

Also, when you try this and none of the environment variables are set, what ends up happening?

[WebKit Commit Bot]

Comment on attachment 284597 [details]
Patch v1

Clearing flags on attachment: 284597

Committed r203742: <http://trac.webkit.org/changeset/203742>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

Comment on attachment 284597 [details]
Patch v1

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

>> Tools/ChangeLog:13
>> +        them after-the-fact.
> 
> Given that -Wexit-time-destructors now exists, do we even need this script at all any more?

The script is not referenced in any Xcode project in WebKit anymore, but the script was very useful in checking for exit-time destructors in a compiled binary (see <rdar://problem/27528158>).

Not sure if the WebKit project is the right place to host such a script, though.  (The check-for-global-initializers script also has a clang warning now.)

Thoughts?

>> Tools/Scripts/check-for-exit-time-destructors:65
>> +    # Running from Xcode build phase script with no arguments.
> 
> Maybe this behavior should be triggered by an explicit command line argument instead of by "no arguments".
> 
> Also, when you try this and none of the environment variables are set, what ends up happening?

The script prints a bunch of error messages, then exits without doing anything useful.

A command-line switch would make it not spew error messages, which would be a better user experience.

[Darin Adler]

I personally think that this script, which is one that I wrote to track progress of the project to eliminate all exit-time destructors from WebKit specifically to help with an iOS-specific issue (see bug 22061), has outlived its usefulness and should be deleted.

[Darin Adler]

Oh, and I now see how this script was useful internally at Apple, Dave. I agree, it should be moved somewhere else since it’s not important for building WebKit any more.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> I personally think that this script, which is one that I wrote to track
> progress of the project to eliminate all exit-time destructors from WebKit
> specifically to help with an iOS-specific issue (see bug 22061), has
> outlived its usefulness and should be deleted.

Filed Bug 160302 to track removing the scripts (and using -Wexit-time-destructors, -Wglobal-constructors consistently).  In hindsight, maybe I should have separated these two steps, though.

(In reply to comment #9)
> Oh, and I now see how this script was useful internally at Apple, Dave. I
> agree, it should be moved somewhere else since it’s not important for
> building WebKit any more.

I have an idea about next steps here.  More radars forthcoming.  :)