We're updating the issue view to help you get more done. 

ICU4C: Treat select warnings as errors in the CI builds (MSVC) in library code

Description

As suggested by Markus in the ICU-TC call on 2018-10-03, we should likely consider treating some of the particular bad warnings as errors in the ICU CI builds in the library code. (This would help catch/prevent issues like during the RC/GA time-frame).

There are two main ways to do this:
(1) All warnings would be treated as errors – but we would disable "noisy" warnings.

(2) Warnings are still warnings. Only enable selection of warnings to be errors.

Both approaches have advantages/disadvantages:
Approach (1):

  • Disadvantage: Might cause more warnings to treated as errors initially.

  • Disadvantage: Updated compiler version might introduce new errors.

  • Advantage: Generally easier to list warnings that we want to "ignore", out of the many possible warnings.

Approach (2):

  • Disadvantage: Need to determine which warnings we want to treat as 'fatal' out of many hundreds of warnings.

  • Disadvantage: Might omit warnings that should have been errors.

  • Advantage: Less 'disruption' as most warnings would still be ignored by the CI builds.

A possible third option would be to setup another build line that treats warnings as errors, but this would require another build line + add more bandwidth usage to the GitHub LFS storage. (This is essentially ICU-13764)

For approach (1):
Possible noisy warnings to disable (suggested by Jungshik on other ICU ticket):

  • C4005 Macro redefinition. (W1)

  • C4068 Unknown pragma. (W1)

  • C4267 Conversion from 'size_t' to type. (W3)

For approach (2):
Possible warnings to make errors:

  • C4251 Export template instantiations. (W1)

  • C4715 Not all control paths return a value. (W1)

  • C4706 Assignment within conditional expression. (W4)

I'm slightly inclined towards option (1), but not strongly.

Status

Assignee

Jeff Genovy

Reporter

Jeff Genovy

Reviewer

Daniel Ju

Components

Fix versions

Priority

minor