fix(sem): fix crash routine pragma redefinition #1445
Conversation
error reporting This existed because of unhelpful assertions in legacy reports.
fix code that was clowning instead of actually using string formatting still not qutie right as there is _some_ flexibility with pragmas on definitions
saem
added
bug
There was a problem hiding this comment.
Given that the test is for an error message, I think it would make sense to move it to the errmsgs category, but that's only a soft suggestion.
There was a problem hiding this comment.
I debated that, I think error messages should be part of the feature/functionality they're defining in the error case, where errmsgs is for stuff we can't otherwise categorize, but maybe that's not as wise an idea as I think it is.
Thoughts?
There was a problem hiding this comment.
Hm, from the top of my head, I'd consider there being an error part of the feature, while a test for the exact message(s) feels like something separate from it.
Following that line of though, in this case, completing a forward-declaration with a header using different pragmas being an error would belong under lang_callable, while a test for the message itself would belong under errmsgs.
Given that there's only a single message format here, there's no point in having two separate tests, so I think the current test location is correct. For ease of later test navigation/lookup, what you could do is add a error_message label to the test, but that's just another (very) soft suggestion.
saem
changed the title
Thanks
Done |
Co-authored-by: zerbina <[email protected]>
saem
changed the title
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below:
|
Summary
The compiler no longer crashes when encountering a pragma redefinition
error on a routine. This was due to unnecessary asserts in error message
code
Details
the symbol for the definition, as they will be the same
%)instead of concatenation
Fixes #1444