Output an accurate error message when unimplemented math functions are called (globally)

[sjinno]

An accurate error message wasn't output when unimplemented math functions were called.

In this case (#1498),

#version 450

const int x = clamp(1, 1, 1);

void main() {}

would say error: const values must have an initializer when it should actually be saying

error: Not implemented: Clamp

Note: This change only applies to unimplemented math functions that are called globally (or should I say externally?).

Feedback on wording/how code should be written is very much appreciated! :)

[jimblandy]

(Taking review, since I'm mentoring @sjinno on this work.)

[jimblandy]

This looks good - thank you very much!

There are just a few things I'd like changed.

[jimblandy]

Even though DeclarationContext is private, and will not appear in documentation unless someone builds with --document-private-items, I think this should still be a /// doc comment, placed above the field, because that's what people are expecting to see.

[sjinno]

Okay!

[jimblandy]

This looks correct, but I think it might be clearer as a plain match result, with a guard expression on the Err case:

Err(err) if ctx.external => return Err(err),

It also deserves a little comment, something like:

Initializers for external declarations must be constants, so if solve_constant failed, that's an error the user should see. But if it's an initializer for a local variable, then failing to produce a constant just means we emit the expression as code; that is not an error.

[sjinno]

Thank you very much for the review! I'd like to use what you wrote as a comment. :)

[sjinno]

Currently maybe_constant assigns the returned value of sove_constant for both externally and internally declared math functions if implemented.

So, should it also be

Ok(res) if ctx.external => Some(res)

so that internally declared math expressions are left as is?

[JCapucho]

There is a problem with your current approach that makes valid shaders not pass

[JCapucho]

Implementation wise it looks good to me, I'll let @jimblandy handle the rest since he is mentoring you

[sjinno]

@JCapucho Thank you for your help!

[JCapucho]

Since the author hasn't touched this PR in a while, I've gone ahead and rebased it and cleaned it up a little, the code is essentially the same, only that now the constants are also folded for non const declarations (as was previously done), I reworded the comments a bit as was requested by jimb in his review and changed the error message to be a bit more helpful.

I've also cleaned the git history since it picked up many unneeded commits.

@jimblandy it still needs an approval from you, so if you could do a review it would be very helpful.

[jimblandy]

Looks good to me - thank you very much!