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

DataStorm linter suggested fixes #3293

Merged
merged 4 commits into from
Dec 19, 2024
Merged

DataStorm linter suggested fixes #3293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d1f89b
DataStorm fixes for C++ lints, nodiscard, and Ice qualification
pepone Dec 18, 2024
94dbd86
Merge remote-tracking branch 'origin/main' into datastorm-lint-fixes1
pepone Dec 19, 2024
331a640
cleanup clang-tidy
pepone Dec 19, 2024
e40bb81
Review fixes
pepone Dec 19, 2024
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 .clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ HeaderFilterRegex: '.*'
ExcludeHeaderFilterRegex: 'include/lmdb.h|Grammar.h'
UseColor: true
FormatStyle: 'file'
ExtraArgs: ['-std=c++20']
ExtraArgs: ['-std=c++17']
Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise we get lints for issues that require using C++ 20 and we are still using C++ 17 with GCC.

Copy link
Member

Choose a reason for hiding this comment

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

However, this breaks the linting of the Ice/span test. Maybe a better solution would be to disable the c++20 lints/suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could a .clang-tidy file to this test folder with an override to enable -std=c++20. I think clang-tidy supports this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a separate config for Ice/span test

CheckOptions:
modernize-use-nullptr.NullMacros: 'NULL'
# std::exception_ptr is a cheap to copy, pointer-like type; we pass it by value all the time.
Expand Down
47 changes: 21 additions & 26 deletions cpp/include/DataStorm/DataStorm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ namespace DataStorm
*
* @return The unread samples.
*/
std::vector<Sample<Key, Value, UpdateTag>> getAllUnread() noexcept;
[[nodiscard]] std::vector<Sample<Key, Value, UpdateTag>> getAllUnread() noexcept;

/**
* Wait for given number of unread samples to be available.
Expand All @@ -286,7 +286,7 @@ namespace DataStorm
* @return The unread sample.
* @throws NodeShutdownException If the node is shut down while waiting.
*/
Sample<Key, Value, UpdateTag> getNextUnread();
[[nodiscard]] Sample<Key, Value, UpdateTag> getNextUnread();

/**
* Calls the given functions to provide the initial set of connected keys and when a key is added or
Expand Down Expand Up @@ -420,14 +420,14 @@ namespace DataStorm
* @return The last written sample.
* @throws std::logic_error If there's no sample.
**/
Sample<Key, Value, UpdateTag> getLast();
[[nodiscard]] Sample<Key, Value, UpdateTag> getLast();

/**
* Get all the written sample kept in the writer history.
*
* @return The sample history.
**/
std::vector<Sample<Key, Value, UpdateTag>> getAll() noexcept;
[[nodiscard]] std::vector<Sample<Key, Value, UpdateTag>> getAll() noexcept;

/**
* Calls the given functions to provide the initial set of connected keys and when a key is added or
Expand Down Expand Up @@ -551,7 +551,7 @@ namespace DataStorm
*
* @return True if data writers are connected, false otherwise.
*/
bool hasWriters() const noexcept;
[[nodiscard]] bool hasWriters() const noexcept;

/**
* Wait for given number of data writers to be online.
Expand Down Expand Up @@ -580,7 +580,7 @@ namespace DataStorm
*
* @return True if data readers are connected, false otherwise.
*/
bool hasReaders() const noexcept;
[[nodiscard]] bool hasReaders() const noexcept;

/**
* Wait for given number of data readers to be online.
Expand Down Expand Up @@ -818,7 +818,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename K, typename V, typename UT>
SingleKeyReader<K, V, UT> makeSingleKeyReader(
[[nodiscard]] SingleKeyReader<K, V, UT> makeSingleKeyReader(
const Topic<K, V, UT>& topic,
const typename Topic<K, V, UT>::KeyType& key,
std::string name = std::string(),
Expand All @@ -838,7 +838,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename SFC, typename K, typename V, typename UT>
SingleKeyReader<K, V, UT> makeSingleKeyReader(
[[nodiscard]] SingleKeyReader<K, V, UT> makeSingleKeyReader(
const Topic<K, V, UT>& topic,
const typename Topic<K, V, UT>::KeyType& key,
const Filter<SFC>& sampleFilter,
Expand All @@ -860,7 +860,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename K, typename V, typename UT>
MultiKeyReader<K, V, UT> makeMultiKeyReader(
[[nodiscard]] MultiKeyReader<K, V, UT> makeMultiKeyReader(
const Topic<K, V, UT>& topic,
const std::vector<typename Topic<K, V, UT>::KeyType>& keys,
std::string name = std::string(),
Expand All @@ -882,7 +882,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename SFC, typename K, typename V, typename UT>
MultiKeyReader<K, V, UT> makeMultiKeyReader(
[[nodiscard]] MultiKeyReader<K, V, UT> makeMultiKeyReader(
const Topic<K, V, UT>& topic,
const std::vector<typename Topic<K, V, UT>::KeyType>& keys,
const Filter<SFC>& sampleFilter,
Expand All @@ -903,7 +903,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename K, typename V, typename UT>
MultiKeyReader<K, V, UT> makeAnyKeyReader(
[[nodiscard]] MultiKeyReader<K, V, UT> makeAnyKeyReader(
const Topic<K, V, UT>& topic,
std::string name = std::string(),
const ReaderConfig& config = ReaderConfig())
Expand All @@ -923,7 +923,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename SFC, typename K, typename V, typename UT>
MultiKeyReader<K, V, UT> makeAnyKeyReader(
[[nodiscard]] MultiKeyReader<K, V, UT> makeAnyKeyReader(
const Topic<K, V, UT>& topic,
const Filter<SFC>& sampleFilter,
std::string name = std::string(),
Expand Down Expand Up @@ -1006,7 +1006,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename KFC, typename K, typename V, typename UT>
FilteredKeyReader<K, V, UT> makeFilteredKeyReader(
[[nodiscard]] FilteredKeyReader<K, V, UT> makeFilteredKeyReader(
const Topic<K, V, UT>& topic,
const Filter<KFC>& filter,
std::string name = std::string(),
Expand All @@ -1026,7 +1026,7 @@ namespace DataStorm
* @param config The optional reader configuration.
*/
template<typename KFC, typename SFC, typename K, typename V, typename UT>
FilteredKeyReader<K, V, UT> makeFilteredKeyReader(
[[nodiscard]] FilteredKeyReader<K, V, UT> makeFilteredKeyReader(
const Topic<K, V, UT>& topic,
const Filter<KFC>& keyFilter,
const Filter<SFC>& sampleFilter,
Expand Down Expand Up @@ -1100,7 +1100,7 @@ namespace DataStorm
* @param tag The partial update tag.
*/
template<typename UpdateValue>
std::function<void(const UpdateValue&)> partialUpdate(const UpdateTag& tag) noexcept;
[[nodiscard]] std::function<void(const UpdateValue&)> partialUpdate(const UpdateTag& tag) noexcept;

/**
* Remove the data element. This generates a {@link Remove} sample.
Expand Down Expand Up @@ -1176,7 +1176,7 @@ namespace DataStorm
* @param tag The partial update tag.
*/
template<typename UpdateValue>
std::function<void(const Key&, const UpdateValue&)> partialUpdate(const UpdateTag& tag) noexcept;
[[nodiscard]] std::function<void(const Key&, const UpdateValue&)> partialUpdate(const UpdateTag& tag) noexcept;

/**
* Remove the data element. This generates a {@link Remove} sample.
Expand Down Expand Up @@ -1219,7 +1219,7 @@ namespace DataStorm
* @param config The optional writer configuration.
*/
template<typename K, typename V, typename UT>
MultiKeyWriter<K, V, UT> makeMultiKeyWriter(
[[nodiscard]] MultiKeyWriter<K, V, UT> makeMultiKeyWriter(
const Topic<K, V, UT>& topic,
const std::vector<typename Topic<K, V, UT>::KeyType>& keys,
std::string name = std::string(),
Expand All @@ -1237,22 +1237,17 @@ namespace DataStorm
* @param config The optional writer configuration.
*/
template<typename K, typename V, typename UT>
MultiKeyWriter<K, V, UT> makeAnyKeyWriter(
[[nodiscard]] MultiKeyWriter<K, V, UT> makeAnyKeyWriter(
const Topic<K, V, UT>& topic,
std::string name = std::string(),
const WriterConfig& config = WriterConfig()) noexcept
{
return MultiKeyWriter<K, V, UT>(topic, {}, std::move(name), config);
}

}

//
// Public template based API implementation
//

namespace DataStorm
Copy link
Member Author

Choose a reason for hiding this comment

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

no point in declaring the same namespace twice.

{
//
// Public template based API implementation
//

//
// Sample template implementation
Expand Down
28 changes: 14 additions & 14 deletions cpp/include/DataStorm/InternalI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace DataStormI
public:
virtual ~KeyFactory() = default;
[[nodiscard]] virtual std::shared_ptr<Key> get(std::int64_t) const = 0;
virtual std::shared_ptr<Key> decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0;
[[nodiscard]] virtual std::shared_ptr<Key> decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0;
};

class Tag : public virtual Element
Expand All @@ -64,7 +64,7 @@ namespace DataStormI
public:
virtual ~TagFactory() = default;
[[nodiscard]] virtual std::shared_ptr<Tag> get(std::int64_t) const = 0;
virtual std::shared_ptr<Tag> decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0;
[[nodiscard]] virtual std::shared_ptr<Tag> decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0;
};

class Sample : public Filterable
Expand Down Expand Up @@ -96,8 +96,8 @@ namespace DataStormI
virtual void setValue(const std::shared_ptr<Sample>&) = 0;

virtual void decode(const Ice::CommunicatorPtr&) = 0;
virtual const Ice::ByteSeq& encode(const Ice::CommunicatorPtr&) = 0;
virtual Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr&) = 0;
[[nodiscard]] virtual const Ice::ByteSeq& encode(const Ice::CommunicatorPtr&) = 0;
[[nodiscard]] virtual Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr&) = 0;

[[nodiscard]] const Ice::ByteSeq& getEncodedValue() const { return _encodedValue; }

Expand All @@ -118,7 +118,7 @@ namespace DataStormI
public:
virtual ~SampleFactory() = default;

virtual std::shared_ptr<Sample> create(
[[nodiscard]] virtual std::shared_ptr<Sample> create(
std::string,
std::string,
std::int64_t,
Expand Down Expand Up @@ -151,7 +151,7 @@ namespace DataStormI

[[nodiscard]] virtual std::shared_ptr<Filter> get(const std::string&, std::int64_t) const = 0;

virtual std::shared_ptr<Filter>
[[nodiscard]] virtual std::shared_ptr<Filter>
decode(const Ice::CommunicatorPtr&, const std::string&, const Ice::ByteSeq&) = 0;
};

Expand All @@ -178,14 +178,14 @@ namespace DataStormI
class DataReader : public virtual DataElement
{
public:
virtual bool hasWriters() = 0;
[[nodiscard]] virtual bool hasWriters() = 0;
virtual void waitForWriters(int) = 0;
[[nodiscard]] virtual int getInstanceCount() const = 0;

virtual std::vector<std::shared_ptr<Sample>> getAllUnread() = 0;
[[nodiscard]] virtual std::vector<std::shared_ptr<Sample>> getAllUnread() = 0;
virtual void waitForUnread(unsigned int) const = 0;
[[nodiscard]] virtual bool hasUnread() const = 0;
virtual std::shared_ptr<Sample> getNextUnread() = 0;
[[nodiscard]] virtual std::shared_ptr<Sample> getNextUnread() = 0;

virtual void onSamples(
std::function<void(const std::vector<std::shared_ptr<Sample>>&)>,
Expand Down Expand Up @@ -224,14 +224,14 @@ namespace DataStormI
class TopicReader : public virtual Topic
{
public:
virtual std::shared_ptr<DataReader> createFiltered(
[[nodiscard]] virtual std::shared_ptr<DataReader> createFiltered(
const std::shared_ptr<Filter>&,
std::string,
DataStorm::ReaderConfig,
std::string = std::string(),
Ice::ByteSeq = {}) = 0;

virtual std::shared_ptr<DataReader> create(
[[nodiscard]] virtual std::shared_ptr<DataReader> create(
const std::vector<std::shared_ptr<Key>>&,
std::string,
DataStorm::ReaderConfig,
Expand All @@ -246,7 +246,7 @@ namespace DataStormI
class TopicWriter : public virtual Topic
{
public:
virtual std::shared_ptr<DataWriter>
[[nodiscard]] virtual std::shared_ptr<DataWriter>
create(const std::vector<std::shared_ptr<Key>>&, std::string, DataStorm::WriterConfig) = 0;

virtual void setDefaultConfig(DataStorm::WriterConfig) = 0;
Expand All @@ -259,15 +259,15 @@ namespace DataStormI
public:
virtual ~TopicFactory() = default;

virtual std::shared_ptr<TopicReader> createTopicReader(
[[nodiscard]] virtual std::shared_ptr<TopicReader> createTopicReader(
std::string,
std::shared_ptr<KeyFactory>,
std::shared_ptr<TagFactory>,
std::shared_ptr<SampleFactory>,
std::shared_ptr<FilterManager>,
std::shared_ptr<FilterManager>) = 0;

virtual std::shared_ptr<TopicWriter> createTopicWriter(
[[nodiscard]] virtual std::shared_ptr<TopicWriter> createTopicWriter(
std::string,
std::shared_ptr<KeyFactory>,
std::shared_ptr<TagFactory>,
Expand Down
Loading
Loading