Thanks, we'll see if we end up needing this or not.

I removed the "Connects to" because this will not be needed for the other pr's since you added the nolint lines.

Greetings! Thanks for this PR. Due to this gcc warning bug, we don't see any other way to allow use of the C++14 literals while also using cpplint. So we made this check somewhat more specific by ensuring the *literals namespace is coming from the std:: prefix.

For completeness I will repeat the comment we received from upstream about considering to propose this change to them:

We (internally) had a long discussion recently and came to the conclusion that using directives are never ever ever ever safe for use in a codebase of any significant size - we cannot endorse this, even just once. Sorry.

(I'm working on finding a proper forum to share some of the Google-internal guidance, hopefully early next year.)

-- google/styleguide#204 (comment)

Anyone following this should also read the follow up comment specifically about using namespace std::*literals:

google/styleguide#204 (comment)

OK. Well, this leaves our transition to standard C++14 literals then kind of stuck at the moment, due to conflicting ideals:

1. never allow a using-declaration
2. never allow a warning on CI that we merge

item 1 happens because we're trying not to diverge (too far) from the Google style guide

item 2 happens because, at least to my knowledge, there is no way to use the C++14 std::chrono literals that does not result in a gcc warning other than using namespace std::chrono_literals; because of the gcc warning bug linked above.

@dirk-thomas thanks for pointing that out. The file permission change is now reverted.

Thanks @Sarcasm for pointing out the inconsistency in the code and comment. The comment is updated now.

Hello @codebot,

TBH, I'm glad GCC has this warning bug because I don't consider the use of the using declaration elegant (personal taste).

Regarding restricting the match to std:: members, I'm not a big fan either but you are the judges.
In the following PR ros2/rclcpp#284,
I'm adding a user defined literal namespace to rclcpp.
I'm trying to follow the STL convention of a namespace named with literals, which I think is a convention quite a few people would follow naturally. I estimate the number of literals namespaces that will be abused by people in the ros2 codebase, without people noticing in the review, to be very close to 0 (risky estimate). Personally I think UDL are nice, for test code, examples, a bit less on real-world code, and I don't think the use should be limited to the STL, if Boost, or any other library defines some lietrals in a namespace, it makes sense to allow them when then follow the STL pattern of naming the namespace *literals.

I haven't given extensive thoughts on the subject but I'm wondering if the std::placeholders namespace could not be allowed as well.

Regarding the flexibility of the linting tool.
I'm wondering if clang-tidy (http://clang.llvm.org/extra/clang-tidy/checks/google-build-using-namespace.html) would not be more welcoming to configurable rules that diverges from the Google Style.
But I digress, clang-tidy integration would be significantly more difficult to integrate than cpplint.

Thanks @Sarcasm for the feedback. We just altered this so that it uses a whitelist that will be easier to change in the future if/when new STL or Boost other "standard-ish" libraries emerge that are well-behaved and convenient to use in this fashion. We also added std::placeholders as per your suggestion.

👍

@codebot I added a few literals in ea5846f and fixed the alphabetical ordering in 787ae55 (doh)

As much as I hate to have patches to cpplint in our project, I think this pr represents the most pragmatic compromise between using new features in the standard as they were intended versus avoiding the abuse of the using directive. So, +1 from me.

@dirk-thomas didn't we have a place to keep track of the changes we have on top of cpplint? I saw the url at the top of the file to the originating commit on the cpplint repository, but not an abbreviated list of changes or anything.

I don't think such a list exists anywhere beside the commit log (https://github.com/ament/ament_lint/commits/master/ament_cpplint/ament_cpplint/cpplint.py). All customizations are can be found in the commit after the last import from upstream (2324287#diff-b5c57d8cc3ce83e990089f94f5ab6e15).