Bug 56000 – Move the alignment related macros in Vector.h to the new Alignment.h.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Renata Hodovan]

Move the alignment related macros in Vector.h to new Alignment.h.

[Renata Hodovan]

Created attachment 85145 [details]
Proposed patch

[Darin Adler]

*** Bug 56182 has been marked as a duplicate of this bug. ***

[David Levin]

Comment on attachment 85145 [details]
Proposed patch

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

You're going to need to add forwarding headers to avoid breaking the mac build.

I wonder if a better name for the header would be AlignedBuffer.h since that appears to be the primary thing that it provides so it is fine as is but consider renaming the header.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:3126
> +		A19B86F7132670040037F979 /* Alignment.h in Headers */ = {isa = PBXBuildFile; fileRef = A19B86F6132670040037F979 /* Alignment.h */; };

The changes in this file shouldn't be done. This file should be reverted.

[Renata Hodovan]

(In reply to comment #3)
> (From update of attachment 85145 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85145&action=review
> 
> You're going to need to add forwarding headers to avoid breaking the mac build.
> 
> I wonder if a better name for the header would be AlignedBuffer.h since that appears to be the primary thing that it provides so it is fine as is but consider renaming the header.
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:3126
> > +		A19B86F7132670040037F979 /* Alignment.h in Headers */ = {isa = PBXBuildFile; fileRef = A19B86F6132670040037F979 /* Alignment.h */; };
> 
> The changes in this file shouldn't be done. This file should be reverted.

Thanks, but it's already resolved in #56182. Sorry for not closing this bug earlier.

[Gabor Loki]

> Thanks, but it's already resolved in #56182. Sorry for not closing this bug earlier.

Well, bug 56182 was a duplicate of this bug, and no patch is landed from there.

[Renata Hodovan]

(In reply to comment #5)
> > Thanks, but it's already resolved in #56182. Sorry for not closing this bug earlier.
> 
> Well, bug 56182 was a duplicate of this bug, and no patch is landed from there.

Ohh, I was sure it was landed. So, do you want to work on it or may I do it? :)

[Renata Hodovan]

Created attachment 88195 [details]
Proposed patch

[Renata Hodovan]

The bug isn't solved yet :)

[Renata Hodovan]

Created attachment 88196 [details]
Proposed patch

[Build Bot]

Attachment 88196 [details] did not build on win:
Build output: http://queues.webkit.org/results/8338121

[Darin Adler]

Comment on attachment 88196 [details]
Proposed patch

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

Looks fine. The Windows build failed, so please make a new patch. I’ll say review- because we don’t want a review+ on a refactoring patch that failed on EWS.

> Source/JavaScriptCore/wtf/Alignment.h:3
> + *  Copyright (C) 2011 University of Szeged.

Since we just moved the code and made no changes, we should add a new copyright.

[Renata Hodovan]

Created attachment 89065 [details]
Proposed patch

[Renata Hodovan]

Created attachment 89168 [details]
Proposed patch

[Renata Hodovan]

I don't know what's the problem with ews. As Gabor mentioned it in the twin bug, svn-apply works fine locally... any idea?

[Gabor Loki]

(In reply to comment #14)
> I don't know what's the problem with ews. As Gabor mentioned it in the twin bug, svn-apply works fine locally... any idea?

Unfortunately the vcproj file needs CRLF on some EWS, and does not on the others. :(

[Renata Hodovan]

Any feedback? :)

[Gabor Loki]

Looks good to me! I couldn't have done a better job myself. ;)

[Eric Seidel (no email)]

Comment on attachment 89168 [details]
Proposed patch

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

> Source/JavaScriptCore/wtf/Alignment.h:21
> +#ifndef Alignment_h

I think this was supposed to be WTF_Alignment_h actually.

[Eric Seidel (no email)]

The clean-header-guards script might help you find the right guard. :)

[Renata Hodovan]

(In reply to comment #18)
> (From update of attachment 89168 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89168&action=review
> 
> > Source/JavaScriptCore/wtf/Alignment.h:21
> > +#ifndef Alignment_h
> 
> I think this was supposed to be WTF_Alignment_h actually.

Thanks to both of you. :)
Btw WTF_Alignment_h seems to be a good choice.

[Renata Hodovan]

Committed r84290: <http://trac.webkit.org/changeset/84290>

[Eric Seidel (no email)]

I think this broke the mac build.  I'm sad this was landed by hand. :(

[Eric Seidel (no email)]

pbxcp: Alignment.h: No such file or directory
pbxcp: Alignment.h: No such file or directory
** BUILD FAILED **


The following build commands failed:
JavaScriptCore:
	CpHeader /Projects/WebKit/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/PrivateHeaders/Alignment.h Alignment.h
(1 failure)

[WebKit Review Bot]

http://trac.webkit.org/changeset/84290 might have broken Leopard Intel Debug (Build) and SnowLeopard Intel Release (Build)