You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Piotr Bartosiewicz (JIRA)" <ji...@apache.org> on 2010/09/22 22:55:34 UTC
[jira] Created: (THRIFT-916) gcc warnings in c++ header files
gcc warnings in c++ header files
--------------------------------
Key: THRIFT-916
URL: https://issues.apache.org/jira/browse/THRIFT-916
Project: Thrift
Issue Type: Improvement
Components: C++ - Library
Affects Versions: 0.4
Reporter: Piotr Bartosiewicz
I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
Currently only with -Wall thrift has no warnings.
Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
So the task is to improve the thrift headers.
I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by Christian Lavoie <ch...@gmail.com>.
On Mon, Oct 18, 2010 at 5:19 PM, Roger Meier (JIRA) <ji...@apache.org> wrote:
> Christian, I just used both of your patches.
> some parts of them result in additional warnings..
> {code}
> thriftl.cc:1643: warning: label find_rule defined but not used
> thriftl.cc: At global scope:
> thriftl.cc:2890: warning: void yyunput(int, char*) defined but not used
> thriftl.cc:3545: warning: int yy_flex_strlen(const char*) defined but not used
These happen with or without my patches (on my box anyway), I've been
ignoring them for now.
> src/concurrency/Mutex.cpp:192:46: warning: extra tokens at end of #ifdef directive
> src/concurrency/ThreadManager.cpp: In member function virtual void apache::thrift::concurrency::ThreadManager::Impl::addWorker(size_t):
> src/concurrency/ThreadManager.cpp:342: warning: declaration class apache::thrift::concurrency::ThreadManager::Worker does not declare anything
> src/concurrency/TimerManager.cpp:63: warning: declaration struct apache::thrift::concurrency::TimerManager::Dispatcher does not declare anything
> src/transport/TFileTransport.cpp: In member function bool
Ok, those are a surprise to me, I'll investigate.
apache::thrift::transport::TFileTransport::isEventCorrupted():
> src/transport/TFileTransport.cpp:748: warning: format %lld expects type long long int, but argument 7 has type long int
> src/transport/TFileTransport.cpp: In member function void apache::thrift::transport::TFileTransport::performRecovery():
> src/transport/TFileTransport.cpp:789: warning: format %llu expects type long long unsigned int, but argument 3 has type long int
Yeah, I was afraid of exactly that -- this is why I was leery of the
fix-format-strings patch -- the variables passed to snprintf are
arch-dependant or OS-dependant, so there might be no portable way to
snprintf them (most likely, they might just need to be declared with a
fixed-width type). I'll investigate.
> src/transport/TFileTransport.cpp: In member function virtual void apache::thrift::transport::TFileTransport::seekToChunk(int32_t):
> src/transport/TFileTransport.cpp:846: warning: comparison between signed and unsigned integer expressions
> {code}
Another surprise. I'll investigate.
Thanks for running these on your machine, appreciated.
>
>> gcc warnings in c++ header files
>> --------------------------------
>>
>> Key: THRIFT-916
>> URL: https://issues.apache.org/jira/browse/THRIFT-916
>> Project: Thrift
>> Issue Type: Improvement
>> Components: C++ - Library
>> Affects Versions: 0.4
>> Reporter: Piotr Bartosiewicz
>> Assignee: Roger Meier
>> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch
>>
>>
>> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
>> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
>> Currently only with -Wall thrift has no warnings.
>> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
>> So the task is to improve the thrift headers.
>> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>
--
Have fun, Christian
http://linkedin.christianlavoie.net
"I won't let you fall apart."
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Attachment: make-k.log
Actually, running the full thing with {{make -k}} (same configure parameters as above), I get another half-dozen files failing -- see the attached {{make-k.log}} file. Relevant program versions:
{noformat}
flex 2.5.35
Darwin -- 10.4.0 Darwin Kernel Version 10.4.0: Fri Apr 23 18:28:53 PDT 2010; root:xnu-1504.7.4~1/RELEASE_I386 i386
i686-apple-darwin10-g++-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5664)
{noformat}
I think your patch should be committed as-is, it's a clear improvement. It's not quite enough to resolve the bug for me, though.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Roger Meier reassigned THRIFT-916:
----------------------------------
Assignee: Roger Meier
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-java-style-and-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Attachment: v3-fix-all-warnings.patch
Without the style fixes.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch, v3-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Force of habit, deep nesting of {{if}} is harder to read to me.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922007#action_12922007 ]
Roger Meier commented on THRIFT-916:
------------------------------------
just commited the TSocket.cpp and TServerSocket.cpp bugfix (use snprintf instead of sprintf)
Thanks Christian!
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Attachment: v1-fix-format-strings.patch
v1-fix-random-silly-warnings.patch
Makes _almost_ everything (C++) compile with {{-Wall}} on my box, {{thriftl.cc}} still has a few warnings:
{noformat}
g++ -DHAVE_CONFIG_H -I. -I../.. -Wall -Wall -I./src -Wall -MT thrift-thriftl.o -MD -MP -MF .deps/thrift-thriftl.Tpo -c -o thrift-thriftl.o `test -f 'thriftl.cc' || echo './'`thriftl.cc
thriftl.cc: In function 'int yylex()':
thriftl.cc:1651: warning: comparison between signed and unsigned integer expressions
thriftl.cc: At global scope:
thriftl.cc:2877: warning: 'void yyunput(int, char*)' defined but not used
thriftl.cc:3531: warning: 'int yy_flex_strlen(const char*)' defined but not used
{noformat}
I haven't had time to look into {{-Wextra}} or {{-pedantic}} yet; though to be fully honest I find either to have much less useful value.
The {{fix-random-silly-warnings}} patch should be safe on all OSes. I'm a wee bit leery of the {{fix-format-strings}} patch, and would love it if someone on a not-Mac not-64-bit OS could test it.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922046#action_12922046 ]
Christian Lavoie commented on THRIFT-916:
-----------------------------------------
(Keep in mind that to test on Mac, you need to first apply my patches from THRIFT-953)
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922005#action_12922005 ]
Roger Meier commented on THRIFT-916:
------------------------------------
Christian, I just used both of your patches.
some parts of them result in additional warnings..
{code}
thriftl.cc:1643: warning: label find_rule defined but not used
thriftl.cc: At global scope:
thriftl.cc:2890: warning: void yyunput(int, char*) defined but not used
thriftl.cc:3545: warning: int yy_flex_strlen(const char*) defined but not used
src/concurrency/Mutex.cpp:192:46: warning: extra tokens at end of #ifdef directive
src/concurrency/ThreadManager.cpp: In member function virtual void apache::thrift::concurrency::ThreadManager::Impl::addWorker(size_t):
src/concurrency/ThreadManager.cpp:342: warning: declaration class apache::thrift::concurrency::ThreadManager::Worker does not declare anything
src/concurrency/TimerManager.cpp:63: warning: declaration struct apache::thrift::concurrency::TimerManager::Dispatcher does not declare anything
src/transport/TFileTransport.cpp: In member function bool apache::thrift::transport::TFileTransport::isEventCorrupted():
src/transport/TFileTransport.cpp:748: warning: format %lld expects type long long int, but argument 7 has type long int
src/transport/TFileTransport.cpp: In member function void apache::thrift::transport::TFileTransport::performRecovery():
src/transport/TFileTransport.cpp:789: warning: format %llu expects type long long unsigned int, but argument 3 has type long int
src/transport/TFileTransport.cpp: In member function virtual void apache::thrift::transport::TFileTransport::seekToChunk(int32_t):
src/transport/TFileTransport.cpp:846: warning: comparison between signed and unsigned integer expressions
{code}
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12924552#action_12924552 ]
Roger Meier commented on THRIFT-916:
------------------------------------
committed parts of [^v3-fix-all-warnings.patch] and missing member initialization within Thrift.h
from my perspective we should use _size_t_ instead of _unsigned int_ on several places.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch, v3-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922155#action_12922155 ]
David Reiss commented on THRIFT-916:
------------------------------------
Why did you change the "else if"s to "if"s in TFileTransport?
Also, I have a better fix for TDebugProtocol. I'll just commit it.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922367#action_12922367 ]
David Reiss commented on THRIFT-916:
------------------------------------
Oh, I didn't notice those all had returns in them. Still, I think that should be a separate diff.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920674#action_12920674 ]
David Reiss commented on THRIFT-916:
------------------------------------
Patch looks okay to me.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Attachment: v2-fix-all-warnings.patch
I don't think the two "declaration class $SOMETHING does not declare anything" were related to my previous patch, but here's a common patch that fixes all warnings in the C++ code when running with {{-Wall}} (with the exception of {{thriftl.cc}}, which I'm still ignoring for now).
This has been tested on 64 bit Mac OS X 10.6, 32 bit Debian testing (gcc 4.4, boost 1.42) and 64 bit Debian testing (gcc 4.4, boost 1.42). No warnings, apart the flex output.
If it doesn't work for you, please include the output of:
{noformat}
uname -a ; cat /etc/debian_release ; dpkg --list | grep -i libboost ; g++ --version
{noformat}
So I can try to reproduce locally.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Assignee: Roger Meier
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-format-strings.patch, v1-fix-java-style-and-warnings.patch, v1-fix-random-silly-warnings.patch, v2-fix-all-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Roger Meier updated THRIFT-916:
-------------------------------
Attachment: THRIFT-916_Wall_pedantic__noerrors.patch
With this patches [^THRIFT-916_Wall_pedantic__noerrors.patch] we have no errors on compiling with CXXFLAGS="-Wall -Wextra -pedantic" on GNU/Linux Debian Lenny
could someone please verify?
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: THRIFT-916_Wall_pedantic__noerrors.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921712#action_12921712 ]
Roger Meier commented on THRIFT-916:
------------------------------------
Thanks Christian!
I verified, tested and committed your patch.
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-java-style-and-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920591#action_12920591 ]
Christian Lavoie commented on THRIFT-916:
-----------------------------------------
Patch looks sane to me, but fails to build on my Mac:
{noformat}
Darwin -- 10.4.0 Darwin Kernel Version 10.4.0: Fri Apr 23 18:28:53 PDT 2010; root:xnu-1504.7.4~1/RELEASE_I386 i386
i686-apple-darwin10-g++-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5664)
{noformat}
With the following error message:
{noformat}
$ ./bootstrap.sh
$ export CFLAGS="-Wall -Werror -pedantic"
$ export CXXFLAGS="-Wall -Werror -pedantic"
$ export CPPFLAGS="-Wall -Werror -pedantic"
$ ./configure --prefix=$HOME/ports/src --with-cpp \
--without-php --without-php_extension --without-perl \
--without-python --without-ruby --without-haskell --without-java
[snip a lot]
g++ -DHAVE_CONFIG_H -I. -I../.. -Wall -Werror -pedantic -Wall -I./src -Wall -Werror -pedantic -MT thrift-t_java_generator.o -MD -MP -MF .deps/thrift-t_java_generator.Tpo -c -o thrift-t_java_generator.o `test -f 'src/generate/t_java_generator.cc' || echo './'`src/generate/t_java_generator.cc
cc1plus: warnings being treated as errors
src/generate/t_java_generator.cc: In member function 'void t_java_generator::generate_java_struct_clear(std::ofstream&, t_struct*)':
src/generate/t_java_generator.cc:3900: warning: enumeration value 'TYPE_VOID' not handled in switch
src/generate/t_java_generator.cc:3900: warning: enumeration value 'TYPE_STRING' not handled in switch
make[3]: *** [thrift-t_java_generator.o] Error 1
make[3]: *** Waiting for unfinished jobs....
mv -f .deps/thrift-t_cpp_generator.Tpo .deps/thrift-t_cpp_generator.Po
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
{noformat}
Did you disable the java generator, perchance?
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: THRIFT-916_Wall_pedantic__noerrors.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-916) gcc warnings in c++ header files
Posted by "Christian Lavoie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Lavoie updated THRIFT-916:
------------------------------------
Attachment: v1-fix-java-style-and-warnings.patch
This removes the following warnings when compiling the thrift compiler:
{noformat}
src/generate/t_java_generator.cc: In member function 'void t_java_generator::generate_java_struct_clear(std::ofstream&, t_struct*)':
src/generate/t_java_generator.cc:3900: warning: enumeration value 'TYPE_VOID' not handled in switch
src/generate/t_java_generator.cc:3900: warning: enumeration value 'TYPE_STRING' not handled in switch
{noformat}
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch, v1-fix-java-style-and-warnings.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-916) gcc warnings in c++ header files
Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920700#action_12920700 ]
Roger Meier commented on THRIFT-916:
------------------------------------
Thanks for Review! just commited that patch.
fully agree with Christian, this is not enough to resolve that issue, but at least a little part is done!
> gcc warnings in c++ header files
> --------------------------------
>
> Key: THRIFT-916
> URL: https://issues.apache.org/jira/browse/THRIFT-916
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Affects Versions: 0.4
> Reporter: Piotr Bartosiewicz
> Attachments: make-k.log, THRIFT-916_Wall_pedantic__noerrors.patch
>
>
> I tried to add an extra gcc warning options in my project but a number of warnings was emmited from thrift headers.
> Following the boost library recommendations, code should be clean with "-Wall -Wextra -pedantic" options.
> Currently only with -Wall thrift has no warnings.
> Other options worth considering (but require more work) are -Wconversion -Wold-style-cast
> So the task is to improve the thrift headers.
> I suggest also update all the thrift makefiles so the thrift sources an tests will indicate the faulty code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.