Missing inner array type dimensions errors do not include correct source locations

[EricRahm]

We currently have a test that checks that "Array dimensions can only be omitted for the outermost dimension", ConstraintsTest.test_error_on_missing_inner_array_size:

  def test_error_on_missing_inner_array_size(self):
    ir = _make_ir_from_emb("struct Foo:\n"
                           "  0 [+1]  UInt:8[][1]  one_byte\n")
    error_array = ir.module[0].type[0].structure.field[0].type.array_type
    self.assertEqual([[
        error.error(
            "m.emb",
            error_array.base_type.array_type.element_count.source_location,
            "Array dimensions can only be omitted for the outermost dimension.")
    ]], error.filter_errors(constraints.check_constraints(ir)))

This test passes as an error is generated and a source location is listed.

While working on a refactoring in #149 I noticed that the error itself is missing valid source locations: due to the nature of auto-generating fields as they're accessed error_array.base_type.array_type.element_count.source_location ends up yielding a default value which matches the default value generated by the compiler error.

We can reproduce this with the following:

$ cat test.emb
struct Foo:
    0 [+1]  UInt:8[][1]  one_byte
$ python3 ./embossc test.emb
test.emb:0:0: error: Array dimensions can only be omitted for the outermost dimension.

Note the error is at 0:0.

[EricRahm]

FYI @robrussell

[reventlov]

This does point to a larger hole in the unit tests for error messages: they should either check the actual numbers for the source locations (thorough, but slightly brittle) or at least check that the source locations are not defaulted.

due to the nature of auto-adding fields as they're accessed

Just a minor point: (in the current implementation) fields aren't added as they are accessed, but they do have default values if they are read without being accessed. ...element_count.HasField('source_location') should still return False here.

[EricRahm]

Just a minor point: (in the current implementation) fields aren't added as they are accessed, but they do have default values if they are read without being accessed. ...element_count.HasField('source_location') should still return False here.

Reworded to "auto-generating"