Can/should cpp2 prevent someone from passing a numeric literal to a function accepting a char?

[gregmarr]

I saw a bug report on an open source repo (names changed to protect the innocent) where someone thought that a function took two ints when it actually took an int and a char. The function used the char for padding, and they were surprised when they ended up with null characters in their output. They figured it out pretty quickly on their own, but I wondered if cpp2 could warn about this, or if there would be too much noise.

void format(int count, char padding);

main: () = {
    format(0, 0);
}

Could this be a warning like "Warning: passing an integer literal to a function that takes a character." I'm thinking this is similar to a warning about passing an integer literal to a function that takes a pointer. Is there an existing cpp1 compiler warning for this? A quick search didn't find one.

We DO get a cpp1 compiler warning for this:

void format(int count, char padding);

main: () = {
    ch := 0;
    format(0, ch);
}

[filipsajdak]

If you want to disable implicit cast you can use concepts (even in cpp1) - not perfect but works.

void format(std::same_as<int> auto count, std::same_as<char> auto padding); // no more implicit casts

[filipsajdak]

Our issue is that cppfront has no information about the function signature that comes from cpp1. By proposing wrapping up with

cpp2::call(format, 0, 0);

I am assuming it is done by cppfront in the same way that we do with CPP2_UFCS. So, it could be automatic (cppfront will do packing for you, and the checking will be on the cpp1 side where the compiler knows the function signatures).

[gregmarr]

Our issue is that cppfront has no information about the function signature that comes from cpp1.

So not cleanly at this time, but maybe in the future if there is a cppfront that has that information.

[filipsajdak]

For numeric literals, we can use a wrapper for arguments:

void fun(int value, char padding);

fun(Arg<123>(), Arg<'-'>()); // this works
fun(Arg<321>(), Arg<0>()); // this triggers static assert

Working prototype here: https://godbolt.org/z/WYa47GTex

Wrapping can be done by cppfront.

[DyXel]

I am all for those safety features, but what if I want to emit clean-cpp and run away with my code? It's becoming harder and harder to even reason about the lowered code, to the point where locally I have it run via clang-format since some errors just don't make sense / align with the emitted Cpp1.

[gregmarr]

Ha, I just spent far too much time trying to figure out what "c a stable" meant. :) When I finally figured out it was "cast able".....

[hsutter]

Thanks for the suggestion! Ah, Cpp1 implicit narrowing fun 😉

Cppfront doesn't do function call name lookup and overload resolution, so we don't have the information even for all-Cpp2 function calls. We leave that to the Cpp1 compiler.

However, Cpp2 does already help where it can, in particular that all initialization is lowered to Cpp1 { } initialization because that disallows such narrowing. For example, this already works:

struct X {
    X(char) { }
};

main: () = {
    x1: X = 'a'; // ok
    x2: X = 420; // Cpp1 error - lowers to 'X x2{420};' which disallows narrowing
}

I'm not sure we can reasonably do more than that right now though.

[gregmarr]

However, Cpp2 does already help where it can, in particular that all initialization is lowered to Cpp1 { } initialization because that disallows such narrowing. For example, this already works:

@hsutter Yes, range checking works with integer literals, and type checking works with variables, but type checking doesn't work with integer literals because the rules effectively say that the integer literal is interpreted as the type of parameter.

The lack of checking also happens with local const variables initialized with integer literals because of constant propagation,

    // error due to type checking: 
    // error: non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list [-Wc++11-narrowing]
    // warning: implicit conversion loses integer precision: 'int' to 'char' [-Wimplicit-int-conversion]
    int x = 0;
    char c1 = x;

    // error due to range checking
    // error: constant expression evaluates to 420 which cannot be narrowed to type 'char' [-Wc++11-narrowing]
    char c2 = 420;

    // ok because only range checking is done
    char c3 = 0; 

    // ok because the value is known at compile type, and only range checking is done.
    const int xconst = 0;
    char c4 = xconst;

This means that x2: X = 0; works just fine, which is exactly the issue here. The user wrote an integer literal to initialize a character variable that was actually being used as a character, not a number.

I'm not sure we can reasonably do more than that right now though.

Just checking, did you see @filipsajdak 's work here? Does that fall under "can reasonably do"? (I'm perfectly okay with "that's not reasonable to do at this time".)

[filipsajdak]

The proper way is to pack it with lambda call similarly to ufcs with types checking. I will make a proposal for that.

[filipsajdak]

@hsutter, I did a small investigation into what is possible. The working prototype is here: https://godbolt.org/z/aed3j6nve

TLDR: we can achieve the following code

void fun(int value, char padding);
void gen(auto value, auto padding);

CALL(fun, 123, ' '); // works
// CALL(fun, 345, 0); // this does not compile - CORRECT!
CALL(gen, 567, 0); // works
// CALL(fun, 'a', ' '); // this does not compile - the current solution requires an exact match for the types (even if there is a valid conversion from char to int)

If you are interested in such a solution, I can spend more time to make it work also for arguments that can be safely cast.

[hsutter]

Thanks for looking into this. I appreciate the suggestion but I worry it's adding too much to the code gen (and another inline function call) for probably not enough value? And that it requires an exact match, because I suspect that may get noisy (implicit conversions are often undesirable, but they're also often desirable).

If we did it, and it worked well enough to pass regression when enabled (i.e., the exact-match requirement isn't too noisy), then I'd probably want it to be off by default and add an opt-in switch. But most of all I think I'd prefer to look at it later (sorry!).

[gregmarr]

Yeah, I don't think an exact match requirement is desirable, and neither is an indirection on every function call that is passed an integer literal. I was only thinking that it would be good to prevent "integer literal" from matching "character literal", and it doesn't look like that's clean at this point, as cpp2 doesn't have enough information. It's not a highly motivated requirement. I don't know that I've ever run into it before this time. I just saw the user's error and thought that it might fit in with the type safety idea.

Thanks for looking into it.