Added SOURCE_LOC macro

[mattw-nws]

Adds an easy way to improve exception messages (or other logging output)

Additions

Added a SOURCE_LOC macro in all.h that can be used to easily (and sustainably) report the location in code from which an exception was thrown or a log message was written.

Also adds some examples of use

Removals

Changes

Testing

1. Added to a Forcing object test, the actual string returned is not tested because this could easily change with unrelated modifications to Forcing.h, so would be a bad test. A commented-out cerr write is present that can be used to check the result.

Screenshots

[----------] 1 test from ForcingTest2
[ RUN      ] ForcingTest2.TestForcingDataLoad
Error: Forcing data ../../test/data/forcing/cat-27115-nwm-aorc-variant-derived-format.csv does not have the expected number of columns.  Did you mean to use csvPerFeature forcing provider instead? (/home/mattw-nws/ngen/include/forcing/Forcing.h:658)
[       OK ] ForcingTest2.TestForcingDataLoad (53618 ms)

Notes

Todos

Checklist

• PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• Code can be automatically merged (no conflicts)
• Code follows project standards (link if applicable)
• Passes all existing automated tests
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Placeholder code is flagged / future todos are captured in comments
• Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
• Reviewers requested with the Reviewers tool ➡️

Testing checklist (automated report can be put here)

Target Environment support

• Linux

[hellkite500]

It occurs to me that this solution is also useful with build/verbosity macros. Should use those here to conditionally define SOURCE_LOC only for debug builds. In production, this will produce paths that probably aren't meaningful, or at least would only be useful in a bug report with a version we could check out and find the right relative file from the absolute paths this reports.

[mattw-nws]

Hmm. We can turn these off in non-debug builds, sure, but actually I think they could still provide meaningful paths in the case of catching exceptions in production (which shouldn't happen, right?). The build path might be throwaway, but the trailing path and filename are still useful for the same reasons they are useful in dev. Since this is compile-time evaluated I don't think there's much cost, other than maybe infinitesimally larger binaries. I'd tend to vote to leave it in.

[hellkite500]

I agree, just thought it might be odd if you have a build system creating binaries that then report paths that don't even exist when an error is logged. I'm good with it as is for the time being.

[mattw-nws]

Yeah, that's not uncommon in my experience--potentially a little confusing but you quickly figure out what's going on. Something that just occurs to me... should we try embedding the Git commit ID into the file and hence the message?

[mattw-nws]

Eh...that's messier than it at first looks. Maybe not, or maybe later.

[hellkite500]

Maybe later...

[hellkite500]

After merging this I decided to do a little digging on #pragma once and opened a can of worms I wasn't quite expecting. Here is a good thread on the subject. As I read through this, it made me think that we should probably move to an include guard as used throughout the rest of the codebase, e.g.

#ifndef _NGEN_ALL
#define _NGEN_ALL

#endif //_NGEN_ALL

Just to ensure consistence with the rest of the code and not rely on a "non-standard" feature.