764286 - Remove unused AC_SUBSTs

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Bug 763987 is going to make autoconf.mk autogenerated. As such, any AC_SUBST that is defined and that is not actually used in autoconf.mk and any other file listed in AC_OUTPUT may hurt. As a matter of fact, at least STL_FLAGS in js/src does.

Comment 1

[Mike Hommey [:glandium]]

When I was saying bug 763987, I really meant bug 742795.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Remove unused AC_SUBSTs

That just removes the AC_SUBSTs that have no use in autoconf.mk and other config.status-generated stuff. I'm pretty sure there are plenty variables defined in autoconf.mk but never used, but I didn't go after those.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Remove unused AC_SUBSTs

Without a syntax problem after iconv check removal.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 632631 [details] [diff] [review]
Remove unused AC_SUBSTs

Review of attachment 632631 [details] [diff] [review]:
-----------------------------------------------------------------

r- because I don't understand a few of these large code removals.

::: configure.in
@@ -809,5 @@
>  AC_SUBST(GNU_CXX)
>  AC_SUBST(INTEL_CC)
>  AC_SUBST(INTEL_CXX)
> -AC_SUBST(CLANG_CC)
> -AC_SUBST(CLANG_CXX)

Huh, we don't actually use these? I guess we can always add them back if we need them.

::: js/src/configure.in
@@ -574,5 @@
>          fi
>  
>          unset _MSVC_VER_FILTER
> -
> -        AC_CACHE_CHECK(for std::_Throw, ac_cv_have_std__Throw,

Why are you removing this hunk? That doesn't seem right.

@@ -3043,5 @@
> -        [ac_cv_func_iconv=no]
> -        )]
> -    )
> -if test "$ac_cv_func_iconv" = "yes"; then
> -    AC_DEFINE(HAVE_ICONV)

It looks like this define actually does get used, why is this getting removed?

Comment 5

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted] from comment #4)
> Comment on attachment 632631 [details] [diff] [review]
> Remove unused AC_SUBSTs
> 
> Review of attachment 632631 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r- because I don't understand a few of these large code removals.
> 
> ::: configure.in
> @@ -809,5 @@
> >  AC_SUBST(GNU_CXX)
> >  AC_SUBST(INTEL_CC)
> >  AC_SUBST(INTEL_CXX)
> > -AC_SUBST(CLANG_CC)
> > -AC_SUBST(CLANG_CXX)
> 
> Huh, we don't actually use these? I guess we can always add them back if we
> need them.

We only use them to trigger other tests during configure.

> ::: js/src/configure.in
> @@ -574,5 @@
> >          fi
> >  
> >          unset _MSVC_VER_FILTER
> > -
> > -        AC_CACHE_CHECK(for std::_Throw, ac_cv_have_std__Throw,
> 
> Why are you removing this hunk? That doesn't seem right.

The AC_SUBST corresponding to the test is not used in js/src. So it's not useful to keep the test.
 
> @@ -3043,5 @@
> > -        [ac_cv_func_iconv=no]
> > -        )]
> > -    )
> > -if test "$ac_cv_func_iconv" = "yes"; then
> > -    AC_DEFINE(HAVE_ICONV)
> 
> It looks like this define actually does get used

Not in js/src.

Comment 6

[(not currently active) Ted Mielczarek]

Oh, js/src/, duh.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 632631 [details] [diff] [review]
Remove unused AC_SUBSTs

Review of attachment 632631 [details] [diff] [review]:
-----------------------------------------------------------------

Hooray for reading comprehension.

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85a4e915fc20

Comment 9

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/85a4e915fc20