900714 - Better handle -fno-strict-aliasing in xpcom/io/Makefile.in

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/file/default/xpcom/io/Makefile.in#l18 contains a workaround for bug 408258. However, bug 408258 appears to be necessary for ScriptableIO, which appears to have gone away a long time ago. On first glance, it's tempting to remove those lines and the addition of -fno-strict-aliasing to compiler flags.

Since the MODULE_OPTIMIZE_FLAGS mucking in xpcom/io/Makefile.in is global instead of target specific, changing this will remove -fno-strict-aliasing from a bunch of compilations in this directory. I'm not sure if that is desired.

I see the following options (in my personal preferred order):

1) Remove -fno-strict-aliasing globally, as it's no longer needed.
2) Only add -fno-strict-aliasing on targets that actually need it.
3) Update the comment to note general applicability.

bz: please advise

Comment 1

[Trevor Saunders (:tbsaunde)]

actually afaict that code is totally useless because C{,XX}FLAGS contain -fno-strict-aliasing globaly, so you can just remove it.

Comment 2

[Gregory Szorc [:gps]]

(In reply to Trevor Saunders (:tbsaunde) from comment #1)
> actually afaict that code is totally useless because C{,XX}FLAGS contain
> -fno-strict-aliasing globaly, so you can just remove it.

You are correct. https://mxr.mozilla.org/mozilla-central/source/configure.in#1236

Comment 3

[Gregory Szorc [:gps]]

Attachment: Remove legacy -fno-strict-aliasing compiler flag adjustment

Trivial patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yep, so two comments:

1)  As comment 1 points out we disable strict aliasing globally.
2)  The actual warnings attached to bug 408258 show warnings in the binary stream code,
    which is very much alive, sadly.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 784701 [details] [diff] [review]
Remove legacy -fno-strict-aliasing compiler flag adjustment

Sorry for the delay - somehow this slipped by. Looks good!

Comment 6

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69dc9d2c72cd

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/69dc9d2c72cd