Add check if type is complete to before checking sizeof type in prefer_pass_by_value check #276
Conversation
|
To help avoid failed builds, I think this should be a |
Rather than avoid failed builds, it actually fails with a more meaningful error message. The current formulation still fails because the use of |
|
I get that practically this needs to build so needs must, but as cpp2 is intended to be order independant, would a type being declared but not defined be an issue for end users to worry about still?
On 12 March 2023 22:20:55 Johel Ernesto Guerrero Peña ***@***.***> wrote:
To help avoid failed builds, I think this should be a requires check.
Or more like fail with a more meaningful error message.
The current formulation still fails because the use of sizeof, even if not evaluated due to the short-circuit, doesn't permit an incomplete type. See https://compiler-explorer.com/z/j9zs675YY.
—
Reply to this email directly, view it on GitHub<#276 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALUZQIU2JDUWMEYQFLTR63W3ZD4LANCNFSM6AAAAAAVYK5YPE>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
The error message seems shorter and clearer if You can see how the meaning of |
|
@JohelEGP Thank you for spotting that! What about this: https://compiler-explorer.com/z/9b7bcEf8c template<typename T, typename = void>
constexpr bool prefer_pass_by_value = false;
template<typename T>
constexpr bool prefer_pass_by_value<T, std::void_t<decltype(sizeof(T))>> =
sizeof(T) < 2*sizeof(void*)
&& std::is_trivially_copy_constructible_v<T>;Can you tell me why it might be an ODR violation? |
In current implementation of `prefer_pass_by_value` we check sizeof type. It fails in case that type is incomplete. This change adds a check if there is a possibility to check the sizeof type.
filipsajdak
force-pushed
the
fsajdak-in-arg-pass-check-type-completeness
branch
from
d894a91 to
6cd3450
Compare
|
@JohelEGP Just to clarify, the order of #270 (comment) should look like following: // declaration of classes (UDT)
struct X {
int i;
double d;
void fun();
double gun();
};
// declarations of free functions
void f(in<X>);
// implementations of class functions
void X::fun() {
// code
}
double X::gun() {
return i * d;
}
// implementations of free functions
void f(in<X>) { } |
|
My issue is that I have a project divided to several My #include <map>
namespace execspec {
using config_t = std::map<std::string, std::string>;
}
execspec: namespace = {
process_stage : type = {
public name: std::string;
public cmd: std::string;
}
spec: type = {
name: std::string;
description: std::string = "";
config: execspec::config_t = ();
stages: std::vector<execspec::process_stage> = ();
operator=: (out this, n: std::string) = {
name = n;
}
get_name: (this) -> forward _ = name;
get_description: (this) -> forward _ = description;
get_config: (this) -> forward _ = config;
get_stages: (this) -> forward _ = stages;
// skipping some code
}
}
debug: (inout o : std::ostream, spec : execspec::spec) -> forward std::ostream = {
o << "# (spec.get_name())$\n";
// skipping some code
return o;
}it generates #include <map>
namespace execspec {
using config_t = std::map<std::string, std::string>;
}
#include "cpp2util.h"
#line 7 "spec.h2"
namespace execspec {
class process_stage;
class spec;
};
#line 49 "spec.h2"
[[nodiscard]] auto debug(std::ostream& o, cpp2::in<execspec::spec> spec) -> std::ostream&;
I have an issue when I include #include "boost/ut.hpp"
using namespace boost::ut;
#include <sstream>
using namespace std::literals;
#include "execspec/spec.h2"
main: () -> int = {
execspec__spec : suite = :() = {
"create a spec"_test = :() = {
t := execspec::spec("spec name"s);
o : std::ostringstream = ();
o.debug(t);
expect(eq(o.str(), "# spec name\n\n"sv));
};
};
}it generates: #include "boost/ut.hpp"
using namespace boost::ut;
#include <sstream>
using namespace std::literals;
#include "execspec/spec.h"
#include "cpp2util.h"
#line 9 "spec_tests.cpp2"
[[nodiscard]] auto main() -> int;
//=== Cpp2 definitions ==========================================================
#include "execspec/spec.hpp"
#line 8 "spec_tests.cpp2"
[[nodiscard]] auto main() -> int{
suite execspec__spec {[]() -> void{
"create a spec"_test = []() -> void{
auto t {execspec::spec("spec name"s)};
std::ostringstream o {};
CPP2_UFCS(debug, o, t);
expect(eq(CPP2_UFCS_0(str, o), "# spec name\n\n"sv));
};
};
}And that fail with an error: With my current patch it compiles fine. |
@SebastianTroy eventually, it will not be a problem - it is just a matter of generating the output files in a specific order - example here: #276 (comment). The current issue is caused by the introduction of classes (which is an immature implementation). The issue is more problematic when you split your project into multiple |
|
Taking another look at this... I think that an ODR violation might be an issue here. Probably I will need to temporarily change the meaning of |
I wish I could give a methodical answer with proof and references. But I used might for a reason. The gist is usually that |
https://compiler-explorer.com/z/Pv69dWqvE. Making an exposition-only type that's complete earlier. This can only really save Cpp2 types. Incomplete Cpp1 types could be covered by a general statement like https://eel.is/c++draft/res.on.functions#2.5. |
In the current implementation of
prefer_pass_by_value, we check the size of the type. It fails in case that type is incomplete. This change adds a check if there is a possibility to check the size of the type.This might be a temporary solution but it can help avoid failed builds.