Bug 152122 – [B3] Fix unused-but-set-variable warning

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

[Csaba Osztrogonác]

../../Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp: In member function 'void JSC::FTL::{anonymous}::LowerDFGToLLVM::lower()':
../../Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:207:14: error: variable 'hasVarargs' set but not used [-Werror=unused-but-set-variable]

hasVarargs variable is set unconditionally, but used only if FTL_USES_B3 is false.

[Csaba Osztrogonác]

Created attachment 267098 [details]
Patch

[Mark Lam]

Comment on attachment 267098 [details]
Patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:271
> +        (void) hasVarargs;

I think you should use UNUSED_PARAM(hasVarargs) instead.

[Csaba Osztrogonác]

(In reply to comment #2)
> Comment on attachment 267098 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267098&action=review
> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:271
> > +        (void) hasVarargs;
> 
> I think you should use UNUSED_PARAM(hasVarargs) instead.

We could ... it was my first idea too.

But I thought that the name of UNUSED_PARAM is confusing, 
because it isn't parameter, but a local variable.

[Csaba Osztrogonác]

Created attachment 267108 [details]
Patch

OK, let's use UNUSED_PARAM

[Mark Lam]

(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 267098 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=267098&action=review
> > 
> > > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:271
> > > +        (void) hasVarargs;
> > 
> > I think you should use UNUSED_PARAM(hasVarargs) instead.
> 
> We could ... it was my first idea too.
> 
> But I thought that the name of UNUSED_PARAM is confusing, 
> because it isn't parameter, but a local variable.

I agree regarding the naming, but we have been using that macro for this purpose all this time.  Perhaps it can be renamed to UNUSED_VAR() or UNUSED_VALUE().  I don't know if others have any opinion on this.

[Mark Lam]

(In reply to comment #5)
> ... Perhaps it can be renamed ...

I mean in a separate patch if you want to take on renaming it.  Or add a new macro for non-param values?  Not sure which is best.

[WebKit Commit Bot]

Comment on attachment 267108 [details]
Patch

Clearing flags on attachment: 267108

Committed r193905: <http://trac.webkit.org/changeset/193905>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.