nmfs-ost / ss3-source-code

The source code for Stock Synthesis (SS3).
https://nmfs-ost.github.io/ss3-website/
Creative Commons Zero v1.0 Universal
37 stars 16 forks source link

[Refactor]: fix warning or update count of warnings in tests #369

Closed iantaylor-NOAA closed 2 years ago

iantaylor-NOAA commented 2 years ago

Refactor request

Recent runs of this test in github actions: https://github.com/nmfs-stock-synthesis/stock-synthesis/actions/workflows/call-build-ss3-warnings.yml have been failing due to the additional warning pasted below.

I don't know whether this warning is something that we should work to make go away or just update the tally of how many warnings are expected when compiling with warnings.

ss.cpp: In function ‘void write_message(int, int)’:
ss.cpp:160:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
  160 |         exitflag = 1;
      |         ~~~~~~~~~^~~
ss.cpp:161:7: note: here
  161 |       case ADJUST:
      |       ^~~~

The lines in question are in the following code at the top of ss.cpp:

  void write_message(int type, int echo)
  {
    int exitflag = 0;
    int warn = 0;
    std::string msg(warnstream.str());
    warnstream.str("");
    if (msg.length() == 0)
      msg = "unknown condition.";
    switch (type)
    {
      case NOTE:
      case SUGGEST:
      case PERFORM:
        N_note++;
        warn = N_note;
        warnstream << "Note " << N_note;
        break;
      case FATAL:
        exitflag = 1;
      case ADJUST:
      case WARN:
        N_warn++;
        warn = N_warn;
        warnstream << "Warning " << N_warn;
        break;
    }
    warnstream << MessageIntro(type) << msg;
    write_msg(warnstream.str(), echo, warn, exitflag);
    warnstream.str("");
  }

Expected behavior

Current tally is 68 warnings. I think this adds 2 more.

Rick-Methot-NOAA commented 2 years ago

I think anything related to compiler warnings should wait until after we switch to ADMB 13, which probably will cause more changes in compiler warnings.

iantaylor-NOAA commented 2 years ago

Good point. Happy to just ignore that test.

iantaylor-NOAA commented 2 years ago

Warnings are much reduced with ADMB 13.0 as noted in https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/257#issuecomment-1242476430 and the test is passing. The warnings test could be further improved as suggested in https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/452 but this issue can now be closed.