Skip to content

Commit ae8a255

Browse files
gkodinovgkodinov
committed
Addendum to the fix for Bug #34828882: Enable MSVC override checks
Enabled the override/virtual/final related warnings. Fixed the code to not produce these. Extended the msvc_cppcheck to treat angle braced #includes as system headers. Also made sure all rapidjson references are using angle braces and that they are not being reshuffled by the clang-format before the my_rapidjson_size_t.h file. Also fixed CL6053 in mysql_client_test.cc and added CL6240 to the list of ignored warnings. Change-Id: Ib7a60b6bdbb181667e91bf558c313e39aa21eb09
1 parent c9908eb commit ae8a255

File tree

24 files changed

+118
-127
lines changed

24 files changed

+118
-127
lines changed

‎cmake/msvc_cppcheck.cmake

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
OPTION(MSVC_CPPCHECK "Enable the extra MSVC CppCheck checks" OFF)
2424

25+
# check https://learn.microsoft.com/en-us/cpp/build/reference/analyze-code-analysis
26+
2527
MACRO(MSVC_CPPCHECK_ADD_SUPPRESSIONS)
2628
IF (MSVC AND MSVC_CPPCHECK)
2729
IF((NOT FORCE_UNSUPPORTED_COMPILER) AND MSVC_VERSION LESS 1929)
@@ -56,9 +58,7 @@ MACRO(MSVC_CPPCHECK_ADD_SUPPRESSIONS)
5658
STRING_APPEND(suppress_warnings " /wd26429") # Symbol '...' is never tested for nullness, it can be marked as not_null (f.23).
5759
STRING_APPEND(suppress_warnings " /wd26430") # Symbol '...' is not tested for nullness on all paths (f.23).
5860
STRING_APPEND(suppress_warnings " /wd26432") # If you define or delete any default operation in the type '...', define or delete them all (c.21).
59-
STRING_APPEND(suppress_warnings " /wd26433") # Function '...' should be marked with 'override' (c.128)
6061
STRING_APPEND(suppress_warnings " /wd26434") # Function '..' hides a non-virtual function '..'.
61-
STRING_APPEND(suppress_warnings " /wd26435") # Function '..' should specify exactly one of virtual, override or final (c.128)
6262
STRING_APPEND(suppress_warnings " /wd26436") # The type '...' with a virtual function needs either public virtual or protected non-virtual destructor (c.35).
6363
STRING_APPEND(suppress_warnings " /wd26438") # Avoid 'goto' (es.76)
6464
STRING_APPEND(suppress_warnings " /wd26439") # This kind of function should not throw. Declare it 'noexcept' (f.6).
@@ -119,6 +119,7 @@ MACRO(MSVC_CPPCHECK_ADD_SUPPRESSIONS)
119119
STRING_APPEND(suppress_warnings " /wd6200") # Index '..' is out of valid index range '..' to '..' for non-stack buffer '...'.
120120
STRING_APPEND(suppress_warnings " /wd6237") # (zero && expression) is always zero. expression is never evaluated and might have side effects
121121
STRING_APPEND(suppress_warnings " /wd6239") # (non-zero constant && expression) always evaluates to the result of expression.
122+
STRING_APPEND(suppress_warnings " /wd6240") # (<expression> && <non-zero constant>) always evaluates to the result of <expression>
122123
STRING_APPEND(suppress_warnings " /wd6244") # Local declaration of '...' hides previous declaration at line '...' of '...'
123124
STRING_APPEND(suppress_warnings " /wd6255") # _alloca indicates failure by raising a stack overflow exception. Consider using _malloca instead
124125
STRING_APPEND(suppress_warnings " /wd6258") # Using TerminateThread does not allow proper thread clean up.
@@ -149,6 +150,13 @@ MACRO(MSVC_CPPCHECK_ADD_ANALYZE)
149150
IF (MSVC AND MSVC_CPPCHECK)
150151
STRING_APPEND(CMAKE_C_FLAGS " /analyze /analyze:external- /analyze:pluginEspXEngine.dll")
151152
STRING_APPEND(CMAKE_CXX_FLAGS " /analyze /analyze:external- /analyze:pluginEspXEngine.dll")
153+
# cmake pre 3.24 doesn't support /external:I for older compilers,
154+
# so use angle brackets as a substitute.
155+
IF((CMAKE_VERSION VERSION_LESS 3.24) OR
156+
(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 19.29.30036.3))
157+
STRING_APPEND(CMAKE_C_FLAGS " /external:anglebrackets")
158+
STRING_APPEND(CMAKE_CXX_FLAGS " /external:anglebrackets")
159+
ENDIF()
152160
ENDIF()
153161
ENDMACRO()
154162

‎components/keyrings/common/config/config_reader.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
#define RAPIDJSON_HAS_STDSTRING 1
2929

3030
#include "my_rapidjson_size_t.h"
31-
#include "rapidjson/document.h"
31+
32+
#include <rapidjson/document.h>
3233

3334
namespace keyring_common {
3435
namespace config {

‎components/keyrings/common/json_data/json_reader.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
#define RAPIDJSON_HAS_STDSTRING 1
3131

3232
#include "my_rapidjson_size_t.h"
33-
#include "rapidjson/document.h"
34-
#include "rapidjson/schema.h"
33+
34+
#include <rapidjson/document.h>
35+
#include <rapidjson/schema.h>
3536

3637
#include "components/keyrings/common/data/data.h"
3738
#include "components/keyrings/common/data/meta.h"

‎components/keyrings/common/json_data/json_writer.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@
2828
#define RAPIDJSON_HAS_STDSTRING 1
2929

3030
#include "my_rapidjson_size_t.h"
31-
#include "rapidjson/document.h"
32-
#include "rapidjson/stringbuffer.h"
33-
#include "rapidjson/writer.h"
31+
32+
#include <rapidjson/document.h>
33+
#include <rapidjson/stringbuffer.h>
34+
#include <rapidjson/writer.h>
3435

3536
#include "components/keyrings/common/data/data.h"
3637
#include "components/keyrings/common/data/meta.h"

‎components/keyrings/keyring_file/config/config.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@
2525
#define RAPIDJSON_HAS_STDSTRING 1
2626

2727
#include "my_rapidjson_size_t.h"
28-
#include "rapidjson/document.h"
29-
#include "rapidjson/stringbuffer.h"
30-
#include "rapidjson/writer.h"
28+
29+
#include <rapidjson/document.h>
30+
#include <rapidjson/stringbuffer.h>
31+
#include <rapidjson/writer.h>
3132

3233
#include <components/keyrings/keyring_file/keyring_file.h>
3334

‎components/logging/log_sink_json.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ error log and as JSON\"", "time" : "1970-01-01T00:00:00.000000Z", "thread" : 0,
5555

5656
#ifdef WITH_LOG_PARSER
5757
#include "my_rapidjson_size_t.h"
58-
#include "rapidjson/document.h"
58+
59+
#include <rapidjson/document.h>
5960
#endif
6061

6162
REQUIRES_SERVICE_PLACEHOLDER(log_builtins);

‎include/manifest.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
#include "scope_guard.h"
3131

3232
#include "my_rapidjson_size_t.h"
33-
#include "rapidjson/document.h"
34-
#include "rapidjson/schema.h"
33+
34+
#include <rapidjson/document.h>
35+
#include <rapidjson/schema.h>
3536

3637
namespace manifest {
3738

‎libbinlogevents/include/control_events.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -883,8 +883,8 @@ class Transaction_payload_event : public Binary_log_event {
883883
std::string to_string() const;
884884

885885
#ifndef HAVE_MYSYS
886-
virtual void print_event_info(std::ostream &) override;
887-
virtual void print_long_info(std::ostream &) override;
886+
void print_event_info(std::ostream &) override;
887+
void print_long_info(std::ostream &) override;
888888
#endif
889889
};
890890

@@ -1111,7 +1111,7 @@ class Gtid_event : public Binary_log_event {
11111111
static const std::int64_t GNO_END = INT64_MAX;
11121112

11131113
public:
1114-
std::int64_t get_gno() const { return gtid_info_struct.rpl_gtid_gno; }
1114+
virtual std::int64_t get_gno() const { return gtid_info_struct.rpl_gtid_gno; }
11151115
Uuid get_uuid() const { return Uuid_parent_struct; }
11161116
/// Total length of post header
11171117
static const int POST_HEADER_LENGTH =
@@ -1527,8 +1527,8 @@ class Heartbeat_event_v2 : public Binary_log_event {
15271527
// Return the max length of an encoded packet.
15281528
static uint64_t max_encoding_length();
15291529
#ifndef HAVE_MYSYS
1530-
virtual void print_event_info(std::ostream &info) override;
1531-
virtual void print_long_info(std::ostream &info) override;
1530+
void print_event_info(std::ostream &info) override;
1531+
void print_long_info(std::ostream &info) override;
15321532
#endif
15331533
protected:
15341534
std::string m_log_filename{};

‎libchangestreams/include/mysql/cs/reader/binary/mysqlproto.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class Mysql_protocol : public cs::reader::Reader {
9090
*
9191
* @returns true if the disconnection hit a problem, false otherwise.
9292
*/
93-
virtual bool close() override;
93+
bool close() override;
9494

9595
/**
9696
* @brief This member function attaches this connector to the stream.
@@ -112,7 +112,7 @@ class Mysql_protocol : public cs::reader::Reader {
112112
*
113113
* @returns true if there is a failure, false otherwise.
114114
*/
115-
virtual bool open(std::shared_ptr<State> state) override;
115+
bool open(std::shared_ptr<State> state) override;
116116

117117
/**
118118
* @brief Gets the next entry in the stream and puts it in the buffer.
@@ -122,7 +122,7 @@ class Mysql_protocol : public cs::reader::Reader {
122122
* @returns true if there was an error while reading from the stream, false
123123
* otherwise.
124124
*/
125-
virtual bool read(std::vector<uint8_t> &buffer) override;
125+
bool read(std::vector<uint8_t> &buffer) override;
126126

127127
/**
128128
* @brief Get the state object.
@@ -135,7 +135,7 @@ class Mysql_protocol : public cs::reader::Reader {
135135
* @return the state of the stream at this point in time. Returns nullptr if
136136
* the stream has not been opened yet.
137137
*/
138-
virtual std::shared_ptr<State> get_state() const override;
138+
std::shared_ptr<State> get_state() const override;
139139

140140
/**
141141
* @brief Get the mysql connection handle.

‎libchangestreams/include/mysql/cs/reader/binary/tracker.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ class Tracker : public cs::reader::Base_tracker {
7070
* @return true if there was an error.
7171
* @return false if there was no error.
7272
*/
73-
virtual bool track_and_update(std::shared_ptr<State> state,
74-
const std::vector<uint8_t> &buffer) override;
73+
bool track_and_update(std::shared_ptr<State> state,
74+
const std::vector<uint8_t> &buffer) override;
7575
};
7676

7777
} // namespace cs::reader::binary

‎libmysql/authentication_win/handshake_client.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ class Handshake_client : public Handshake {
4343

4444
public:
4545
Handshake_client(Connection &con, const char *target, size_t len);
46-
~Handshake_client();
46+
~Handshake_client() override;
4747

4848
Blob first_packet();
49-
Blob process_data(const Blob &);
49+
Blob process_data(const Blob &) override;
5050

51-
Blob read_packet();
52-
int write_packet(Blob &data);
51+
Blob read_packet() override;
52+
int write_packet(Blob &data) override;
5353
};
5454

5555
/**

‎plugin/x/tests/driver/json_to_any_handler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include <cstdint>
2929
#include <stack>
3030

31-
#include "my_rapidjson_size_t.h" // IWYU pragma: keep
31+
#include <my_rapidjson_size_t.h> // IWYU pragma: keep
3232

3333
#include <rapidjson/document.h>
3434
#include <rapidjson/stringbuffer.h>

‎sql/auth/sql_user_table.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,11 +1774,11 @@ class acl_tables_setup_for_write_and_acquire_mdl_error_handler
17741774
@param [in] msg Message string. Unused.
17751775
*/
17761776

1777-
virtual bool handle_condition(THD *thd [[maybe_unused]], uint sql_errno,
1778-
const char *sqlstate [[maybe_unused]],
1779-
Sql_condition::enum_severity_level *level
1780-
[[maybe_unused]],
1781-
const char *msg [[maybe_unused]]) override {
1777+
bool handle_condition(THD *thd [[maybe_unused]], uint sql_errno,
1778+
const char *sqlstate [[maybe_unused]],
1779+
Sql_condition::enum_severity_level *level
1780+
[[maybe_unused]],
1781+
const char *msg [[maybe_unused]]) override {
17821782
m_hit_deadlock = (sql_errno == ER_LOCK_DEADLOCK);
17831783
return m_hit_deadlock;
17841784
}

‎sql/changestreams/misc/column_filters/column_filter_inbound_func_indexes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ namespace util {
5757
that data.
5858
*/
5959
class ColumnFilterInboundFunctionalIndexes : public ColumnFilter {
60-
virtual bool filter_column(TABLE const *table, size_t column_index) override;
60+
bool filter_column(TABLE const *table, size_t column_index) override;
6161
/**
6262
@brief Is this filter needed given context passed in the parameters
6363

‎sql/changestreams/misc/column_filters/column_filter_inbound_gipk.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ namespace util {
4747
This class filters the first column on iteration.
4848
*/
4949
class ColumnFilterInboundGipk : public ColumnFilter {
50-
virtual bool filter_column(TABLE const *, size_t column_index) override;
50+
bool filter_column(TABLE const *, size_t column_index) override;
5151

5252
/**
5353
@brief Is this filter needed given context passed in the parameters

‎sql/changestreams/misc/column_filters/column_filter_outbound_func_indexes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ namespace util {
5656
Outbound states we are encoding something to be transmitted.
5757
*/
5858
class ColumnFilterOutboundFunctionalIndexes : public ColumnFilter {
59-
virtual bool filter_column(TABLE const *table, size_t column_index) override;
59+
bool filter_column(TABLE const *table, size_t column_index) override;
6060
/**
6161
@brief Is this filter needed given context passed in the parameters
6262

‎sql/conn_handler/named_pipe_connection.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class Channel_info_named_pipe : public Channel_info {
5050
HANDLE m_handle;
5151

5252
protected:
53-
virtual Vio *create_and_init_vio() const {
53+
Vio *create_and_init_vio() const override {
5454
return vio_new_win32pipe(m_handle);
5555
}
5656

@@ -62,7 +62,7 @@ class Channel_info_named_pipe : public Channel_info {
6262
*/
6363
Channel_info_named_pipe(HANDLE handle) : m_handle(handle) {}
6464

65-
virtual THD *create_thd() {
65+
THD *create_thd() override {
6666
THD *thd = Channel_info::create_thd();
6767

6868
if (thd != NULL) {
@@ -72,8 +72,8 @@ class Channel_info_named_pipe : public Channel_info {
7272
return thd;
7373
}
7474

75-
virtual void send_error_and_close_channel(uint errorcode, int error,
76-
bool senderror) {
75+
void send_error_and_close_channel(uint errorcode, int error,
76+
bool senderror) override {
7777
Channel_info::send_error_and_close_channel(errorcode, error, senderror);
7878

7979
DisconnectNamedPipe(m_handle);

‎sql/conn_handler/shared_memory_connection.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class Channel_info_shared_mem : public Channel_info {
5656
HANDLE m_event_conn_closed;
5757

5858
protected:
59-
virtual Vio *create_and_init_vio() const {
59+
Vio *create_and_init_vio() const override {
6060
return vio_new_win32shared_memory(m_handle_client_file_map,
6161
m_handle_client_map, m_event_client_wrote,
6262
m_event_client_read, m_event_server_wrote,
@@ -87,7 +87,7 @@ class Channel_info_shared_mem : public Channel_info {
8787
m_event_client_read(event_client_read),
8888
m_event_conn_closed(event_conn_closed) {}
8989

90-
virtual THD *create_thd() {
90+
THD *create_thd() override {
9191
THD *thd = Channel_info::create_thd();
9292

9393
if (thd != NULL) {
@@ -97,8 +97,8 @@ class Channel_info_shared_mem : public Channel_info {
9797
return thd;
9898
}
9999

100-
virtual void send_error_and_close_channel(uint errorcode, int error,
101-
bool senderror) {
100+
void send_error_and_close_channel(uint errorcode, int error,
101+
bool senderror) override {
102102
Channel_info::send_error_and_close_channel(errorcode, error, senderror);
103103

104104
// Channel_info::send_error_and_close_channel will have closed

‎sql/field.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4258,7 +4258,7 @@ class Field_typed_array final : public Field_json {
42584258
}
42594259
void sql_type(String &str) const final;
42604260
void make_send_field(Send_field *field) const final;
4261-
void set_field_index(uint16 f_index) final override;
4261+
void set_field_index(uint16 f_index) final;
42624262
Field *get_conv_field();
42634263
};
42644264

‎sql/json_schema.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
#include "sql/json_schema.h"
2424

25-
#include "my_rapidjson_size_t.h" // IWYU pragma: keep
25+
#include <my_rapidjson_size_t.h> // IWYU pragma: keep
2626

2727
#include <assert.h>
2828
#include <rapidjson/document.h>

‎sql/log_event.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,9 @@ class Log_event {
802802
virtual bool write_data_body(Basic_ostream *) { return false; }
803803
#endif
804804

805-
Log_event_type get_type_code() const { return common_header->type_code; }
805+
virtual Log_event_type get_type_code() const {
806+
return common_header->type_code;
807+
}
806808

807809
/**
808810
Return true if the event has to be logged using SBR for DMLs.
@@ -1776,7 +1778,9 @@ class XA_prepare_log_event : public binary_log::XA_prepare_event,
17761778
xid = nullptr;
17771779
return;
17781780
}
1779-
Log_event_type get_type_code() { return binary_log::XA_PREPARE_LOG_EVENT; }
1781+
Log_event_type get_type_code() const override {
1782+
return binary_log::XA_PREPARE_LOG_EVENT;
1783+
}
17801784
size_t get_data_size() override {
17811785
return xid_bufs_size + my_xid.gtrid_length + my_xid.bqual_length;
17821786
}
@@ -1890,7 +1894,9 @@ class Stop_log_event : public binary_log::Stop_event, public Log_event {
18901894
}
18911895

18921896
~Stop_log_event() override = default;
1893-
Log_event_type get_type_code() { return binary_log::STOP_EVENT; }
1897+
Log_event_type get_type_code() const override {
1898+
return binary_log::STOP_EVENT;
1899+
}
18941900

18951901
private:
18961902
#if defined(MYSQL_SERVER)
@@ -2249,7 +2255,9 @@ class Unknown_log_event : public binary_log::Unknown_event, public Log_event {
22492255

22502256
~Unknown_log_event() override = default;
22512257
void print(FILE *file, PRINT_EVENT_INFO *print_event_info) const override;
2252-
Log_event_type get_type_code() { return binary_log::UNKNOWN_EVENT; }
2258+
Log_event_type get_type_code() const override {
2259+
return binary_log::UNKNOWN_EVENT;
2260+
}
22532261
};
22542262
#endif
22552263
char *str_to_hex(char *to, const char *from, size_t len);
@@ -2661,7 +2669,6 @@ class Rows_log_event : public virtual binary_log::Rows_event, public Log_event {
26612669

26622670
MY_BITMAP const *get_cols() const { return &m_cols; }
26632671
MY_BITMAP const *get_cols_ai() const { return &m_cols_ai; }
2664-
size_t get_width() const { return m_width; }
26652672
const Table_id &get_table_id() const { return m_table_id; }
26662673

26672674
#if defined(MYSQL_SERVER)
@@ -3873,7 +3880,7 @@ class Gtid_log_event : public binary_log::Gtid_event, public Log_event {
38733880
*/
38743881
rpl_sidno get_sidno(Sid_map *sid_map) { return sid_map->add_sid(sid); }
38753882
/// Return the GNO for this GTID.
3876-
rpl_gno get_gno() const { return spec.gtid.gno; }
3883+
rpl_gno get_gno() const override { return spec.gtid.gno; }
38773884

38783885
/// string holding the text "SET @@GLOBAL.GTID_NEXT = '"
38793886
static const char *SET_STRING_PREFIX;

0 commit comments

Comments
 (0)