Skip to content

Commit

Permalink
Disable some clang conversion warnings in cpp2util.h and cppfront i…
Browse files Browse the repository at this point in the history
…tself (#1212)

* Disable clang conversion warnings when compiling with -Wconversion

cppfront and cpp2util.h use signed integer types for indices and container sizes so disable signed-to-unsigned conversion warnings.

cppfront also uses implicit conversions from string literal to bool for:
`assert(!"message")`
so disable those warnings too.

* Update regression tests caused by line number changes in `cpp2util.h`

* Removed dependency on `!"string literal"`

* Address `.size()` narrowing warnings

Using the answer we recommend for everyone else, so model the right behavior here

---------

Co-authored-by: Herb Sutter <herb.sutter@gmail.com>
  • Loading branch information
bluetarpmedia and hsutter authored Aug 9, 2024
1 parent c618ed5 commit 658d307
Show file tree
Hide file tree
Showing 25 changed files with 89 additions and 69 deletions.
11 changes: 11 additions & 0 deletions include/cpp2util.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@
#include <vector>
#endif

// cpp2util.h uses signed integer types for indices and container sizes
// so disable clang signed-to-unsigned conversion warnings in this header.
#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wsign-conversion"
#endif

//-----------------------------------------------------------------------
//
Expand Down Expand Up @@ -2833,4 +2839,9 @@ using cpp2::cpp2_new;
#define CPP2_REQUIRES_(...) requires (__VA_ARGS__)
#endif

// Restore clang signed-to-unsigned conversion warnings
#ifdef __clang__
#pragma clang diagnostic pop
#endif

#endif // CPP2_CPP2UTIL_H
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(964) decltype(auto) cpp2::impl::assert_in_bounds(auto &&, std::source_location) [arg = 5, x:auto = std::vector<int>]: Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
../../../include/cpp2util.h(970) decltype(auto) cpp2::impl::assert_in_bounds(auto &&, std::source_location) [arg = 5, x:auto = std::vector<int>]: Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(776) : Bounds safety violation
../../../include/cpp2util.h(782) : Bounds safety violation
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(776) : Contract violation: fill: value must contain at least count elements
../../../include/cpp2util.h(782) : Contract violation: fill: value must contain at least count elements
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
sending error to my framework... [dynamic null dereference attempt detected]
from source location: ../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = int *&]
from source location: ../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = int *&]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::expected<int, bool>]: Null safety violation: std::expected has an unexpected value
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::expected<int, bool>]: Null safety violation: std::expected has an unexpected value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::optional<int>]: Null safety violation: std::optional does not contain a value
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::optional<int>]: Null safety violation: std::optional does not contain a value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::shared_ptr<int>]: Null safety violation: std::shared_ptr is empty
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::shared_ptr<int>]: Null safety violation: std::shared_ptr is empty
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::unique_ptr<int>]: Null safety violation: std::unique_ptr is empty
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::unique_ptr<int>]: Null safety violation: std::unique_ptr is empty
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(964) decltype(auto) cpp2::impl::assert_in_bounds(auto &&, std::source_location) [arg = 5, x:auto = std::vector<int>]: Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
../../../include/cpp2util.h(970) decltype(auto) cpp2::impl::assert_in_bounds(auto &&, std::source_location) [arg = 5, x:auto = std::vector<int>]: Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(776) : Bounds safety violation
../../../include/cpp2util.h(782) : Bounds safety violation
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(776) : Contract violation: fill: value must contain at least count elements
../../../include/cpp2util.h(782) : Contract violation: fill: value must contain at least count elements
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
sending error to my framework... [dynamic null dereference attempt detected]
from source location: ../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = int *&]
from source location: ../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = int *&]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::optional<int>]: Null safety violation: std::optional does not contain a value
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::optional<int>]: Null safety violation: std::optional does not contain a value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::shared_ptr<int>]: Null safety violation: std::shared_ptr is empty
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::shared_ptr<int>]: Null safety violation: std::shared_ptr is empty
Original file line number Diff line number Diff line change
@@ -1 +1 @@
../../../include/cpp2util.h(855) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::unique_ptr<int>]: Null safety violation: std::unique_ptr is empty
../../../include/cpp2util.h(861) decltype(auto) cpp2::impl::assert_not_null(auto &&, std::source_location) [arg:auto = std::unique_ptr<int>]: Null safety violation: std::unique_ptr is empty
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
In file included from mixed-bugfix-for-ufcs-non-local.cpp:6:
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:13:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:13:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:31:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:31:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:33:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:33:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
In file included from mixed-bugfix-for-ufcs-non-local.cpp:6:
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:13:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:13:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:31:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:31:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:33:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:33:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 | constexpr auto is( X const& x ) -> bool
2100 | requires (std::is_same_v<X,std::any> && !std::is_same_v<T,std::any> && !std::is_same_v<T,empty>)
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 |
2137 | { return std::any_cast<T>( x ); }
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pure2-assert-expected-not-null.cpp2(7): error C2143: syntax error: missing ';' b
pure2-assert-expected-not-null.cpp2(7): error C2143: syntax error: missing ';' before '}'
pure2-assert-expected-not-null.cpp2(9): error C2065: 'ex': undeclared identifier
pure2-assert-expected-not-null.cpp2(9): error C2672: 'cpp2::impl::assert_not_null': no matching overloaded function found
D:\a\cppfront\cppfront\include\cpp2util.h(855): note: could be 'decltype(auto) cpp2::impl::assert_not_null(_T0 &&,std::source_location)'
D:\a\cppfront\cppfront\include\cpp2util.h(861): note: could be 'decltype(auto) cpp2::impl::assert_not_null(_T0 &&,std::source_location)'
pure2-assert-expected-not-null.cpp2(14): error C2039: 'expected': is not a member of 'std'
predefined C++ types (compiler internal)(347): note: see declaration of 'std'
pure2-assert-expected-not-null.cpp2(14): error C2062: type 'int' unexpected
Expand All @@ -19,4 +19,4 @@ pure2-assert-expected-not-null.cpp2(14): note: while trying to match the argumen
pure2-assert-expected-not-null.cpp2(14): error C2143: syntax error: missing ';' before '}'
pure2-assert-expected-not-null.cpp2(15): error C2065: 'ex': undeclared identifier
pure2-assert-expected-not-null.cpp2(15): error C2672: 'cpp2::impl::assert_not_null': no matching overloaded function found
D:\a\cppfront\cppfront\include\cpp2util.h(855): note: could be 'decltype(auto) cpp2::impl::assert_not_null(_T0 &&,std::source_location)'
D:\a\cppfront\cppfront\include\cpp2util.h(861): note: could be 'decltype(auto) cpp2::impl::assert_not_null(_T0 &&,std::source_location)'
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,26 @@ Running tests_10_escapes:
08_y: OK regex: foo(\h)bar parsed_regex: foo(\h)bar str: foo bar result_expr: $1 expected_results
09_y: OK regex: (\H)(\h) parsed_regex: (\H)(\h) str: foo bar result_expr: $1-$2 expected_results o-
10_y: OK regex: (\h)(\H) parsed_regex: (\h)(\H) str: foo bar result_expr: $1-$2 expected_results -b
11_y: OK regex: foo(\v+)bar parsed_regex: foo(\v+)bar str: foo
11_y: OK regex: foo(\v+)bar parsed_regex: foo(\v+)bar str: foo


bar result_expr: $1 expected_results


bar result_expr: $1 expected_results

12_y: OK regex: (\V+)(\v) parsed_regex: (\V+)(\v) str: foo


bar result_expr: $1-$2 expected_results foo-
13_y: OK regex: (\v+)(\V) parsed_regex: (\v+)(\V) str: foo


12_y: OK regex: (\V+)(\v) parsed_regex: (\V+)(\v) str: foo


bar result_expr: $1-$2 expected_results foo-
13_y: OK regex: (\v+)(\V) parsed_regex: (\v+)(\V) str: foo


bar result_expr: $1-$2 expected_results

bar result_expr: $1-$2 expected_results


-b
14_y: OK regex: foo(\v)bar parsed_regex: foo(\v)bar str: foobar result_expr: $1 expected_results
15_y: OK regex: (\V)(\v) parsed_regex: (\V)(\v) str: foobar result_expr: $1-$2 expected_results o-
14_y: OK regex: foo(\v)bar parsed_regex: foo(\v)bar str: foobar result_expr: $1 expected_results
15_y: OK regex: (\V)(\v) parsed_regex: (\V)(\v) str: foobar result_expr: $1-$2 expected_results o-
16_y: OK regex: (\v)(\V) parsed_regex: (\v)(\V) str: foobar result_expr: $1-$2 expected_results -b
17_y: OK regex: foo\t\n\r\f\a\ebar parsed_regex: foo\t\n\r\f\a\ebar str: foo
bar result_expr: $& expected_results foo
Expand Down
11 changes: 10 additions & 1 deletion source/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif

// Disable some clang conversion warnings:
// cppfront uses signed integer types for indices and container sizes.
// Note: We don't pop the diagnostic because we want them disabled in the
// entire cppfront translation unit.
#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wsign-conversion"
#endif

#include "cpp2util.h"


Expand Down Expand Up @@ -97,7 +106,7 @@ struct source_line
break;case category::cpp1: return "/* 1 */ ";
break;case category::cpp2: return "/* 2 */ ";
break;case category::rawstring: return "/* R */ ";
break;default: assert(!"illegal category"); abort();
break;default: assert(false && "ICE: illegal category"); abort();
}
}
};
Expand Down
6 changes: 3 additions & 3 deletions source/cppfront.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ auto main(
auto total = count.cpp1_lines + count.cpp2_lines;
auto total_lines = print_with_thousands(total);
out << " Cpp1 "
<< std::right << std::setw(total_lines.size())
<< std::right << std::setw(unsafe_narrow<int>(total_lines.size()))
<< print_with_thousands(count.cpp1_lines) << " line" << (count.cpp1_lines != 1 ? "s" : "");
out << "\n Cpp2 "
<< std::right << std::setw(total_lines.size())
<< std::right << std::setw(unsafe_narrow<int>(total_lines.size()))
<< print_with_thousands(count.cpp2_lines) << " line" << (count.cpp2_lines != 1 ? "s" : "");
if (total > 0) {
out << " (";
Expand Down Expand Up @@ -145,7 +145,7 @@ auto main(
for (auto [elapsed, name] : sorted_timers) {
std::cout
<< "\n "
<< std::right << std::setw(total_time.size())
<< std::right << std::setw(unsafe_narrow<int>(total_time.size()))
<< print_with_thousands(elapsed) << " ms" << " in " << name;
}
}
Expand Down
Loading

0 comments on commit 658d307

Please sign in to comment.