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

Switch uses of __attribute__((__constructor__)) to dynamic initializers #66

Closed
wants to merge 1 commit into from

Conversation

Orvid
Copy link
Contributor

@Orvid Orvid commented Nov 11, 2015

Because the attribute isn't portable, and the dynamic initializers do the same thing.

Because the attribute isn't portable, and the dynamic initializers do the same thing.
@yfeldblum
Copy link
Contributor

Looks like a fun SIOF nightmare waiting to happen.

Consider using folly::Singleton instead with eager initialization?

@Orvid
Copy link
Contributor Author

Orvid commented Nov 17, 2015

As a note, __attribute__((__constructor__)) runs at the same time as the initializers.

@yfeldblum
Copy link
Contributor

There is no set initialization order between files before main. This can easily cause bugs, especially in larger projects.

Better to have these values be done using some kind of singleton, either Meyers singleton (static local variable) or folly::Singleton. That will establish proper dependency ordering, and prevent this kind of bug.

@russoue
Copy link
Contributor

russoue commented Feb 12, 2016

@Orvid, are you planning to address @yfeldblum's comment?

@Orvid
Copy link
Contributor Author

Orvid commented Feb 12, 2016

Yep, but I'll bring it up as an internal diff when I do that.

@Orvid Orvid closed this Feb 12, 2016
ghost pushed a commit that referenced this pull request Mar 25, 2016
Summary:Because MSVC doesn't support it.

This is the new form of [Proxygen PR #66](#66).

This currently only changes one of them, as I'm looking for feedback to make sure this is what was wanted.

Reviewed By: afrind

Differential Revision: D2946935

fb-gh-sync-id: 8e98b28628851c58592d046bcfd954a9c579a9cc
fbshipit-source-id: 8e98b28628851c58592d046bcfd954a9c579a9cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants