Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the C++ type DSL to also work for cpp/value, enabling non-string literals and structured DSL forms to resolve to C++ values.
Changes:
- Update reader parsing for
#cppso literals are passed directly tocpp/value(not string-wrapped). - Extend analyzer/codegen to resolve
cpp/valuefrom literals + DSL forms (including member / member-ptr handling). - Refresh and expand test coverage for literal values and member/member-ptr cases.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| compiler+runtime/third-party/cppinterop | Bumps cppinterop submodule to pick up interop support needed for new DSL behavior. |
| compiler+runtime/test/jank/reader-macro/cpp/real/pass-real.jank | Updates cpp/value usage to accept a real literal directly. |
| compiler+runtime/test/jank/reader-macro/cpp/integer/pass-integer.jank | Updates cpp/value usage to accept an integer literal directly. |
| compiler+runtime/test/jank/cpp/operator/star/pass-unary-reference.jank | Moves from string-based cpp/value lookup to DSL symbol form for globals. |
| compiler+runtime/test/jank/cpp/opaque-box/box/pass-reference.jank | Moves from string-based cpp/value lookup to DSL symbol form for globals. |
| compiler+runtime/test/jank/cpp/member/pass-access-base-template.jank | Uses DSL template form for function value lookup and switches to shorthand member access. |
| compiler+runtime/test/jank/cpp/literal-value/string/pass-escaped.jank | Adds coverage for escaped string literal handling via cpp/value. |
| compiler+runtime/test/jank/cpp/literal-value/real/pass-normal.jank | Adds coverage for real literals via cpp/value. |
| compiler+runtime/test/jank/cpp/literal-value/pass-string-literal.jank | Removes old string-literal test in favor of the new literal-value suite. |
| compiler+runtime/test/jank/cpp/literal-value/pass-static-variable.jank | Removes old static-variable string-expression test (replaced in member suite). |
| compiler+runtime/test/jank/cpp/literal-value/pass-npos.jank | Removes old npos string-expression test (coverage shifted with DSL approach). |
| compiler+runtime/test/jank/cpp/literal-value/pass-int-literal.jank | Removes old integer string-expression test (replaced by integer literal suite). |
| compiler+runtime/test/jank/cpp/literal-value/pass-function-template-address.jank | Removes old function-template-address string-expression test (replaced by DSL forms). |
| compiler+runtime/test/jank/cpp/literal-value/pass-function-call.jank | Removes old raw function-call string-expression test. |
| compiler+runtime/test/jank/cpp/literal-value/pass-function-address.jank | Removes old function-address string-expression test (replaced by DSL forms). |
| compiler+runtime/test/jank/cpp/literal-value/pass-enum.jank | Removes old enum string-expression test (DSL/member handling now used). |
| compiler+runtime/test/jank/cpp/literal-value/pass-cast.jank | Removes old cast string-expression test. |
| compiler+runtime/test/jank/cpp/literal-value/member/pass-static-variable.jank | Adds coverage for static member variable access via (:member ...) in cpp/value. |
| compiler+runtime/test/jank/cpp/literal-value/member/pass-not-copyable.jank | Adds coverage for non-copyable static member access via cpp/value DSL. |
| compiler+runtime/test/jank/cpp/literal-value/member/pass-nested-value.jank | Adds coverage for nested member lookup via chained (:member ...). |
| compiler+runtime/test/jank/cpp/literal-value/member/fail-type.jank | Adds negative coverage ensuring cpp/value rejects type-only members. |
| compiler+runtime/test/jank/cpp/literal-value/member-ptr/pass-int.jank | Adds coverage for data-member pointer values via (:member* ...). |
| compiler+runtime/test/jank/cpp/literal-value/member-ptr/pass-function.jank | Adds coverage for member-function pointer values via (:member* ...). |
| compiler+runtime/test/jank/cpp/literal-value/member-ptr/fail-static-function.jank | Adds negative coverage rejecting static member functions in (:member* ...). |
| compiler+runtime/test/jank/cpp/literal-value/integer/pass-normal.jank | Adds coverage for integer literals (incl. negative) via cpp/value. |
| compiler+runtime/test/jank/cpp/literal-value/fail-unknown-value.jank | Removes old unknown-value string-expression negative test. |
| compiler+runtime/test/jank/cpp/literal-value/fail-no-args.jank | Adds negative coverage for missing args to cpp/value. |
| compiler+runtime/test/jank/cpp/literal-value/fail-multiple-expressions.jank | Removes old multi-expression string guard test. |
| compiler+runtime/test/jank/cpp/literal-value/fail-invalid-syntax.jank | Removes old invalid-syntax string guard test. |
| compiler+runtime/test/jank/cpp/literal-value/boolean/pass-normal.jank | Adds coverage for boolean literals via cpp/value. |
| compiler+runtime/test/jank/cpp/literal-type/member-ptr/pass-valid.jank | Updates literal-type tests to use cpp/value DSL for member pointers. |
| compiler+runtime/test/jank/cpp/literal-type/member-ptr/pass-valid-base.jank | Updates literal-type tests to use cpp/value DSL for member pointers. |
| compiler+runtime/test/jank/cpp/literal-type/member-ptr/fail-mismatched-class.jank | Updates negative test to use cpp/value DSL for member pointers. |
| compiler+runtime/test/jank/cpp/literal-type/fail-no-args.jank | Adds negative coverage for missing args to cpp/type. |
| compiler+runtime/test/jank/cpp/call/not-callable/fail-integer.jank | Updates to call cpp/value with integer literal directly. |
| compiler+runtime/test/jank/cpp/call/member/pass-base-template.jank | Uses DSL template form for function value lookup. |
| compiler+runtime/test/bash/clojure-test-suite/clojure-test-suite | Bumps clojure-test-suite submodule. |
| compiler+runtime/src/jank/jank/nrepl/server/capture.jank | Rewrites stream handle capture to use DSL-based cpp/value + operators. |
| compiler+runtime/src/jank/clojure/core.jank | Uses DSL member access for std::numeric_limits<...>::min/max instead of string C++ snippets. |
| compiler+runtime/src/cpp/jank/read/parse.cpp | Changes #cpp parsing to wrap the literal form directly in (cpp/value <form>). |
| compiler+runtime/src/cpp/jank/codegen/processor.cpp | Ensures qualified names include template args and prefixes & for pointer-to-member values. |
| compiler+runtime/src/cpp/jank/analyze/processor.cpp | Implements analyze_cpp_dsl and extends cpp/value analysis to support literals + DSL values. |
| compiler+runtime/src/cpp/jank/analyze/cpp_util.cpp | Ensures templated function instantiation also instantiates return type when needed. |
| compiler+runtime/include/cpp/jank/analyze/processor.hpp | Exposes analyze_cpp_dsl and removes old analyze_cpp_literal plumbing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3889
to
+3895
| if(l->count() != 2) | ||
| { | ||
| if(kind == literal_kind::type) | ||
| { | ||
| return error::analyze_invalid_cpp_type("The first and only argument to 'cpp/type' must be " | ||
| "a string containing a C++ type.", | ||
| object_source(string_obj), | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(l, 1)); | ||
| } | ||
| return error::analyze_invalid_cpp_value( | ||
| "The first and only argument to 'cpp/value' must be a string containing a C++ type.", | ||
| object_source(string_obj), | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(l, 1)); | ||
| return error::analyze_invalid_cpp_type( | ||
| util::format("Exactly 1 argument is expected for 'cpp/value', but {} were provided.", | ||
| l->count() - 1), | ||
| object_source(l), | ||
| latest_expansion(macro_expansions)); |
Comment on lines
+3955
to
+3962
| if(res.expect_ok()->kind == expression_kind::cpp_type) | ||
| { | ||
| auto const literal_value{ cpp_util::resolve_literal_value(str) }; | ||
| if(literal_value.is_ok()) | ||
| { | ||
| auto const &result{ literal_value.expect_ok() }; | ||
| auto const source{ jtl::make_ref<expr::cpp_value>(position, | ||
| current_frame, | ||
| needs_box, | ||
| /* TODO: Is symbol needed? */ | ||
| try_object<obj::symbol>(l->first()), | ||
| Cpp::GetTypeFromScope(result.fn_scope), | ||
| result.fn_scope, | ||
| expr::cpp_value::value_kind::function) }; | ||
| auto const res{ | ||
| build_cpp_call(source, {}, {}, {}, current_frame, position, needs_box, macro_expansions) | ||
| }; | ||
| if(res.is_ok()) | ||
| { | ||
| llvm::cast<expr::cpp_call>(res.expect_ok().data)->function_code = result.function_code; | ||
| } | ||
| return res; | ||
| } | ||
|
|
||
| return error::analyze_invalid_cpp_value(literal_value.expect_err(), | ||
| object_source(string_obj), | ||
| latest_expansion(macro_expansions)) | ||
| ->add_usage(read::parse::reparse_nth(l, 1)); | ||
| return error::analyze_invalid_cpp_type( | ||
| util::format("A value was expected here, not a type. The type found is '{}'.", | ||
| cpp_util::get_qualified_type_name( | ||
| static_box_cast<expr::cpp_type>(res.expect_ok())->type)), | ||
| object_source(l), | ||
| latest_expansion(macro_expansions)); |
Comment on lines
+5334
to
+5361
|
|
||
| jtl::option<expr::cpp_value::value_kind> vk; | ||
| jtl::ptr<void> inst_type{ Cpp::GetTypeFromScope(inst_scope) }; | ||
| if(Cpp::IsVariable(inst_scope)) | ||
| { | ||
| vk = expr::cpp_value::value_kind::variable; | ||
| if(!Cpp::IsPointerType(inst_type)) | ||
| { | ||
| inst_type = Cpp::GetLValueReferenceType(inst_type); | ||
| } | ||
| } | ||
| else if(Cpp::IsEnumConstant(inst_scope)) | ||
| { | ||
| vk = expr::cpp_value::value_kind::enum_constant; | ||
| inst_type = Cpp::GetNonReferenceType(inst_type); | ||
| } | ||
| else if(Cpp::IsFunction(inst_scope)) | ||
| { | ||
| vk = expr::cpp_value::value_kind::function; | ||
| } | ||
|
|
||
| return jtl::make_ref<expr::cpp_value>(position, | ||
| current_frame, | ||
| false, | ||
| sym, | ||
| inst_type, | ||
| instantiated_scope.expect_ok(), | ||
| vk.unwrap()); |
Comment on lines
+3932
to
+3939
| false, | ||
| /* TODO: Is symbol needed? */ | ||
| try_object<obj::symbol>(l->first()), | ||
| Cpp::GetTypeFromScope(result.fn_scope), | ||
| result.fn_scope, | ||
| expr::cpp_value::value_kind::function) }; | ||
| auto const res{ | ||
| build_cpp_call(source, {}, {}, {}, current_frame, position, false, macro_expansions) |
| @@ -0,0 +1,2 @@ | |||
| (let* [_ ((cpp/type))] | |||
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This carries over the same DSL for types to also work with values.
Test plan