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.

Environment

Status

Assignee

Jeff Genovy

Reporter

Jeff Genovy

Reviewer

Daniel Ju

Components

Fix versions

Priority

minor