Skip to content

Commit

Permalink
Auto-enable file/line/function info on contracts violations + usabili…
Browse files Browse the repository at this point in the history
…ty improvements

Thanks @MikeShah for your YouTube video here: https://youtu.be/ZslemeHsOjk?si=qPoNdJLxBoSgFh6b . I'm treating the video as a "looking over your shoulder using Cpp2" mini-UX study to make a better experience for folks coming to the syntax the first time...

Diagnose attempt to declare two things at once, e.g., `x,y : int = 5;`

Diagnose attempt to for-loop over two ranges, e.g., `for x,y do ..`

Automatically use <source_location>, including for file/line/function information on contracts checks so when using a capable compiler you get nice bounds/other violation messages out of the box that point directly to the offending source line

Remove the -add-source-info flag as no longer needed, now <source_location> is now reliable and automated (modulo a workaround in cpp2util.h for an MSVC bug where importing source_location via a module doesn't work correctly)
  • Loading branch information
hsutter committed Jul 13, 2024
1 parent a252d50 commit 8504e86
Show file tree
Hide file tree
Showing 26 changed files with 92 additions and 59 deletions.
26 changes: 19 additions & 7 deletions include/cpp2util.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@

// If this implementation doesn't support source_location yet, disable it
#include <version>
#if !defined(_MSC_VER) && !defined(__cpp_lib_source_location)
#undef CPP2_USE_SOURCE_LOCATION

#undef CPP2_USE_SOURCE_LOCATION
#if defined(__cpp_lib_source_location)
#define CPP2_USE_SOURCE_LOCATION Yes
#endif

// If the user requested making the entire C++ standard library available
Expand Down Expand Up @@ -553,15 +555,17 @@ auto pointer_eq(T const* a, T const* b) {
//

#ifdef CPP2_USE_SOURCE_LOCATION
#define CPP2_SOURCE_LOCATION_PARAM , std::source_location where
#define CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT , std::source_location where = std::source_location::current()
#define CPP2_SOURCE_LOCATION_PARAM_SOLO std::source_location where
#define CPP2_SOURCE_LOCATION_PARAM , [[maybe_unused]] std::source_location where
#define CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT , [[maybe_unused]] std::source_location where = std::source_location::current()
#define CPP2_SOURCE_LOCATION_PARAM_SOLO [[maybe_unused]] std::source_location where
#define CPP2_SOURCE_LOCATION_ARG , where
#define CPP2_SOURCE_LOCATION_VALUE (cpp2::to_string(where.file_name()) + "(" + cpp2::to_string(where.line()) + ") " + where.function_name())
#else
#define CPP2_SOURCE_LOCATION_PARAM
#define CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT
#define CPP2_SOURCE_LOCATION_PARAM_SOLO
#define CPP2_SOURCE_LOCATION_ARG
#define CPP2_SOURCE_LOCATION_VALUE std::string("")
#endif

// For C++23: make this std::string_view and drop the macro
Expand Down Expand Up @@ -1520,8 +1524,16 @@ inline constexpr auto as() -> auto
return cpp2::to_string(CPP2_FORWARD(x));
}

// Work around MSVC modules bugs: source_location doesn't work correctly if imported via a module
#if defined(_MSC_VER) && defined(CPP2_IMPORT_STD)
#define CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT_AS
#define CPP2_SOURCE_LOCATION_ARG_AS
#else
#define CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT_AS CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT
#define CPP2_SOURCE_LOCATION_ARG_AS CPP2_SOURCE_LOCATION_ARG
#endif
template< typename C >
auto as(auto&& x CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
auto as(auto&& x CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT_AS) -> decltype(auto)
// This "requires" list may need to be tweaked further. The idea is to have
// this function used for all the cases it's supposed to cover, but not
// hide user-supplied extensions (such as the ones later in this file for
Expand Down Expand Up @@ -1555,7 +1567,7 @@ auto as(auto&& x CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
const C c = static_cast<C>(CPP2_FORWARD(x));
type_safety.enforce( // precondition check: must be round-trippable => not lossy
static_cast<CPP2_TYPEOF(x)>(c) == x && (c < C{}) == (x < CPP2_TYPEOF(x){}),
"dynamic lossy narrowing conversion attempt detected" CPP2_SOURCE_LOCATION_ARG
"dynamic lossy narrowing conversion attempt detected" CPP2_SOURCE_LOCATION_ARG_AS
);
return CPP2_COPY(c);
}
Expand Down
12 changes: 10 additions & 2 deletions regression-tests/mixed-lifetime-safety-and-null-contracts.cpp2
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,17 @@ try_pointer_stuff: () = {
// to show -n
}

call_my_framework: (msg: * const char) = {
auto call_my_framework(const char* msg CPP2_SOURCE_LOCATION_PARAM) {
std::cout
<< "sending error to my framework... ["
<< msg << "]\n";
<< msg
<< "]\n";
auto loc = CPP2_SOURCE_LOCATION_VALUE;
if (!loc.empty()) {
std::cout
<< "from source location: "
<< loc
<< "]\n";

This comment has been minimized.

Copy link
@gregmarr

gregmarr Jul 15, 2024

Contributor

@hsutter You have a ] here without a corresponding [.

This comment has been minimized.

Copy link
@hsutter

hsutter Jul 15, 2024

Author Owner

Oops, fixing. Thanks!

This comment has been minimized.

Copy link
@hsutter

hsutter Jul 15, 2024

Author Owner

Fixed: 2cd1aeb

}
exit(0);
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
mixed-bounds-check.cpp2(9) int main(): 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 @@
Bounds safety violation
mixed-bounds-safety-with-assert.cpp2(11) void print_subrange(const auto:89&, cpp2::impl::in<int>, cpp2::impl::in<int>) [with auto:89 = std::vector<int>; cpp2::impl::in<int> = const int]: Bounds safety violation
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 | template<typename T>
2100 | {
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | class c_raii {
2137 | // Speculative: RAII wrapping for the C standard library
| ^
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 | template<typename T>
2100 | {
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | class c_raii {
2137 | // Speculative: RAII wrapping for the C standard library
| ^
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 | template<typename T>
2100 | {
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | class c_raii {
2137 | // Speculative: RAII wrapping for the C standard library
| ^
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 | template<typename T>
2100 | {
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | class c_raii {
2137 | // Speculative: RAII wrapping for the C standard library
| ^
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 | template<typename T>
2100 | {
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | class c_raii {
2137 | // Speculative: RAII wrapping for the C standard library
| ^
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 +1 @@
Contract violation: fill: value must contain at least count elements
mixed-initialization-safety-3-contract-violation.cpp2(25) void fill(cpp2::impl::out<std::__cxx11::basic_string<char> >, cpp2::impl::in<std::__cxx11::basic_string<char> >, cpp2::impl::in<int>): Contract violation: fill: value must contain at least count elements
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
sending error to my framework... [dynamic null dereference attempt detected]
from source location: mixed-lifetime-safety-and-null-contracts.cpp2(17) void try_pointer_stuff()]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::expected has an unexpected value
pure2-assert-expected-not-null.cpp2(15) int bad_expected_access(): Null safety violation: std::expected has an unexpected value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::optional does not contain a value
pure2-assert-optional-not-null.cpp2(14) int bad_optional_access(): Null safety violation: std::optional does not contain a value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::shared_ptr is empty
pure2-assert-shared-ptr-not-null.cpp2(15) int bad_shared_ptr_access(): Null safety violation: std::shared_ptr is empty
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::unique_ptr is empty
pure2-assert-unique-ptr-not-null.cpp2(15) int bad_unique_ptr_access(): Null safety violation: std::unique_ptr is empty
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,23 @@
auto null_from_cpp1() -> int* { return nullptr; }

auto try_pointer_stuff() -> void;
#line 20 "mixed-lifetime-safety-and-null-contracts.cpp2"

auto call_my_framework(const char* msg CPP2_SOURCE_LOCATION_PARAM) {
std::cout
<< "sending error to my framework... ["
<< msg
<< "]\n";
auto loc = CPP2_SOURCE_LOCATION_VALUE;
if (!loc.empty()) {
std::cout
<< "from source location: "
<< loc
<< "]\n";
}
exit(0);
}

#line 21 "mixed-lifetime-safety-and-null-contracts.cpp2"
auto call_my_framework(char const* msg) -> void;

//=== Cpp2 function definitions =================================================

Expand All @@ -46,11 +60,3 @@ auto try_pointer_stuff() -> void{
// to show -n
}

#line 21 "mixed-lifetime-safety-and-null-contracts.cpp2"
auto call_my_framework(char const* msg) -> void{
std::cout
<< "sending error to my framework... ["
<< msg << "]\n";
exit(0);
}

Original file line number Diff line number Diff line change
@@ -1 +1 @@
Bounds safety violation: out of bounds access attempt detected - attempted access at index 5, [min,max] range is [0,4]
mixed-bounds-check.cpp2(9) int __cdecl main(void): 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 @@
Bounds safety violation
mixed-bounds-safety-with-assert.cpp2(11) void __cdecl print_subrange<class std::vector<int,class std::allocator<int> >>(const class std::vector<int,class std::allocator<int> > &,const int,const int): Bounds safety violation
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Contract violation: fill: value must contain at least count elements
mixed-initialization-safety-3-contract-violation.cpp2(25) void __cdecl fill(class cpp2::impl::out<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > >,const class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > &,const int): Contract violation: fill: value must contain at least count elements
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
sending error to my framework... [dynamic null dereference attempt detected]
from source location: mixed-lifetime-safety-and-null-contracts.cpp2(17) void __cdecl try_pointer_stuff(void)]
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::expected has an unexpected value
pure2-assert-expected-not-null.cpp2(15) int __cdecl bad_expected_access(void): Null safety violation: std::expected has an unexpected value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::optional does not contain a value
pure2-assert-optional-not-null.cpp2(14) int __cdecl bad_optional_access(void): Null safety violation: std::optional does not contain a value
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::shared_ptr is empty
pure2-assert-shared-ptr-not-null.cpp2(15) int __cdecl bad_shared_ptr_access(void): Null safety violation: std::shared_ptr is empty
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Null safety violation: std::unique_ptr is empty
pure2-assert-unique-ptr-not-null.cpp2(15) int __cdecl bad_unique_ptr_access(void): Null safety violation: std::unique_ptr is empty
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
pure2-statement-parse-error.cpp2...
pure2-statement-parse-error.cpp2(3,9): error: invalid statement encountered inside a compound-statement (at 'b')
pure2-statement-parse-error.cpp2(3,5): error: invalid statement encountered inside a compound-statement (at 'int')

2 changes: 1 addition & 1 deletion regression-tests/test-results/version
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

cppfront compiler v0.7.0 Build 9712:1020
cppfront compiler v0.7.1 Build 9713:1156
Copyright(c) Herb Sutter All rights reserved

SPDX-License-Identifier: CC-BY-NC-ND-4.0
Expand Down
2 changes: 1 addition & 1 deletion source/build.info
Original file line number Diff line number Diff line change
@@ -1 +1 @@
"9712:1020"
"9713:1156"
17 changes: 17 additions & 0 deletions source/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -6960,6 +6960,9 @@ class parser
expression_statement_node::current_expression_statements.push_back(n.get());
auto guard = finally([&]{ expression_statement_node::current_expression_statements.pop_back(); });

// Remember current position, in case this isn't a valid expression-statement
auto start_pos = pos;

if (!(n->expr = expression(true, true))) {
return {};
}
Expand All @@ -6978,6 +6981,7 @@ class parser
// it doesn't destabilize any regression tests
)
{
pos = start_pos; // backtrack
return {};
}
if (
Expand Down Expand Up @@ -7248,6 +7252,10 @@ class parser
return {};
}

if (curr().type() == lexeme::Comma) {
error("iterating over multiple ranges at once is not currently supported");
}

if (!handle_optional_next_clause()) { return {}; }

if (
Expand Down Expand Up @@ -7671,6 +7679,15 @@ class parser
}

else {
if (
curr().type() == lexeme::Identifier
&& peek(1)
&& peek(1)->type() == lexeme::Comma
)
{
next();
error("declaring multiple names at once is not currently supported");
}
return {};
}
}
Expand Down
12 changes: 0 additions & 12 deletions source/to_cpp1.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,6 @@ static cmdline_processor::register_flag cmd_safe_comparisons(
[]{ flag_safe_comparisons = false; }
);

static auto flag_use_source_location = false;
static cmdline_processor::register_flag cmd_enable_source_info(
2,
"add-source-info",
"Enable source_location information for contract checks",
[]{ flag_use_source_location = true; }
);

static auto flag_cpp1_filename = std::string{};
static cmdline_processor::register_flag cmd_cpp1_filename(
8,
Expand Down Expand Up @@ -1309,10 +1301,6 @@ class cppfront
printer.print_extra( "#define " + cpp1_FILENAME+"_CPP2" + "\n\n" );
}

if (flag_use_source_location) {
printer.print_extra( "#define CPP2_USE_SOURCE_LOCATION Yes\n" );
}

if (flag_include_std) {
printer.print_extra( "#define CPP2_INCLUDE_STD Yes\n" );
}
Expand Down
2 changes: 1 addition & 1 deletion source/version.info
Original file line number Diff line number Diff line change
@@ -1 +1 @@
"v0.7.0"
"v0.7.1"

1 comment on commit 8504e86

@MikeShah
Copy link

Choose a reason for hiding this comment

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

You are most welcome! :)

Please sign in to comment.