Optionally write source location for logs

[VReaperV]

Based on #1741 because it adds the cmake option for building with a higher C++ standard.

Adds optional logging of source location with logs, controlled by logs.writeSrcLocation.* cvars, to help with debugging.

[slipher]

#1741 was merged so this needs a rebase

[VReaperV]

Rebased.

[slipher]

The main point of source_location is to avoid macros. So I rewrote it to avoid macros on the slipher/source_location branch. It also fixes the cvar multiple registration issue. (If you want to help work toward a future where registering a cvar from multiple modules is allowed, please review #1626!)

[VReaperV]

The main point of source_location is to avoid macros. So I rewrote it to avoid macros on the slipher/source_location branch.

That adds templates which means worse errors and increased build times. I'd rather just change it to use __FILE__ etc. and lose the column() part, but make it supported regardless of the c++ version.

[slipher]

That adds templates which means worse errors and increased build times.

It does add one more template, but I think it should be doing better overall on compile time/binary size, since it moves AddSrcLocation and cvar checks into the .cpp. But if it's a problem we can get rid of the template constructor for FormatStringT and put the 3 possible constructors for StringRef.

I'd rather just change it to use __FILE__ etc. and lose the column() part, but make it supported regardless of the c++ version.

Not a bad idea and it would make more sense than the current version. Macro-based logging APIs have certain advantages e.g. not evaluating arguments when the log level is disabled. In that case we should really redesign the API to avoid using common words like Warn as macros which is likely to break 3rd-party headers.

[VReaperV]

In that case we should really redesign the API to avoid using common words like Warn as macros which is likely to break 3rd-party headers.

How would you propose doing that? I mean, we use that everywhere in unv/daemon, it would be a huge compatibility break if we just rename it. Maybe new names as an altenative and deprecate the current ones, then remove them at some later point?

[sweet235]

How would you propose doing that? I mean, we use that everywhere in unv/daemon, it would be a huge compatibility break if we just rename it.

Yeah that is what they are proposing. I wonder who will bother to test this.

[slipher]

How would you propose doing that? I mean, we use that everywhere in unv/daemon, it would be a huge compatibility break if we just rename it.

Yeah that would be annoying; personally I would keep the version without macros. If we did change it maybe we could have both APIs exist for a period and you just wouldn't get the line number if you use the old one.

[VReaperV]

Changed it to use __FILE__ etc.

[illwieckz]

LGTM. If someone has a better idea for a part of this he can submit a patch after this is merged.

[illwieckz]

So, I only catch it now, because I did not do a LTO build before…

When I build with LTO enabled I get those errors when building the native DLL/EXE game (not the engine!):

In file included from Unvanquished/daemon/src/common/Common.h:43:
Unvanquished/daemon/src/common/Log.h:316:34: error: variable or field ‘DebugExt’ declared void
  316 |     #define Debug( format, ... ) DebugExt( __FILE__, __func__, __LINE__, format, ##__VA_ARGS__ )
      |                                  ^~~~~~~~
Unvanquished/src/sgame/sg_public.h:148:14: note: in expansion of macro ‘Debug’
  148 |         void Debug();
      |              ^~~~~
Unvanquished/daemon/src/common/Log.h:316:54: warning: ‘__func__’ is not defined outside of function scope
  316 |     #define Debug( format, ... ) DebugExt( __FILE__, __func__, __LINE__, format, ##__VA_ARGS__ )
      |                                                      ^~~~~~~~
Unvanquished/src/sgame/sg_public.h:148:14: note: in expansion of macro ‘Debug’
  148 |         void Debug();
      |              ^~~~~

@VReaperV Any idea on how to prevent that?

[illwieckz]

I also get that error when building the nexe game.

[slipher]

As I said above:

In that case we should really redesign the API to avoid using common words like Warn as macros which is likely to break 3rd-party headers.

We could switch to the version from the slipher/source_location branch which does not use macros.

[VReaperV]

Whose bright fucking idea was it to randomly declare a function called Debug and never even use it

[VReaperV]

Unvanquished/Unvanquished#3429 should fix it.