Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Remove unused AC_SUBSTs

RESOLVED FIXED in mozilla16

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
When I was saying bug 763987, I really meant bug 742795.
Blocks: 742795
No longer blocks: 763987
(Assignee)

Comment 2

5 years ago
Created attachment 632619 [details] [diff] [review]
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.
Attachment #632619 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 3

5 years ago
Created attachment 632631 [details] [diff] [review]
Remove unused AC_SUBSTs

Without a syntax problem after iconv check removal.
Attachment #632631 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #632619 - Attachment is obsolete: true
Attachment #632619 - Flags: review?(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?
Attachment #632631 - Flags: review?(ted.mielczarek) → review-
Assignee: nobody → mh+mozilla
(Assignee)

Comment 5

5 years ago
(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.
Oh, js/src/, duh.
Comment on attachment 632631 [details] [diff] [review]
Remove unused AC_SUBSTs

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

Hooray for reading comprehension.
Attachment #632631 - Flags: review- → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/85a4e915fc20
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/85a4e915fc20
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.