Ticket #8234 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Fix warnings which accidentally got into master

Reported by: Arturs Bekasovs Owned by: Arturs Bekasovs
Priority: minor Milestone: Release 3.0
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Russell Taylor

Description

They were introduced as a part of #6980 and I've not noticed them before closing the ticket:

[..]/Graph.cpp: In member function ‘void Graph::guessUniqueCurveLayout(int&, int&)’:
[..]/Graph.cpp:5030:64: warning: operation on ‘colorIndex’ may be undefined [-Wsequence-point]
[..]/Graph.cpp:5034:35: warning: operation on ‘symbolIndex’ may be undefined [-Wsequence-point]

Change History

comment:1 Changed 7 years ago by Arturs Bekasovs

  • Status changed from new to inprogress

Refs #8234. Replaced increments with a simple plus operations.

Changeset: 367e1bb5f614e86047b72658140740a04f644313

comment:2 Changed 7 years ago by Arturs Bekasovs

Tester:

Make sure warnings were resolved. Verify that what I've done shouldn't affect the way the code works.

I've checked the build server results, and it seems that Ubuntu 12.04 is only one which was noticing these problems. You can see them at http://download.mantidproject.org/jenkins/view/Develop%20Clean%20Builds%20and%20Tests/job/is_clean_ubuntu-12.04_develop/237/warnings20Result

To be 100% it's all OK and I've not broken #6980, please check the following:

  1. Plot some spectra of any data file.
  2. Set the color of the last spectra to pink.
  3. Add more spectra, and make sure they all get different colors, in the same order colors appear in the ColorBox. The white color should be skipped.
Last edited 7 years ago by Arturs Bekasovs (previous) (diff)

comment:3 Changed 7 years ago by Arturs Bekasovs

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:4 Changed 7 years ago by Russell Taylor

  • Status changed from verify to verifying
  • Tester set to Russell Taylor

comment:5 Changed 7 years ago by Russell Taylor

  • Status changed from verifying to closed

Merge remote branch 'origin/feature/8234_sequence_point_warnings'

Full changeset: 19937a7ad44c15e634eacddf30a533d35ce4caaa

comment:6 Changed 7 years ago by Russell Taylor

The warnings are gone, and I've learnt something new. The problem was that the colorIndex (or symbolIndex) variable was being assigned a new value twice in the expression - once via operator++ and once in the assignment. I thought using operator++ looked more concise and was fine, since the RHS has to be evaluated before being assigned to the LHS. But it turns out that the timing/order of the stores to colorIndex is not guaranteed - hence the warning. Of course, colorIndex+1 only reads from the variable so it's fine.

Last edited 7 years ago by Russell Taylor (previous) (diff)

comment:7 Changed 7 years ago by Arturs Bekasovs

Deleted

Last edited 7 years ago by Arturs Bekasovs (previous) (diff)

comment:8 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9079

Note: See TracTickets for help on using tickets.