Update macro_log typespec

[lukaszsamson]

log_allowed which is called by macro_log actually accepts location() or #{}. I noticed this issue when running dialyzer on elixir codebase. There are justified usages of macro_log with an empty map argument and dialyzer emits:

lib/logger.ex:1005:41: The call 'Elixir.Logger':'__do_log__'
         (_level@2 ::
              'alert' | 'critical' | 'debug' | 'emergency' | 'error' |
              'info' | 'notice' | 'warning',
          _message_or_fun@1 :: any(),
          #{},
          map()) will never return since it differs in the 3rd argument from the success typing arguments: 
         ('alert' | 'critical' | 'debug' | 'emergency' | 'error' |
          'info' | 'notice' | 'warning',
          any(),
          #{'file' := string(),
            'line' := non_neg_integer(),
            'mfa' := {atom(), atom(), non_neg_integer()}},
          map())

[github-actions]

CT Test Results

    2 files     70 suites   1h 6m 22s ⏱️
1 588 tests 1 333 ✅ 254 💤 1 ❌
1 840 runs  1 527 ✅ 312 💤 1 ❌

For more details on these failures, see this check.

Results for commit 8063623.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• No Windows Installer found

// Erlang/OTP Github Action Bot

[jhogberg]

There are justified usages of macro_log with an empty map argument and dialyzer emits:

macro_log/3,4,5 is undocumented (and explicitly noted as off-limits in logger.hrl), so there are no "justified usages." If we change how the macros work, the Elixir Logger will break because it's using this function. :-/

Since the calls land in log_allowed/4 immediately anyway, why not call that directly instead?

[lukaszsamson]

I'm not the autor of the elixir code so I don't know the reason macro_log was used. I can raise an issue on elixir tracker to make sure the elixir Logger depends on the right API.

Since the calls land in log_allowed/4 immediately anyway, why not call that directly instead?

I see that allow_log is not exported from https://github.com/erlang/otp/blob/master/lib/kernel/src/logger.erl

[jhogberg]

I'm not the autor of the elixir code so I don't know the reason macro_log was used. I can raise an issue on elixir tracker to make sure the elixir Logger depends on the right API.

Since the calls land in log_allowed/4 immediately anyway, why not call that directly instead?

I see that allow_log is not exported from https://github.com/erlang/otp/blob/master/lib/kernel/src/logger.erl

Sorry, I misread the code. log_allowed is indeed not exported, but as far as I can tell the same effect can be had by merging the location into the metadata before calling logger:log.

[lukaszsamson]

@josevalim Do you know the reason elixir is using hidden macro_log?
@jhogberg What is stopping us from improving the spec?

[josevalim]

Looking at the code I can see we have some logic between checking if something is allowed and then doing the logging but perhaps it is no longer necessary, since we have closed many of the gaps between Elixir and Erlang logging. I will take a another look.

[josevalim]

We need to call logger:macro_log for the same reason that Erlang does it:

otp/lib/kernel/include/logger.hrl

Line 68 in ec6323d

-define(DO_LOG(Level,Args),

We need to check for logger:allow and only then expand/evaluate the arguments given to the log macros. And, once we do so, we already checked if it is allowed, so no reason to do it again. Of course, it is no excuse to use private APIs and we would be on the hook if this changes, but I am not sure if this is enough to add a similar public function.