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

Fix crash on macOS with clang compiler #17898

Closed
wants to merge 7 commits into from
Closed

Fix crash on macOS with clang compiler #17898

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 3, 2016

Fixes #17714.

@ghost
Copy link
Author

ghost commented Aug 3, 2016

Oops, build bot just marked the check failed because Makefile is modified in this PR.

@@ -33,6 +33,7 @@ class distribution

distribution operator+( const distribution &other ) const;
distribution operator*( const distribution &other ) const;
distribution& operator=( const distribution &other );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copy assignment operator should be automatically created by the compiler. Does your compiler complain without this?

Copy link
Author

@ghost ghost Aug 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the whole story: #17714. I suspect clang compiler doesn't copy the std::function object properly with optimization switch turned on, causes the crash.

@ghost ghost mentioned this pull request Aug 8, 2016
@SpencerMichaels
Copy link
Contributor

Excellent work, @dlaboratory — Mac users express their gratitude! But I suspect the devs might be overlooking this fix since the Makefile changes make the build fail unconditionally; maybe you could split the bugfix and Makefile alterations into two separate commits?

@ghost
Copy link
Author

ghost commented Aug 16, 2016

Okay, changes on Makefile is reverted.

@ghost ghost closed this Aug 17, 2016
@ghost
Copy link
Author

ghost commented Aug 17, 2016

diff --git a/src/npc_class.cpp b/src/npc_class.cpp
index 7a6e467..f369a55 100644
--- a/src/npc_class.cpp
+++ b/src/npc_class.cpp
@@ -383,3 +383,10 @@ distribution distribution::operator*( const distribution &other ) const
         return my_fun() * other_fun();
     } );
 }
+
+distribution &distribution::operator=( const distribution &other )
+{
+    generator_function = other.generator_function;
+    return *this;
+}
+
diff --git a/src/npc_class.h b/src/npc_class.h
index 9e0182f..98c7233 100644
--- a/src/npc_class.h
+++ b/src/npc_class.h
@@ -33,6 +33,7 @@ class distribution

         distribution operator+( const distribution &other ) const;
         distribution operator*( const distribution &other ) const;
+        distribution &operator=( const distribution &other );

         static distribution constant( float val );
         static distribution rng_roll( int from, int to );

@Zireael07
Copy link
Contributor

Why are you closing issues before they can be reviewed/merged?

@cainiaowu
Copy link
Contributor

Yelling at a ghost wont give you any reply, he deleted his account.

@Soyweiser
Copy link
Contributor

Soyweiser commented Aug 23, 2016

Did he delete his/her account or github?

This pull request was closed.
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.

5 participants