Re: [TCLCORE] TIP 720: Updated Tcl Bytecode Opcodes CFV (NOTE: NO DELAY)

[<apn...@ya...>]

Donal wrote:

 

> In general, this was on the verge of not being TIPped at all; it's only there really because of the impact on the TDK compiler/tbcload. Because of this, I'm pushing it through the process faster than normal.

 

Yes, I understand the borderline need for a TIP. My request for time was not so much about the TIP process as the conclusion from a previous discussion in the online meet that any substantial implementation changes should be reviewed by a second pair of eyes before acceptance (with the full understanding that very few folks will have the required byte code competence!). With the absolute greatest respect for your technical smarts, it’s a slippery slope if we selectively ignore that requirement.

 

> If MSVC can't deprecate an enum element…

 

Right, that’s what I was suggesting.

 

> I wish we could use <stdint.h> (and <stdbool.h> as well).

 

Can’t we? I thought that was part of C99 which is a requirement for 9.1?

 

> Re the range checks, I guess we'll need to fix the type of unsigned operands (as part of the Tcl_Size project). The bytecode compiler is now matching the usage, but the usage in TEBC is itself wrong. (I shudder to think what code that makes a stack frame with that many local variable entries would look like, and it will certainly take quite a while to compile.) This particular branch isn't meant to touch LOCAL meaningfully, but LVT indices are definitely intended to be positive (with -1 being "no LVT entry") so the assumption that they're able to be handled as unsigned int within TEBC is likely a good one; the -1 should never result in an issue of opcodes that use LVT entries. 

 

I’m really not worried about LVT indices, it just happened to be the first (and only) range check that showed up in the diffs. The bigger issue is that these range checks appear in lots of places and any of them could have similar repercussions. I feel changing the limit to UINT_MAX from INT_MAX has limited value and not worth the risk of fiddling with TEBCResume to *selectively* split the signed and unsigned use cases. And as discussed ad naseum in the Tcl_Size discussion, modifying types from signed->unsigned is not to be taken lightly.

 

At the end of the day, I think everyone is supportive of the TIP. My post was regarding (a) affording a little more time for review before merge, and (b) reconsidering the change from INT_MAX to UINT_MAX as not worth the risk.

 

/Ashok