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

Fix clang-tidy warnings for a batch of files #3245

Merged
merged 6 commits into from
Oct 20, 2021
Merged

Fix clang-tidy warnings for a batch of files #3245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
243f940
fix clang-tidy warnings for a batch of files
dragonly Oct 18, 2021
967f60f
Merge branch 'master' into dragonly/clang-tidy-batch-3
dragonly Oct 18, 2021
d1f890e
address comments
dragonly Oct 19, 2021
d5d1f81
Merge branch 'master' into dragonly/clang-tidy-batch-3
ti-chi-bot Oct 20, 2021
fa818f3
Merge branch 'master' into dragonly/clang-tidy-batch-3
ti-chi-bot Oct 20, 2021
3bf57dd
Merge branch 'master' into dragonly/clang-tidy-batch-3
ti-chi-bot Oct 20, 2021
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
6 changes: 3 additions & 3 deletions dbms/src/Common/CompactArray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ template <typename BucketIndex, UInt8 content_width, size_t bucket_count>
class CompactArray<BucketIndex, content_width, bucket_count>::Reader final
{
public:
Reader(ReadBuffer & in_)
explicit Reader(ReadBuffer & in_)
: in(in_)
{
}
Expand Down Expand Up @@ -177,7 +177,7 @@ class CompactArray<BucketIndex, content_width, bucket_count>::Locus final
friend class CompactArray::Reader;

public:
ALWAYS_INLINE operator UInt8() const
ALWAYS_INLINE explicit operator UInt8() const
{
if (content_l == content_r)
return read(*content_l);
Expand Down Expand Up @@ -211,7 +211,7 @@ class CompactArray<BucketIndex, content_width, bucket_count>::Locus final
private:
Locus() = default;

Locus(BucketIndex bucket_index)
explicit Locus(BucketIndex bucket_index)
{
init(bucket_index);
}
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Common/ConcurrentBoundedQueue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class ConcurrentBoundedQueue
Poco::Semaphore empty_count;

public:
ConcurrentBoundedQueue(size_t max_fill)
explicit ConcurrentBoundedQueue(size_t max_fill)
: fill_count(0, max_fill)
, empty_count(max_fill, max_fill)
{}
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Common/Config/ConfigReloader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void ConfigReloader::FilesChangesTracker::addIfExists(const std::string & path)
}
}

bool ConfigReloader::FilesChangesTracker::isDifferOrNewerThan(const FilesChangesTracker & rhs)
bool ConfigReloader::FilesChangesTracker::isDifferOrNewerThan(const FilesChangesTracker & rhs) const
{
return (files.size() != rhs.files.size()) || !std::equal(files.begin(), files.end(), rhs.files.begin(), FileWithTimestamp::isTheSame);
}
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Common/Config/ConfigReloader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ConfigReloader
void reload() { reloadIfNewer(/* force */ true, /* throw_on_error */ true); }

protected:
virtual void reloadIfNewer(bool force, bool throw_on_error);
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove the virtual?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise it prompts with this

$ clang-tidy-12 -p build/compile_commands.json dbms/src/Common/Config/ConfigReloader.cpp
26966 warnings generated.
/mnt/pingcap/tics/dbms/src/Common/Config/ConfigReloader.cpp:22:9: error: Call to virtual method 'ConfigReloader::reloadIfNewer' during construction bypasses virtual dispatch [clang-analyzer-optin.cplusplus.VirtualCall,-warnings-as-errors]
        reloadIfNewer(/* force = */ true, /* throw_on_error = */ true);
        ^
/mnt/pingcap/tics/dbms/src/Common/Config/ConfigReloader.cpp:21:9: note: Assuming 'already_loaded' is false
    if (!already_loaded)
        ^
/mnt/pingcap/tics/dbms/src/Common/Config/ConfigReloader.cpp:21:5: note: Taking true branch
    if (!already_loaded)
    ^
/mnt/pingcap/tics/dbms/src/Common/Config/ConfigReloader.cpp:22:9: note: Call to virtual method 'ConfigReloader::reloadIfNewer' during construction bypasses virtual dispatch
        reloadIfNewer(/* force = */ true, /* throw_on_error = */ true);
        ^

void reloadIfNewer(bool force, bool throw_on_error);
Updater & getUpdater() { return updater; }

private:
Expand All @@ -58,7 +58,7 @@ class ConfigReloader
std::set<FileWithTimestamp> files;

void addIfExists(const std::string & path);
bool isDifferOrNewerThan(const FilesChangesTracker & rhs);
bool isDifferOrNewerThan(const FilesChangesTracker & rhs) const;
};

FilesChangesTracker getNewFileList() const;
Expand Down
10 changes: 5 additions & 5 deletions dbms/src/Common/Config/TOMLConfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ bool TOMLConfiguration::getRaw(const std::string & key, std::string & value) con
}
}

bool TOMLConfiguration::find_parent(const std::string & key, TOMLTablePtr & parent, std::string & child_key)
bool TOMLConfiguration::findParent(const std::string & key, TOMLTablePtr & parent, std::string & child_key)
{
auto pos = key.find_last_of('.');

Expand Down Expand Up @@ -85,7 +85,7 @@ void TOMLConfiguration::setRaw(const std::string & key, const std::string & valu
{
TOMLTablePtr parent;
std::string child_key;
if (!find_parent(key, parent, child_key))
if (!findParent(key, parent, child_key))
throw Poco::NotFoundException("Key not found in TOML configuration", key);

parent->erase(child_key);
Expand All @@ -100,16 +100,16 @@ void TOMLConfiguration::enumerate(const std::string & key, Keys & range) const
if (!table)
return;

for (auto it = table->begin(); it != table->end(); it++)
range.push_back(it->first);
for (const auto & it : *table)
range.push_back(it.first);
}

void TOMLConfiguration::removeRaw(const std::string & key)
{
TOMLTablePtr parent;
std::string child_key;

if (find_parent(key, parent, child_key))
if (findParent(key, parent, child_key))
parent->erase(child_key);
}

Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Common/Config/TOMLConfiguration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ using TOMLTablePtr = std::shared_ptr<cpptoml::table>;
class TOMLConfiguration : public Poco::Util::AbstractConfiguration
{
public:
TOMLConfiguration(TOMLTablePtr toml_doc);
explicit TOMLConfiguration(TOMLTablePtr toml_doc);

protected:
bool getRaw(const std::string & key, std::string & value) const;
Expand All @@ -36,7 +36,7 @@ class TOMLConfiguration : public Poco::Util::AbstractConfiguration
/// Does nothing if the key does not exist.
private:
TOMLTablePtr root;
bool find_parent(const std::string & key, TOMLTablePtr & parent, std::string & child_key);
bool findParent(const std::string & key, TOMLTablePtr & parent, std::string & child_key);
};

} // namespace DB
2 changes: 1 addition & 1 deletion dbms/src/Common/CurrentMetrics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Increment
}

public:
Increment(Metric metric, Value amount = 1)
explicit Increment(Metric metric, Value amount = 1)
: Increment(&values[metric], amount)
{}

Expand Down