「view this page in B3 βῆτα server」

Revisions №61443

branch: master 「№61443」
Commited by: Vikram K. Mulligan
GitHub commit link: 「2e03da6a6d3def9a」 「№5005」
Difference from previous tested commit:  code diff
Commit date: 2020-10-01 14:51:23

Merge pull request #5005 from RosettaCommons/vmullig/fix_modernize_use_nullptr_warnings Trying to fix clang-tidy warnings They're currently making a lot of red on the test server. These warnings are false alarms, and can be safely ignored, but I don't like having a lot of red lights that people are trained to ignore. So here there's a bit of a problem. The original point of pull request #4941 (which created these false alarms) was to deal with the fact that if a developer wrote `tag->getOption<bool>( "myoption", "false" )`, the string literal would be interpreted as a boolean `true`. To get around this, it was necessary to introduce a `Tag::getOption( std::string const &, char * const )` variant, but that means that `tag->getOption<core::Size>( "myoption", 0 )` gets interpreted as `nullptr` (even if I explicitly delete the variant for core::Size). This means I have to explicitly delete the variant for core::Size _and_ explicitly say `tag->getOption<core::Size>( "myoption", core::Size(0) )`, which is a bit frustrating... <ins>Edit:</ins> Rocco suggested implementing `Tag::getOption( std::string const&, int const)`, since `int` is a "preferred" interpretation of `0` (over `core::Size` or `nullptr`). This seems to work, and ensures that `tag->getOption<core::Size>( "myoption", 0 )` can still be used as it was previously. I'll leave the places where I converted to `tag->getOption<core::Size>( "myoption", core::Size(0) )` since they're harmless, but I won't bother converting the rest. Note that I am deliberately deleting the Real and bool versions of the int `getOption`, since doing so has revealed places where people were setting the wrong type. (So initializing a Real requires `tag->getOption("myoption", 0.0)`, and `tag->getOption("myoption", 0)` will result in an error. If people really hate this, we can switch it back later, but it does catch bugs.) <ins>Note to reviewers:</ins> I know that a lot of files have changed here, but most changes are trivial. There are five types of changes in this PR: 1. Changes to Tag.cc/Tag.hh. <b>These are the most important.</b> 2. A few places where the changes revealed the wrong type of datum being set (e.g. setting a bool with a tag parsed as a core::Real). The `SidechainMover` is one example. <b>These are not so important.</b> 3. Lots of places where `tag->getOption<core::Real>("myoption", <some int>)` was replaced with `tag->getOption<core::Real>("myoption", <the equivalent float>)`. These are largely cosmetic, but they allow us to be more pedantic about the structure of `getOption` so that we can catch the errors in number 2. <b>These are not so important.</b> 4. Some places where I was changing `tag->getOption<core::Size>("myoption", 0)` to `tag->getOption<core::Size>("myoption", core::Size(0))` before Rocco suggested a fix that no longer necessitated this. I've left these since it was too much trouble to change them all back and since they do no harm. <b>These are not so important.</b> 5. Addition of the `OLDER_CLANG` definition for clang < 4.0. This allows me to get around the problem described below. <ins>Added note:</ins> this PR revealed a bug in older versions of clang. In clang 3.4 (on the test server), it seems you can't have a deleted general template followed by an explicitly-defined template specialization (which is useful to say, "I don't want this function to accept anything but the types that I define, and I want a compiler error otherwise"). I'm special-casing that here by defining OLDER_CLANG when compiling with clang < 4.0, and implementing a general version of `T Tag::getOption<T>(std::string const &, int const) const` that throws an error if invoked if OLDER_CLANG is defined (using `delete` to turn this runtime error into a compile-time error for newer compilers). So for older clang we have the following pseudo-code: ```c++ class Tag { ... template< class T > T getOption( std::string const &, int const ) const { /*Throw error here at runtime if ever called.*/ } ... }; template <> core::Size Tag::getOption( std::string const &s, int const i ) const { /*Actually parse the tag without error.*/ } ``` On newer clang or on gcc or icc, it becomes: ```c++ class Tag { ... template< class T > T getOption( std::string const &, int const ) const = delete; //Compiler error instead of runtime error -- better for catching stuff (e.g. a `bool` or `string` initialized with `8`). ... }; template <> core::Size Tag::getOption( std::string const &s, int const i ) const { /*Actually parse the tag without error.*/ } ```

...