microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 236 forks source link

Feature request: Slim source location without function name #1337

Closed jonwis closed 8 months ago

jonwis commented 1 year ago

Version

2.0.220131.2

Summary

The std::source_location is super useful, but very very noisy in terms of binary size. Turning it on for a certain OS binary increased its size from 1500kb to 2100kb, or about 40%. While the actual strings are likely to be optimized away into a cold section of the binary, the additional DLL size tradeoff is not appropriate. WIL, for instance, uses just __FILE__ and __LINE__ as the markers, and we've been very successful debugging with those.

There is no means to tell std::source_location to capture only file & line information.

I propose an option for winrt::source_location which uses a technique like

Reproducible example

#include <algorithm>
#include <source_location>

namespace winrt
{
    namespace details
    {
        struct source_location
        {
            const char* file;
            uint32_t line;
        };

        template<size_t length> struct string_literal {
            constexpr string_literal(const char(&str)[length]) {
                std::copy_n(str, length, value);
            }
            char value[length];
        };

        template<string_literal file, uint32_t line> struct source_loc
        {
            inline static constexpr source_location loc{file.value, line};
        };

        constexpr source_location from_std_location(std::source_location src)
        {
            source_location loc;
            loc.file = src.file_name();
            loc.line = src.line();
            return loc;
        }
    }
}

#define SOURCE_LOCATION_CURRENT winrt::details::source_loc<__FILE__, __LINE__>::loc
#define SOURCE_LOCATION_CURRENT_STD winrt::details::from_std_location(std::source_location::current())

Expected behavior

No response

Actual behavior

No response

Additional comments

No response

kennykerr commented 1 year ago

I'll defer to @dmachaj as this was his feature.

dmachaj commented 1 year ago

We have an email thread going on this topic. I think it would make sense for DEBUG to include full std::source_location and non-DEBUG to have a more minimal source_location that excludes the function name (which is the biggest binary impact).

sylveon commented 1 year ago

Might not be a terrible idea to fill a feature request at https://github.com/microsoft/STL to give users some macro/escape hatch to remove function name - I know of a few other people not wanting to use it specifically because it would embed function names into their binaries.

frederick-vs-ja commented 1 year ago

Possible implementation:

struct slim_source_location {
    [[nodiscard]] static consteval slim_source_location current(const uint_least32_t _line_ = __builtin_LINE(),
        const uint_least32_t _col_ = __builtin_COLUMN(), const char* const _file_ = __builtin_FILE()) noexcept {
        slim_source_location res{};
        res.line_   = _line_;
        res.column_ = _col_;
        res.file_   = _file_;
        return res;
    }

    [[nodiscard]] constexpr slim_source_location() noexcept = default;

    [[nodiscard]] constexpr uint_least32_t line() const noexcept {
        return line_;
    }
    [[nodiscard]] constexpr uint_least32_t column() const noexcept {
        return column_;
    }
    [[nodiscard]] constexpr const char* file_name() const noexcept {
        return file_;
    }

private:
    uint_least32_t line_{};
    uint_least32_t column_{};
    const char* file_ = "";
};
github-actions[bot] commented 1 year ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 12 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 11 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 11 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 10 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 10 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

dmachaj commented 8 months ago

@kennykerr the bot seems a bit overzealous. I don't see how attaching a PR to an issue counts as "no activity".

kennykerr commented 8 months ago

It's a standard github action. There may be an update or config option you can tweak.