-
-
Notifications
You must be signed in to change notification settings - Fork 114
Implement support for typed catch clauses #610
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.
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
Open
jianlingzhong
wants to merge
8
commits into
jank-lang:main
Choose a base branch
from
jianlingzhong:catch_type
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
34f5c16
Implement support for typed catch clauses
jianlingzhong 3b5f662
ignore false positive clang-tidy warnings
jianlingzhong 2b18b30
rename duplicated testing functions
jianlingzhong 6892c5a
update catch syntax in test.jank; remove option in catch bodies
jianlingzhong a1854d2
update catch syntax in test.jank
jianlingzhong 6e0df99
Consolidate and enhance try/catch test suite
jianlingzhong f5a1aa2
Fix clang-tidy issue and 'Jank'
jianlingzhong 6115179
Add TODO full error handling for duplicated catch types
jianlingzhong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2259,7 +2259,7 @@ namespace jank::analyze | |
|
|
||
| /* All bindings in a letfn appear simultaneously and may be mutually recursive. | ||
| * This makes creating a letfn locals frame a bit more involved than let, where locals | ||
| * are introduced left-to-right. For example, each binding in (letfn [(a [] b) (b [] a)]) | ||
| * are introduced left-to-right. For example, each binding in (letfn [(a [] b) (b [] a)]) | ||
| * requires the other to be in scope in order to be analyzed. | ||
| * | ||
| * We tackle this in two steps. First, we create empty local bindings for all names. | ||
|
|
@@ -2616,7 +2616,6 @@ namespace jank::analyze | |
| auto try_frame(jtl::make_ref<local_frame>(local_frame::frame_type::try_, current_frame)); | ||
| /* We introduce a new frame so that we can register the sym as a local. | ||
| * It holds the exception value which was caught. */ | ||
| auto catch_frame(jtl::make_ref<local_frame>(local_frame::frame_type::catch_, current_frame)); | ||
| auto finally_frame(jtl::make_ref<local_frame>(local_frame::frame_type::finally, current_frame)); | ||
| auto ret{ jtl::make_ref<expr::try_>(position, try_frame, true, jtl::make_ref<expr::do_>()) }; | ||
|
|
||
|
|
@@ -2701,69 +2700,124 @@ namespace jank::analyze | |
| object_source(item), | ||
| latest_expansion(macro_expansions)); | ||
| } | ||
| if(has_catch) | ||
| { | ||
| /* TODO: Note where the other catch is. */ | ||
| return error::analyze_invalid_try("Only one 'catch' form may be supplied.", | ||
| object_source(item), | ||
| latest_expansion(macro_expansions)); | ||
| } | ||
| has_catch = true; | ||
|
|
||
| /* Verify we have (catch <sym> ...) */ | ||
| /* Verify we have (catch cpp/type <sym> ...) */ | ||
| auto const catch_list(runtime::list(item)); | ||
| auto const catch_body_size(catch_list->count()); | ||
| if(catch_body_size == 1) | ||
| if(auto const catch_body_size(catch_list->count()); catch_body_size < 2) | ||
| { | ||
| return error::analyze_invalid_try( | ||
| "A symbol is required after 'catch', which is used as the binding to " | ||
| "hold the exception value.", | ||
| "Each 'catch' form requires the type of exception to catch and a symbol for the " | ||
| "name of the exception value.", | ||
| object_source(item), | ||
| latest_expansion(macro_expansions)); | ||
| } | ||
| auto catch_it(catch_list->data.rest()); | ||
| auto const catch_type_form(catch_it.first().unwrap()); | ||
| catch_it = catch_it.rest(); | ||
| auto const catch_sym_form(catch_it.first().unwrap()); | ||
| auto const catch_type(analyze(catch_type_form, current_frame, position, fn_ctx, true)); | ||
| if(catch_type.is_err()) | ||
| { | ||
| return error::analyze_invalid_try(catch_type.expect_err()->message, | ||
| object_source(item), | ||
| error::note{ | ||
| "An exception type is required before this form.", | ||
| object_source(catch_sym_form), | ||
| }, | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(item, 1)); | ||
| } | ||
|
|
||
| if(catch_type.expect_ok()->kind != expression_kind::cpp_type) | ||
| { | ||
| return error::analyze_invalid_try("Exception is not a type.", | ||
| object_source(item), | ||
| error::note{ | ||
| "An exception type is required before this form.", | ||
| object_source(catch_sym_form), | ||
| }, | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(item, 1)); | ||
| } | ||
|
|
||
| auto const sym_obj(catch_list->data.rest().first().unwrap()); | ||
| if(sym_obj->type != runtime::object_type::symbol) | ||
| if(catch_sym_form->type != runtime::object_type::symbol) | ||
| { | ||
| return error::analyze_invalid_try( | ||
| "A symbol required after 'catch', which is used as the binding to " | ||
| "A symbol is required after 'catch', which is used as the binding to " | ||
| "hold the exception value.", | ||
| object_source(item), | ||
| error::note{ | ||
| "A symbol is required before this form.", | ||
| object_source(sym_obj), | ||
| object_source(catch_sym_form), | ||
| }, | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(item, 1)); | ||
| ->add_usage(read::parse::reparse_nth(item, 2)); | ||
| } | ||
| auto const catch_sym(runtime::expect_object<runtime::obj::symbol>(catch_sym_form)); | ||
| if(!catch_sym->get_namespace().empty()) | ||
| { | ||
| return error::analyze_invalid_try( | ||
| "The binding symbol in 'catch' must be unqualified.", | ||
| object_source(item), | ||
| latest_expansion(macro_expansions)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want a reparse on this, too. |
||
| } | ||
| auto const catch_type_ref(static_ref_cast<expr::cpp_type>(catch_type.expect_ok())); | ||
| /* If we're catching a C++ class/struct by value, we want to promote it to a reference | ||
| * to avoid object slicing and to enable polymorphism. | ||
| * However, we must NOT promote types in the jank::runtime namespace (like object_ref), | ||
| * as these are smart pointers expected to be passed by value in the runtime. */ | ||
| if(!Cpp::IsPointerType(catch_type_ref->type) | ||
| && !Cpp::IsReferenceType(catch_type_ref->type) | ||
| && !Cpp::IsBuiltin(catch_type_ref->type)) | ||
| { | ||
| /* Check if this type is in the jank::runtime namespace */ | ||
| auto const type_scope{ Cpp::GetScopeFromType(catch_type_ref->type) }; | ||
| auto const type_name{ cpp_util::get_qualified_name(type_scope) }; | ||
| bool const is_jank_runtime_type{ type_name.find("jank::runtime::") == 0 }; | ||
|
|
||
| auto const sym(runtime::expect_object<runtime::obj::symbol>(sym_obj)); | ||
| if(!sym->get_namespace().empty()) | ||
| if(!is_jank_runtime_type) | ||
| { | ||
| catch_type_ref->type = Cpp::GetLValueReferenceType(catch_type_ref->type); | ||
| } | ||
| } | ||
|
|
||
| /* Check for duplicate catch types. */ | ||
| /*TODO Add full error handling for duplicated catch types: | ||
| * Add a new error kind and notes pointing to the duplicated types*/ | ||
| for(auto const &existing_catch : ret->catch_bodies) | ||
| { | ||
| return error::analyze_invalid_try("The symbol after 'catch' must be unqualified.", | ||
| object_source(sym_obj), | ||
| latest_expansion(macro_expansions)); | ||
| if(existing_catch.type.data == catch_type_ref->type.data) | ||
| { | ||
| return error::analyze_invalid_try("Each catch form must specify a unique type.", | ||
| object_source(item), | ||
| latest_expansion(macro_expansions)); | ||
| } | ||
| } | ||
|
|
||
| catch_frame->locals.emplace(sym, local_binding{ sym, sym->name, none, catch_frame }); | ||
| auto catch_frame( | ||
| jtl::make_ref<local_frame>(local_frame::frame_type::catch_, current_frame)); | ||
| catch_frame->locals.emplace(catch_sym, | ||
| local_binding{ catch_sym, | ||
| catch_sym->name, | ||
| none, | ||
| catch_frame, | ||
| true, | ||
| false, | ||
| false, | ||
| catch_type_ref->type }); | ||
|
|
||
| /* Now we just turn the body into a do block and have the do analyzer handle the rest. */ | ||
| auto const do_list( | ||
| catch_list->data.rest().rest().conj(make_box<runtime::obj::symbol>("do"))); | ||
| auto const do_list(catch_it.rest().conj(make_box<runtime::obj::symbol>("do"))); | ||
| auto const do_res(analyze(make_box(do_list), catch_frame, position, fn_ctx, true)); | ||
| if(do_res.is_err()) | ||
| { | ||
| return do_res.expect_err(); | ||
| } | ||
|
|
||
| /* TODO: Read this from the catch form. */ | ||
| static auto const object_ref_type{ cpp_util::resolve_literal_type( | ||
| "jank::runtime::oref<jank::runtime::object>") | ||
| .expect_ok() }; | ||
|
|
||
| ret->catch_body = expr::catch_{ sym, | ||
| object_ref_type, | ||
| static_ref_cast<expr::do_>(do_res.expect_ok()) }; | ||
| do_res.expect_ok()->frame = catch_frame; | ||
| ret->catch_bodies.emplace_back(catch_sym, | ||
| catch_type_ref->type, | ||
| static_ref_cast<expr::do_>(do_res.expect_ok())); | ||
| } | ||
| break; | ||
| case try_expression_type::finally_: | ||
|
|
@@ -2797,10 +2851,6 @@ namespace jank::analyze | |
|
|
||
| ret->body->frame = try_frame; | ||
| ret->body->propagate_position(position); | ||
| if(ret->catch_body.is_some()) | ||
| { | ||
| ret->catch_body.unwrap().body->frame = catch_frame; | ||
| } | ||
| if(ret->finally_body.is_some()) | ||
| { | ||
| ret->finally_body.unwrap()->frame = finally_frame; | ||
|
|
@@ -2861,12 +2911,12 @@ namespace jank::analyze | |
| /* Eval the literal to resolve exprs such as quotes. */ | ||
| auto const pre_eval_expr( | ||
| jtl::make_ref<expr::vector>(position, current_frame, true, std::move(exprs), o->meta)); | ||
| auto const o(evaluate::eval(pre_eval_expr)); | ||
| auto const oref(evaluate::eval(pre_eval_expr)); | ||
|
|
||
| /* TODO: Order lifted constants. Use sub constants during codegen. */ | ||
| current_frame->lift_constant(o); | ||
| current_frame->lift_constant(oref); | ||
|
|
||
| return jtl::make_ref<expr::primitive_literal>(position, current_frame, true, o); | ||
| return jtl::make_ref<expr::primitive_literal>(position, current_frame, true, oref); | ||
| } | ||
|
|
||
| return jtl::make_ref<expr::vector>(position, current_frame, true, std::move(exprs), o->meta); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error for this (which you didn't change in this PR) is also missing an 'is'.