base: Don't generate unused BoundFunctorTraits typedef

base: Don't generate unused BoundFunctorTraits typedef

Having it there causes a warning or error (unused-local-typedefs).

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249808

[jadahl, 2014-01-10 13:27:47]

Hi,

I don't know if this has been fixed already, but this was causing compile errors
(since -Werror=unused-local-typedefs, GCC 4.8.1) here and I couldn't find
another review fixing it.

Jonas

[awong, 2014-01-10 17:18:53]

Hi Jonas,

Thanks for looking into this, but I think this typedef is indeed used:

https://code.google.com/p/chromium/codesearch#search/&q=BoundFunctorTraits&sq...

-Albert


On Fri, Jan 10, 2014 at 5:27 AM, <jadahl@opera.com> wrote:

> Reviewers: awong,
>
> Message:
> Hi,
>
> I don't know if this has been fixed already, but this was causing compile
> errors
> (since -Werror=unused-local-typedefs, GCC 4.8.1) here and I couldn't find
> another review fixing it.
>
> Jonas
>
> Description:
> base: Remove unused BoundFunctorTraits typedef
>
> Having it there causes a warning or error (unused-local-typedefs),
> so remove it.
>
> BUG=
>
> Please review this at https://codereview.chromium.org/133553003/
>
> SVN Base: http://git.chromium.org/chromium/src.git@master
>
> Affected files (+0, -6 lines):
>   M base/bind.h
>
>
> Index: base/bind.h
> diff --git a/base/bind.h b/base/bind.h
> index 5cf124df32f30bb5b4080d8370d1b05f199cc19e..
> b14f70c109f777ef78991990aedef270a2e82bc8 100644
> --- a/base/bind.h
> +++ b/base/bind.h
> @@ -65,12 +65,6 @@ Bind(Functor functor) {
>    typedef typename internal::FunctorTraits<Functor>::RunnableType
> RunnableType;
>    typedef typename internal::FunctorTraits<Functor>::RunType RunType;
>
> -  // Use RunnableType::RunType instead of RunType above because our
> -  // checks should below for bound references need to know what the actual
> -  // functor is going to interpret the argument as.
> -  typedef internal::FunctionTraits<typename RunnableType::RunType>
> -      BoundFunctorTraits;
> -
>    typedef internal::BindState<RunnableType, RunType, void()> BindState;
>
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jadahl, 2014-01-11 11:43:22]

On 2014/01/10 17:18:53, awong wrote:
> Hi Jonas,
> 
> Thanks for looking into this, but I think this typedef is indeed used:
> 
>
https://code.google.com/p/chromium/codesearch#search/&q=BoundFunctorTraits&sq...
> 

Hi,

There are multiple BoundFunctorTraits type definitions in bind.h; this patch
removes only one which AFAICS is unused.

Jonas

[jadahl, 2014-01-11 13:59:13]

Hi,

Uploaded a new version that also changes base/bind.h.pump.

Jonas

[akalin, 2014-01-28 23:30:02]

On 2014/01/11 13:59:13, jadahl wrote:
> Hi,
> 
> Uploaded a new version that also changes base/bind.h.pump.
> 
> Jonas

I'll let awong take care of this.

[jadahl, 2014-02-05 13:14:20]

On 2014/01/28 23:30:02, akalin wrote:
> On 2014/01/11 13:59:13, jadahl wrote:
> > Hi,
> > 
> > Uploaded a new version that also changes base/bind.h.pump.
> > 
> > Jonas
> 
> I'll let awong take care of this.

Ping?

[awong, 2014-02-05 19:51:07]

LGTM

Sorry for the delay.

[jadahl, 2014-02-06 08:32:00]

The CQ bit was checked by jadahl@opera.com

[commit-bot: I haz the power, 2014-02-06 08:34:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jadahl@opera.com/133553003/70001

[commit-bot: I haz the power, 2014-02-06 08:34:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-06 08:34:51]

Failed to apply patch for base/bind.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file base/bind.h
  Hunk #1 FAILED at 65.
  1 out of 1 hunk FAILED -- saving rejects to file base/bind.h.rej

Patch:       base/bind.h
Index: base/bind.h
diff --git a/base/bind.h b/base/bind.h
index
5cf124df32f30bb5b4080d8370d1b05f199cc19e..b14f70c109f777ef78991990aedef270a2e82bc8
100644
--- a/base/bind.h
+++ b/base/bind.h
@@ -65,12 +65,6 @@ Bind(Functor functor) {
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
   typedef typename internal::FunctorTraits<Functor>::RunType RunType;
 
-  // Use RunnableType::RunType instead of RunType above because our
-  // checks should below for bound references need to know what the actual
-  // functor is going to interpret the argument as.
-  typedef internal::FunctionTraits<typename RunnableType::RunType>
-      BoundFunctorTraits;
-
   typedef internal::BindState<RunnableType, RunType, void()> BindState;

[jadahl, 2014-02-06 08:40:21]

On 2014/02/06 08:34:51, I haz the power (commit-bot) wrote:
> Failed to apply patch for base/bind.h:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
>   patching file base/bind.h
>   Hunk #1 FAILED at 65.
>   1 out of 1 hunk FAILED -- saving rejects to file base/bind.h.rej
> 
> Patch:       base/bind.h
> Index: base/bind.h
> diff --git a/base/bind.h b/base/bind.h
> index
>
5cf124df32f30bb5b4080d8370d1b05f199cc19e..b14f70c109f777ef78991990aedef270a2e82bc8
> 100644
> --- a/base/bind.h
> +++ b/base/bind.h
> @@ -65,12 +65,6 @@ Bind(Functor functor) {
>    typedef typename internal::FunctorTraits<Functor>::RunnableType
RunnableType;
>    typedef typename internal::FunctorTraits<Functor>::RunType RunType;
>  
> -  // Use RunnableType::RunType instead of RunType above because our
> -  // checks should below for bound references need to know what the actual
> -  // functor is going to interpret the argument as.
> -  typedef internal::FunctionTraits<typename RunnableType::RunType>
> -      BoundFunctorTraits;
> -
>    typedef internal::BindState<RunnableType, RunType, void()> BindState;

An incorrect patch was uploaded and committed some days after this review
(identical to my initial version). I will rebase and upload the proper fix.

[jadahl, 2014-02-06 08:46:21]

New version uploaded. Please have a look.

[awong, 2014-02-06 22:06:02]

On 2014/02/06 08:46:21, jadahl wrote:
> New version uploaded. Please have a look.

I'm somewhat confused...gyp doesn't run pump for you.  You have to do it by
hand. I was actually expecting to see both bind.h and bind.h.pump in here...

[jadahl, 2014-02-07 08:32:53]

On 2014/02/06 22:06:02, awong wrote:
> On 2014/02/06 08:46:21, jadahl wrote:
> > New version uploaded. Please have a look.
> 
> I'm somewhat confused...gyp doesn't run pump for you.  You have to do it by
> hand. I was actually expecting to see both bind.h and bind.h.pump in here...

Indeed. This was the mistake you pointed out after having uploaded the first
patch. Then some days after that, someone else uploaded an identical patch to my
first version, which was promptly accepted and commited. This patch simply fixes
the pump file that previous patch missed to update. See
https://codereview.chromium.org/136553005/ .

[awong, 2014-02-07 17:26:49]

LGTM

On 2014/02/07 08:32:53, jadahl wrote:
> On 2014/02/06 22:06:02, awong wrote:
> > On 2014/02/06 08:46:21, jadahl wrote:
> > > New version uploaded. Please have a look.
> > 
> > I'm somewhat confused...gyp doesn't run pump for you.  You have to do it by
> > hand. I was actually expecting to see both bind.h and bind.h.pump in here...
> 
> Indeed. This was the mistake you pointed out after having uploaded the first
> patch. Then some days after that, someone else uploaded an identical patch to
my
> first version, which was promptly accepted and commited. This patch simply
fixes
> the pump file that previous patch missed to update. See
> https://codereview.chromium.org/136553005/ .

Gotcha. Cool. I'll CQ it for you.

[awong, 2014-02-07 17:26:53]

The CQ bit was checked by ajwong@chromium.org

[commit-bot: I haz the power, 2014-02-07 17:27:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jadahl@opera.com/133553003/190001

[commit-bot: I haz the power, 2014-02-07 22:06:41]

Change committed as 249808