Bug 152122 - [B3] Fix unused-but-set-variable warning
Summary: [B3] Fix unused-but-set-variable warning
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks: 152248
  Show dependency treegraph
 
Reported: 2015-12-10 05:52 PST by Csaba Osztrogonác
Modified: 2015-12-14 04:56 PST (History)
9 users (show)

See Also:


Attachments
Patch (1.19 KB, patch)
2015-12-10 05:53 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (1.20 KB, patch)
2015-12-10 07:26 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2015-12-10 05:52:04 PST
../../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.
Comment 1 Csaba Osztrogonác 2015-12-10 05:53:34 PST
Created attachment 267098 [details]
Patch
Comment 2 Mark Lam 2015-12-10 07:22:25 PST
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.
Comment 3 Csaba Osztrogonác 2015-12-10 07:25:04 PST
(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.
Comment 4 Csaba Osztrogonác 2015-12-10 07:26:53 PST
Created attachment 267108 [details]
Patch

OK, let's use UNUSED_PARAM
Comment 5 Mark Lam 2015-12-10 07:28:58 PST
(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.
Comment 6 Mark Lam 2015-12-10 07:30:12 PST
(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.
Comment 7 WebKit Commit Bot 2015-12-10 08:08:19 PST
Comment on attachment 267108 [details]
Patch

Clearing flags on attachment: 267108

Committed r193905: <http://trac.webkit.org/changeset/193905>
Comment 8 WebKit Commit Bot 2015-12-10 08:08:23 PST
All reviewed patches have been landed.  Closing bug.