Code annotations incompatible with mixed-case target

[disinvite]

If the target in your reccmp-project.yml uses lower-case chars, everything will work except parsing code annotations. The problem is that the code parser converts the module name to upper case, and so it cannot match the input target.

Historically, reccmp target names have been upper-case, starting with LEGO1 and ISLE, but we don't document this as a requirement. Using a mixed-case module in your annotation will raise the bad_recomp_marker warning in decomplint, but (IIRC) my intention was for processing to go through regardless.

We could patch this easily by converting the input target name to upper case, but this implies that targets have case-insensitive uniqueness in the YML. (i.e. Another thing to validate.)

#230 is another potential solution (apart from its own merits as an independent enhancement).

@pytest.mark.xfail(reason="TODO")
def test_mixed_case_module_name(parser):
    parser.read(
        dedent(
            """\
        // FUNCTION: Hello 0x1234
        void interesting_function() {
        }
        """
        )
    )

    # Preserve module case
    assert len(parser.functions) == 1
    assert parser.functions[0].module == "Hello"

    # Should alert to this
    assert len(parser.alerts) == 1
    assert parser.alerts[0].code == ParserError.BAD_DECOMP_MARKER

[madebr]

Nobody asked for it, but here are my 2c.

The parsers should be case sensitive.
If unknown targets are encountered during parsing, then warnings should be emitted.
These warnings can be configuring using gcc-style options: e.g. -Wunknown-target and -Wno-unknown-target.
Since these are warnings, and we don't (yet) have -Werror, parsing should just continue but ignore these targets.

Programmers generally don't want a mix of lower- and uppercase targets in their code.

[disinvite]

One problem is that we have many items that are all intertwined.

We have two possible inputs:

• A: Target name from reccmp-project.yml.
• B: Base name of the original binary.

Whichever one you provide (via the --target or --paths option), you get this internal value:

• C: Query for decomp annotation parser.

(#230 proposes to allow you to specify C explicitly.)

C has to match X, the identifier used in each code annotation. My intention was for C to match X case-insensitively. This flexibility is not actually there, hence this issue.

I agree that A should require case-sensitive match. This is analogous to a cmake/makefile target.

B may not match anything relevant to the project because the user can name their local copy of the file anything they want. We also do not call Path.resolve() before extracting the name. This means that on Windows (where paths are case-insensitive) we will use whatever value was provided. (This is probably worth its own issue 😶)

I'm not sure what to do about X that will feel correct to users. My instinct would be to consider this a typo, not two different targets built out of the same file:

// STUB: Isle 0x1000
void func_1000() {
}

// STUB: ISLE 0x2000
void func_2000() {
}

But if we now require that C is a case-sensitive match for X, we can't report either of these annotations as being "invalid" because they very well could be two different targets. We could validate X against the list of possible A values for the project, but this is not possible if we have only provided B via the --paths option.