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.

Sign up for GitHub

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

ci: enable check for static non-POD #5602

Merged
merged 6 commits into from
Jan 27, 2019
Merged

ci: enable check for static non-POD #5602

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c915186
ci: enable check for static non-POD
lizan Jan 15, 2019
679bd49
add NOLINT properly
lizan Jan 15, 2019
57d1c47
NOLINTNEXTLINE
lizan Jan 15, 2019
b3669c0
NOLINT in macro
lizan Jan 15, 2019
7fbf23d
fix format
lizan Jan 16, 2019
31c24f3
Merge remote-tracking branch 'upstream/master' into static_initializa…
lizan Jan 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/envoy/singleton/manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ template <const char* name_param> class RegistrationImpl : public Registration {
*/
#define SINGLETON_MANAGER_REGISTRATION(NAME) \
static constexpr char NAME##_singleton_name[] = #NAME "_singleton"; \
static Envoy::Registry::RegisterFactory< \
static Envoy::Registry::RegisterFactory< /* NOLINT(fuchsia-statically-constructed-objects) */ \
Copy link
Member Author

Choose a reason for hiding this comment

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

@mattklein123 we can do sth like this for extensions if we use macro to register, WDYT? cc @htuch

Copy link
Member

Choose a reason for hiding this comment

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

I guess? In general this strikes me as solving a problem that doesn't actually need solving, but I guess if we insist on not having any non-POD statics in the code base we have to do ithis?

Copy link
Member

Choose a reason for hiding this comment

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

I think let's switch to a macro here, we might find this useful in the future due to the increased freedom of implementation.

On the point of being too draconian, my main wish being fulfilled with this is to reduce review noise re: static init fiasco and to avoid having a human in the loop missing the bad static init fiasco cases. So, if we can do this without too much NOLINT noise I'm still in favor.

Envoy::Singleton::RegistrationImpl<NAME##_singleton_name>, Envoy::Singleton::Registration> \
NAME##_singleton_registered_;

Expand Down