Declare implicit array types #335
Conversation
This adds a `Declare_Itypes` call to `Do_Type_Reference` to make sure we don’t miss out on declaring Itypes when they are referenced. This fixes some issues with types being missing from the symbol table, however we do get some additional debug output in some cases now because we don’t correctly handle all kinds of Itypes yet (this isn’t a new issue really, we’re just making an existing bug visible that would’ve otherwise caused a crash down the line if we actually tried to use the types we’re referencing).
NlightNFotis
force-pushed
the
key_not_in_map_issue
branch
from
ab29fb6 to
896710c
Compare
|
@chrisr-diffblue @hannes-steffenhagen-diffblue Can I get a re-review on this one please? |
| Occurs: 8 times | ||
| Calling function: Process_Pragma_Declaration | ||
| Error message: Unsupported pragma: Obsolescent | ||
| Nkind: N_Pragma |
There was a problem hiding this comment.
Most of the golden results changes appear to be just things moving around for some weird reason. Very few actual changes.
There was a problem hiding this comment.
That looks like a move, but actually its because "Do_Type_Reference" went from 7 -> 8 times...
There was a problem hiding this comment.
@chrisr-diffblue Yes, I believe this is because some of the nodes that we now handle were not handled before. This means that some nodes in these projects now propagate, and they might result into some previously unreported errors showing up.
There was a problem hiding this comment.
@chrisr-diffblue Keep in mind that the changes made in this PR cause gnat2goto to not crash in some circumstances where it would’ve crashed before, so it’s producing more output and potentially some additional missing but unrelated features as well.
| Nkind: N_Defining_Operator_Symbol | ||
| -- | ||
| Occurs: 1 times | ||
| Calling function: Do_Itype_Integer_Subtype |
There was a problem hiding this comment.
Another one where we get an extra occurence
| (if Ekind (E) = E_Access_Subtype then Etype (E) else E); | ||
| Type_Id : constant Symbol_Id := Intern (Type_Name); | ||
| begin | ||
| Declare_Itype (E); |
There was a problem hiding this comment.
This is the important bit; We now just proactively declare (possible) Itypes whenever we see them (Declare_Itype will take non-Itypes and just ignore them). This way, at least theoretically, we should always have types in the symbol table as soon as we reference them.
| @@ -1,2 +1,25 @@ | |||
| Standard_Output from gnat2goto test: | |||
There was a problem hiding this comment.
What's going on with these two tests? This looks like the tests are now producing missing feature reports which they were not before.... Were this test and procedure_pointer both passing before this change?
There was a problem hiding this comment.
It’s basically just Itypes that we never supported, but we just “happened” to pass these tests without missing feature reports or crashes because we don’t actually use these types anywhere in the resulting goto program.
I.e. as a side effect of some of the changes we’ve made in this PR some things that didn’t work before now visibly don’t work.
There was a problem hiding this comment.
OK. There's recently been some debate on #327 about whether we should or should not add "missing feature" reports to expected outputs - I'm not sure we came to a conclusion. In this case, it's probably better to accept it in the expected output and keep the test passing, rather than turn a passing test into an xfail.
hannes-steffenhagen-diffblue
left a comment
hannes-steffenhagen-diffblue
left a comment
There was a problem hiding this comment.
I think this is mostly OK?
| ---------------------------- | ||
|
|
||
| function Do_Itype_Array_Type (E : Entity_Id) return Irep is | ||
| Var : constant Node_Id := |
There was a problem hiding this comment.
This should get a better name than Var. I don’t know, something like Type_Of_Underlyign_Array or something of the sort?
4 participants
This PR adds support for declaring Itypes (implicit types) for array declarations.