Skip to content
New issue

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

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

Already on GitHub? # to your account

Update checks for dynamic_cast usage when compiled with no rtti #3963

Merged
merged 2 commits into from
May 19, 2024

Conversation

edo9300
Copy link
Contributor

@edo9300 edo9300 commented May 18, 2024

The FMT_USE_TYPEID macro was renamed to FMT_HAS_RTTI and moved to fmt/base.h so that it can be used in other parts of the library.
That is now used to check wheter or not dynamic_cast should be used in the codebase, currently the only instances were in write_ostream_unicode.
This allows the library to be compiled properly with rtti disabled.

@vitaut
Copy link
Contributor

vitaut commented May 18, 2024

Could you provide a godbolt repro for the ostream issue?

@edo9300
Copy link
Contributor Author

edo9300 commented May 18, 2024

Could you provide a godbolt repro for the ostream issue?

It was more from a warning raised by visual studio when building my project with the latest fmt
C:\vcpkg2\installed\x86-windows-static\include\fmt\ostream.h(77,19): warning C4541: 'dynamic_cast' used on polymorphic type 'std::basic_streambuf<char,std::char_traits<char>>' with /GR-; unpredictable behavior may result, anyways I managed to generate a test case where this scenario actually occurs, since the code with dynamic cast is windows only, i'll just put the sample code here

#define FMT_HEADER_ONLY
#include <fmt/ostream.h>
#include <iostream>

struct custom_ostream :  private std::streambuf , public std::ostream
{
    custom_ostream() : std::ostream(this) {}
private:
    int overflow(int c) override
    {
        std::cout.put(c);
        return 0;
    }
};

int main()
{
    custom_ostream b;
    fmt::print(b, "working\n");
    return 0;
}

Building it as cl test.cpp /GR- /utf-8 /EHsc /I "fmt\include\path" will produce that warning, and when executing nothing will be printed (at least on my machine), building it with rtti on cl test.cpp /utf-8 /EHsc /I "fmt\include\path" will instead properly print the output to the console. This sample has been built with visual studio 2019.

@vitaut vitaut merged commit 028bffa into fmtlib:master May 19, 2024
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented May 19, 2024

Merged, thanks.

matt77hias pushed a commit to matt77hias/fmt that referenced this pull request May 20, 2024
…ib#3963)

* Rename FMT_USE_TYPEID to FMT_HAS_RTTI and use it as check to enable dynamic_cast usage

* FMT_HAS_RTTI->FMT_USE_RTTI
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants