Skip to content
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

Question: experimenting with Qt - how to cast away const for argv #592

Closed
vladimir-kraus opened this issue Aug 14, 2023 · 5 comments
Closed

Comments

@vladimir-kraus
Copy link

vladimir-kraus commented Aug 14, 2023

I am experimenting with combination of Qt, one of the most widely used C++ frameworks, and cppfront. I was curious how hard this could be. And I believe it can give cppfront some hard battle testing because Qt is very opinionated framework which is doing lots of thinks "a bit differently". In order to test it, I created this code to make the simplest possible QtWidgets application.

#include <QApplication>
#include <QWidget>

main: (args) -> int =
{
    a := QApplication(args.argc, args.argv as **char);
    w := QWidget();
    w.show();
    return a.exec();
}

It builds with cppfront and then with MSVC well and runs without any problems. But it fails to build with clang. It gives error error: static_assert failed due to requirement 'program_violates_type_safety_guarantee<char **, const char *const *>' "No safe 'as' cast available - please check your cast".

The cast is there because the required signature is QApplication(int&, char**) while args.argv is char const* const*. I tried static_cast but cppfront compiler told me it is not allowed. I believe there is a good memory-safety reason why this is not possible.

Is there any vanilla cppfront way of solving this issue and allowing instantiate QApplication object? Something that is simple and memory safe at the same?

And also - isn't the fact that it builds well with MSVC a problem? It seems that this constitutes a memory safety issue. IWould this be possible to catch and handle (compile erorr?) at cppfront level?

@vladimir-kraus vladimir-kraus changed the title Question: experimenting with Qt - how to const cast away args for constructing QApplication Question: experimenting with Qt - how to cast away const for argv Aug 14, 2023
@vladimir-kraus
Copy link
Author

Btw. this is the generated cpp file:



//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"



//=== Cpp2 type definitions and function declarations ===========================

#include <QApplication>
#include <QWidget>

#line 4 "main.cpp2"
[[nodiscard]] auto main(int const argc_, char const* const* const argv_) -> int;


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


#line 4 "main.cpp2"
[[nodiscard]] auto main(int const argc_, char const* const* const argv_) -> int
{
    auto args = cpp2::make_args(argc_, argv_); 
#line 6 "main.cpp2"
    auto app {QApplication(args.argc, cpp2::as_<char**>(args.argv))}; 
    auto w {QWidget()}; 
    CPP2_UFCS_0(show, std::move(w));
    return CPP2_UFCS_0(exec, std::move(app)); 
}

@hsutter
Copy link
Owner

hsutter commented Aug 14, 2023

Thanks, good question! (Edited to add:) Removing the const would be unsafe in general, that's why there's no such cast in Cpp2. But the issue you're pointing out is that I should be providing a way to not add const in the first place since I'm the one doing that.

It's good to have the default be const.

But the standard does allow the strings to be mutable, so we want to support that valid use case and provide a way to get at the mutable argv contents when needed. So I'll add a mutable_argv member for that... that way, the default nice name argv is still read-only which is the right default, but we can opt into getting a read-write mutable_argv when desired.

How does that sound?

@vladimir-kraus
Copy link
Author

@hsutter Thank you. Yes it solves the issue. But maybe another idea could be worthy to consider. I noticed that args are also a vector of string views. Which sounds actually as a very memory-safe thing to work with (as long as the underlying strings remain alive), the users can iterate over this collection or do whatever read-only operations they like. So maybe this vector<string_view> interface could be the safe recommended default way of working with args in pure cpp2 code while its member argv could remain the good old unsafe char** as required by the C++ standard and as is expected to be passed by some inherently unsafe third party code (such as QApplication constructor). So args.argv would remain there for compatibility but the new and preferred thing would be to use args as a vector of string views.

Pro: This would keep things simple to learn and explain to users. There would be no need for additional mutable_args member.

hsutter added a commit that referenced this issue Aug 16, 2023
The real default-const value is in the safe `args` themselves, and `argc` and `argv` should be their standard selves for compatibility. See #592 comment thread, thanks @vladimir-kraus !
@hsutter
Copy link
Owner

hsutter commented Aug 16, 2023

I think you've convinced me, that the real default-const value is in the safe args themselves, and that argc and argv should be their standard selves for compatibility. After the next commit your original code ought to work. Thanks!

@vladimir-kraus
Copy link
Author

Thank you. I think this design now draws clear and easy-to-communicate dividing line between the "old" style and the "new" style. Btw. the new style, which is basically an array of strings, is similar to what programmers may know from Java or C#.

zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
See discussion in hsutter#592 comment thread
zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
The real default-const value is in the safe `args` themselves, and `argc` and `argv` should be their standard selves for compatibility. See hsutter#592 comment thread, thanks @vladimir-kraus !
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

No branches or pull requests

2 participants