Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a forward declaration headers #1481

Closed
horenmar opened this issue Mar 20, 2020 · 8 comments
Closed

Provide a forward declaration headers #1481

horenmar opened this issue Mar 20, 2020 · 8 comments

Comments

@horenmar
Copy link

I have multiple places where I want to only deal with pointer to spdlog::logger, without having to include spdlog headers.

I can (and currently do) forward declare the class myself, but forward declaring names from thirdparty namespaces is generally considered poor form and I would like to avoid it.

@gabime
Copy link
Owner

gabime commented Mar 20, 2020

you can include only logger.h. It is is not fwd, but close enough..

@horenmar
Copy link
Author

logger.h includes at least 10 stdlib headers within 1 level of transitivity:

  • <atomic>
  • <chrono>
  • <functional>
  • <initializer_list>
  • <memory>
  • <mutex>
  • <exception>
  • <string>
  • <type_traits>
  • <vector>

@gabime
Copy link
Owner

gabime commented Mar 20, 2020

Fair enough. However I am not sure what meaningful fwd head is possible since a major part of the logger are templates.

@horenmar
Copy link
Author

The important part is that with a forward declaration, you can make (smart) pointers/references to a spdlog::logger without having to include spdlog/logger.h. Given the relative weight of that header, this can be quite beneficial.

To use this pointer/reference, you then do need to include the logger header, but that can be done in the .cpp file. The main use is that if a class caches logger lookup, I do not force all users of that class to also include all of the logger headers.

@horenmar
Copy link
Author

This is a utility header for measuring time taken by a blck of code we have in our codebase:

#include <memory>
#include <string>

namespace spdlog {
    class logger;
}

struct Timer {
    // Keeps track of how long it was running
};

struct TimerMeasureRAII {
    TimerMeasureRAII(std::string id, std::shared_ptr<spdlog::logger> const &logger)
        : m_id(std::move(id))
        , m_logger(logger)
    {}

    //! Initializes m_logger to `spdlog::get("main-logger")`.
    explicit TimerMeasureRAII(std::string id);

    //! Log the time elapsed since calling the constructor
    ~TimerMeasureRAII();
private:
    Timer m_timer;
    std::shared_ptr<spdlog::logger> m_logger;
    const std::string m_id;
    static thread_local std::size_t s_measure_nesting;
};

template <typename F>
auto Timer::measure(std::string id, F &&f) -> decltype(f()) {
    TimerMeasureRAII timer(std::move(id));
    return f();
}

template <typename F>
auto Timer::measure(std::string id, std::shared_ptr<spdlog::logger> const &logger, F &&f) -> decltype(f()) {
    TimerMeasureRAII timer(std::move(id), logger);
    return f();
}

Notice that it doesn't need to include spdlog/logger.h, so users of this header do not need to include it either.

@gabime
Copy link
Owner

gabime commented Mar 21, 2020

Not sure that those 3 lines really worth a new header file (on the other hand its mere existence might remind developers to use it which is good).

@horenmar
Copy link
Author

You should probably also provide fwd decls for other classes/types your users might want to customize, e.g. sinks.

But even if you provide just the logger, you should provide it because it is generally a bad practice to create forward declarations for 3rd namespaces.

@gabime
Copy link
Owner

gabime commented Mar 21, 2020

Fixed by adding spdlog/fwd.h file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants