Don't catch std::exception in Debug mode so we can catch array bound errors in debugger

[jmarrec]

Pull request overview

cmake will properly define NDEBUG on msvc as well in release mode cf https://gitlab.kitware.com/cmake/cmake/-/blob/1e35163a/Modules/Platform/Windows-MSVC.cmake#L481-483

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[mjwitte]

Oooh, does this mean I won't have to set a breakpoint here in std::vector anymore?

        _Xout_of_range("invalid vector subscript");

[Myoldmopar]

Oooh, does this mean I won't have to set a breakpoint here in std::vector anymore?

@mjwitte maybe? I mean this appears to just add a section of code inside a NDEBUG (Not-Debug) that will gracefully catch generic std::exceptions and report them to the error file nicely. I may edit the title of the PR to be a bit more clear. Or maybe I'm missing something. (@jmarrec ?)

But on that note, @mjwitte, if there is something we can do to make your debugging easier, I'm all for it. So you regularly have to just put a breakpoint in the vector code!?

[mjwitte]

But on that note, @mjwitte, if there is something we can do to make your debugging easier, I'm all for it. So you regularly have to just put a breakpoint in the vector code!?

Yep. If it's std::vector that's overrun, then you just get "invalid vector subscript" in the err output, even when running in the debugger. So adding a breakpoint let's you look up the chain to find the offending line. I'll test that with this branch.

[Myoldmopar]

I mean this appears to just add a section of code inside a NDEBUG

Geez, it's not adding the section of code, it's just nesting it inside ndebug. Yep, got it. This is great and should help. If you are able to confirm the behavior at some point, it would be great to drop this in. If not, I'll whip up a windows test situation.

[mjwitte]

sigh
I can't reproduce the need for a breakpoint in vector (I know I stumbled over that several times in the past month). But this change looks good to me.

[jmarrec]

sigh I can't reproduce the need for a breakpoint in vector (I know I stumbled over that several times in the past month). But this change looks good to me.

Can you try this #10677

Basically try to run https://github.com/NREL/EnergyPlusDevSupport/blob/master/DefectFiles/10000s/10665/in.idf

This is what prompted me to open this PR. I got annoyed one time too many about E+ gracefully handling an out of bounds error when I really do want it to throw so I know WHERE it's happening.

Definitely works on gcc/clang.

[mjwitte]

Hmm, I'm getting the same exception either way with that file.
With develop

With this branch

I may have altered some of the exception settings in my MSVC setup in the past to catch div by zero etc. Anyway, this isn't hurting anything, so I'm ok with it.

[Myoldmopar]

OK, let's drop this little maybe no-op / maybe improvement in. CI is completely clean so this won't affect any other PRs waiting on CI results. Thanks @jmarrec