Skip to content

Improve wording of intro and spec API of P3294 #172

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bernhardmgruber
Copy link

Hi! @andralex asked me to review is paper, but I only had time to read the Spec API section. Here are some wording improvements and feedback:

  • "The Spec API"
    • I have no meaningful association with the term "Spec", so the heading confuses me. I would rather name this "Extending the define_class API".
  • "because invoking it is an expression and we want to do it in contexts that want a declaration"
    • The paper does not explain why this is a problem.
  • "So a simple example of injecting a single member int named x is: ..."
    • I feel the paper is trying to make a point that the shown example is somehow problematic, but I am not sure what the critique is.
    • One possibility: define_class can only define an entire class, but not add to a started class that is already partially filled with declarations. If you want to make that point, please add a sentence .
  • "But that doesn’t work - in order to splice member, it needs to be a constant expression - and it’s not in this context."
    • I assume inject to be consteval function, so the returned value can be assigned to a constexpr variable, which the lambda can access without capturing:
consteval auto property(string_view name, meta::info type)
    -> void
{
    constexpr auto member = inject(data_member_spec{ // !!! constexpr variable
        .name=std::format("m_{}", name),
        .type=type
    });

    inject(function_member_spec{
        .name=std::format("get_{}", name),
        .body=^[](auto const& self) -> auto const& {
            return self.[:member:]; // !!! access without capture
        }
    });

    // ...
}
  • Regarding getting the class name into the lambda parameter list, what about that solution:
template <std::meta::info Class> // !!! take reflection of class
consteval auto property(string_view name, meta::info type) -> void;

struct Book {
    consteval {
        property<^Book>("author", ^std::string); // !!! pass reflection to containing class
        property<^Book>("title", ^std::string);
    }
};

which allows:

template <std::meta::info Class>
consteval auto property(string_view name, meta::info type) -> void {
    constexpr auto member = inject(data_member_spec{ // !!! inject() is consteval
        .name=std::format("m_{}", name),
        .type=type
    });
    
    inject(function_member_spec{
        .name=std::format("get_{}", name),
        .body=^[]([:Class:] const& self) -> auto const& { // !!! splice Class
            return self.[:member:];
        }
    });
    
    // setter
}

Which seems to provide a solution to the question. It's still cumbersome to pass in the scope to inject into. But if std::meta::current() were consteval as well (which it should), we could just do:

    inject(function_member_spec{
        .name=std::format("get_{}", name),
        .body=^[]([:std::meta::current():] const& self) -> auto const& {
            return self.[:member:];
        }
    });
  • Please point out why my approach would not work! Otherwise, I think the paper should be updated with the examples provided above. I can provide a PR for that if you like.
  • Btw, in all examples, the parameter self should probably be a pointer so it corresponds to this.
  • Discussion of Postfix Increment is missing for the Spec API. Could look like:
    inject(function_member_spec{
        .name="operator++",
        .body=^[]([:std::meta::current():]* const this_, int) -> auto& {
            ++(*this_);
            return *this_;
        }
    });
  • Disposition likely needs some rewriting with the above findings, since I think we can figure out how to do basic things. The argument of poor scaling with C++ syntactical constructs is very valid though and I think the main problem with the spec API. Token streams are just indefinitly forward compatible.

@bernhardmgruber bernhardmgruber changed the title Improve wording of intro and spec API Improve wording of intro and spec API of P3294 Jun 3, 2024
@brevzin brevzin force-pushed the master branch 2 times, most recently from d215d5d to d8dae46 Compare February 13, 2025 11:01
@brevzin brevzin force-pushed the master branch 4 times, most recently from c8f13e1 to a515803 Compare March 31, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant