Silence useless warning in src/json/json_spirit_writer_template.h to mak... #2980

pull wtogami wants to merge 1 commits into bitcoin:master from wtogami:silenceunusedwarning changing 1 files +2 −1

wtogami commented at 7:21 AM on September 8, 2013: contributor

Silence useless warning in src/json/json_spirit_writer_template.h to make important warnings easier to see.

warning: typedef ‘Char_type’ locally defined but not used [-Wunused-local-typedefs]
Silence useless warning in src/json/json_spirit_writer_template.h to make important warnings easier to see.
warning: typedef ‘Char_type’ locally defined but not used [-Wunused-local-typedefs]
a6b3de1395
jgarzik commented at 7:23 AM on September 8, 2013: contributor

ACK, though ideally a better solution could be found.

I think there is another pull req for updating json_spirit, and this sort of thing needs coordination with that PR.
Diapolo commented at 11:15 AM on September 8, 2013: none

NACK, we should not change code here. As far as I remember this is under some license, which permits it? There was a discussion about this some months ago.
wtogami commented at 11:54 AM on September 8, 2013: contributor

If bitcoin relies on code that doesn't permit changes, then we need a policy discussion.
laanwj commented at 12:11 PM on September 8, 2013: member

It's not a license problem @Diapolo . We just don't like json spirit changes that diverge further from upstream.
gmaxwell commented at 1:28 PM on September 8, 2013: contributor

ACK if and only if these changes have been submitted upstream (assuming they're still relevant upstream).

The warning noise has been an irritation for me to.
gavinandresen commented at 12:25 AM on September 9, 2013: contributor

I have a pet peeve for commenting out code; remove it if it should be removed. Or use a #pragma to suppress the warning if you're worried some obscure use of json_spirit might need the typedef.
BitcoinPullTester commented at 12:53 AM on September 9, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/a6b3de139554574075008546e725d1ea604b266f for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
sipa commented at 10:41 AM on September 13, 2013: member

Some time ago, there was a suggestion in a discussion to move json_spirit to a separate repository (like LevelDB), so such changes could be made outside the Bitcoin tree.

Having this submitted upstream sounds like the best solution here though.
laanwj commented at 1:41 PM on November 11, 2013: member

On the other hand we don't really keep up with upstream (http://www.codeproject.com/Articles/20027/JSON-Spirit-A-C-JSON-Parser-Generator-Implemented). Last time we tried to upgrade json spirit was a horrible fail that we had to revert immediately.

What's the verdict here? In general I agree with the "let's not diverge further from upstream" however seemingly we've already diverged and it's only a single line change so I'm fine with merging it (or deleting the line as @gavinandresen prefers).
Diapolo commented at 5:23 PM on November 11, 2013: none

I'm doing this also, so perhaps we should just merge it.

in src/json/json_spirit_writer_template.h:None in a6b3de1395

  27 | @@ -28,7 +28,8 @@
  28 |      template< class String_type >
  29 |      String_type non_printable_to_string( unsigned int c )
  30 |      {
  31 | -        typedef typename String_type::value_type Char_type;
  32 | +        // Silence the warning: typedef ‘Char_type’ locally defined but not used [-Wunused-local-typedefs]
  33 | +        // typedef typename String_type::value_type Char_type;

Diapolo commented at 5:24 PM on November 11, 2013:

I think it's cleaner to comment code out by using // in front of the code without indentation.

gavinandresen commented at 1:44 AM on November 12, 2013: contributor

Tired of talking about this....
gavinandresen referenced this in commit 07866e3cd6 on Nov 12, 2013
gavinandresen merged this on Nov 12, 2013
gavinandresen closed this on Nov 12, 2013
Bushstar referenced this in commit 8d5781f408 on Apr 8, 2020
DrahtBot locked this on Sep 8, 2021

Contributors