Checking for format arguments

[ygorpontelo]

This is just an idea, i tested in a branch in my fork and works like i somewhat expected.

Context: Sometimes we forget to pass arguments to the format function, this is remedied with the \<MISSING> being added to the string, however, a better aproach in my opinion would be to cause an error or at least a warning of some kind.

Possible solution: Leverage the contracts in c3 to make said check. With the experiment i made, it would only be a runtime error, but with #1161 it would be possible to make it into compile time. I would like to discuss more, if it even makes sense. My code is very simplistic too, so we would need to develop something better.

Experiment code: https://github.com/ygorpontelo/c3c/blob/contract-format/lib/std/io/formatter.c3#L356

[lerno]

It was deliberate not to make it a runtime error. Since it can't always be checked, this means it will sometimes be a runtime error and that's fairly fragile in places where fragility is very bad (e.g. reporting an error in an untested code path that has a bug, here broken output is preferable)

One thing would be to differentiate between compile time format strings and runtime format strings. It's a bit complicated, but that would be possible.

[ygorpontelo]

I see. I think if the format can be checked at compile time, then it must give an error. I don't see why this wouldn't be helpful in this scenario.

Now to the case that can't be checked at compile time, then just adding the \<MISSING> also seems fragile to me since there are formats that can return strings, so it's not always printed in the terminal. Maybe log a warning on those cases?

[lerno]

The correct behaviour is probably both to return an error on io::printf in the return value AND add I don't recall if that's the current behaviour. If the user chooses then they can detect it, for example through io::printf(...)!!, otherwise just getting it printed seems ok?

[ygorpontelo]

Looking here and printf does return a fault, but you can ignore that because of "@maydiscard" ?. What is the usefulness of maydiscard? Could we shoot ourselves in the foot?

The format from String doesn't return a fault, this would be very valuable i think, even not being a compile error. At least makes the programmer deal with the possible error.

[lerno]

There are multiple possible errors from printf, as it is running fprintf under the covers, and it must take into account error emitting to the output.

I personally don't think this is valuable but dangerous. You want static checks where possible yes, but you don't want runtime crashes due to formatting errors, it is a huge area for exploits and DOS attacks.

[ygorpontelo]

I can see checking with the contract could be a bad idea in this sense, as it would force a runtime error.

Using faults however could be a good compromise, they are not necessarily a runtime error unless you choose to panic. The printf already returns a fault, so other formats could do the same, like the format from string::new_format(). Perhaps it's even more important in these other ones since we are definitely interested in the value they return.

Having the compiler detect this at compile time would still be awesome, could be extended to checking inevitable faults given static values in the code.

I was a bit concerned about @maydiscard at first, but there are other ways to ignore faults as well. In the end, how something is implemented is what matters.

[lerno]

Let me take a real life example: on a java code base I worked on, it happened more than once that error logging crashed the system due to incorrect string formatting arguments / string in the error string.

Because these are hard-to-trigger code paths where it's important to get SOME information when they occur, using printf style became a no-no and we exclusively used the ugly-but-safe string concatenation.

I've seen the same thing repeating over and over. Format strings are very unsafe to use exactly because they're not handled gracefully when they fail. This is why for this particular case I think that it's better to generate broken text than to crash.

[ygorpontelo]

I'm not sure what you mean by crash in that context, does that include returning an optional fault? I don't consider this a crash. My original approach would cause a crash and i understand that it can lead to many problems, so let's not use it.

The printfn function already returns a fault, that we can ignore it by default, but still. What i proposed in the last comment is that other format functions also return a fault so that the programmer can handle better the cases where it fails. For example, if i try to use string::new_format and that generates a broken text, that is very difficult to debug, specially using splats.

The program may not have crashed, but now it has weird behaviours. In the case of printf, it's less problematic because you are not really using the string. And as i said, would be even cooler to have a compile time detection if possible.

[lerno]

Changing the interface of new_format to return an Optional would make it much more onerous to use on the off hand chance that someone passed the wrong formatting string / parameters to it. Most likely people would not like to handle this, and would then use !! which is a panic. So I'd say it's not really an option. The error string will hold information on how it was broken. This information can be improved, see how Go handles the different scenarios for example.

[ygorpontelo]

Can't say i'm convinced, but i could be wrong about it. If nothing else i can close this for now, probably better to focus on more important issues.

[lerno]

For compile time formatting checks it's either adding an attribute or changing all the functions to take unevaluated expressions, in which case they need to be macros. At least right now. I think this could be revisited yes.

[ygorpontelo]

Sounds good to me.