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

Don't always enable typeid usage under MSVC #3821

Merged
merged 1 commit into from
Jan 21, 2024
Merged

Conversation

edo9300
Copy link
Contributor

@edo9300 edo9300 commented Jan 20, 2024

FMT_USE_TYPEID is always being enabled currently when building with MSVC, even if the comment says that typeid is always available with MSVC, it is not exactly correct. In fact, building the first example from here

#include <iostream>
#include <typeinfo>

class Base {
public:
   virtual void vvfunc() {}
};

class Derived : public Base {};

using namespace std;
int main() {
   Derived* pd = new Derived;
   Base* pb = pd;
   cout << typeid( pb ).name() << endl;   //prints "class Base *"
   cout << typeid( *pb ).name() << endl;   //prints "class Derived"
   cout << typeid( pd ).name() << endl;   //prints "class Derived *"
   cout << typeid( *pd ).name() << endl;   //prints "class Derived"
   delete pd;
}

with exeptions disabled, will fail with the c++ exeption Access violation - no RTTI data! when calling typeid on the first dereferenced pointer. Even if this is not an issue with the current only place where it is used in fmt (haven't actually tested it), it's better to just disable it and not risk an exception being thrown when formatting an exception (would be pretty ironic tho).

@vitaut vitaut merged commit 06fc25f into fmtlib:master Jan 21, 2024
41 checks passed
@vitaut
Copy link
Contributor

vitaut commented Jan 21, 2024

Thanks

happymonkey1 pushed a commit to happymonkey1/fmt that referenced this pull request Apr 7, 2024
@edo9300 edo9300 deleted the patch-1 branch May 18, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants