# Keyman Conventional Commit suggestions:
#
# - Link to a Sentry issue with git trailer:
# Fixes: _MODULE_-_ID_
# - Give credit to co-authors:
# Co-authored-by: _Name_ <_email_>
# - Use imperative, present tense ('attach' not 'attaches', 'attached' etc)
# - Don't include a period at the end of the title
# - Always include a blank line before trailers
# - More: https://github.com/keymanapp/keyman/wiki/Pull-Request-and-Commit-workflow-notes
Remove the `VERIFY_KEYBOARD_VERSION()` and
`VERIFY_KEYBOARD_VERSION_ret()` macros, and instead use
`VerifyKeyboardVersion()`. The DRYing out of this function call pattern
moved the return statement into a macro, which is an inappropriate way
to reduce repetition, because (a) it is hard for devs to spot function
exits, and (b) automated code analysis tools stumble on it.
Also means that we uncover more clearly the return mismatches between
`KMX_DWORD` and `KMX_BOOL` through the compiler source.
Fixes: #12047
The `AddWarning()` and `AddWarningBool()` macros just called
`ReportCompilerMessage()`. A side-effect of removing `AddWarningBool()`
is that the functions that called it always returned `TRUE` (and the
return value was never checked anyway), and so these functions have been
made `void`.
* Remove compiler message definitions from kmcmplib
* Add parameterization to compiler message structures
* Translate parameters for existing parameterized messages (except for
`ERROR_InvalidToken`, which requires a bigger refactor of
`GetXStringImpl()` and many friends)
* Add columnNumber to message structures (not yet used in kmc-kmn)
* Add filename to message structures (not yet used in kmcmplib)
* Rename `INFO_Info` to `INFO_MinimumCoreEngineVersion` and
`INFO_MinimumEngineVersion` to `INFO_MinimumWebEngineVersion`
Relates-to: #10866
This is definitely a yak shave, but should have payoff in terms of
cleaner and more descriptive error messages in the long run.
In the existing design, many (but not all) functions return a KMX_DWORD
error value, and that is passed on up the call stack to be reported as
an error elsewhere. This separation of error reporting from error site
makes it difficult to add parameters (and masks the actual error site in
analysis).
This commit starts to move the error reporting to the actual site of the
error, in `AddStore()`, `ProcessSystemStore()`, `ParseLine()`,
`AddCompilerVersionStore()`, `BuildVKDictionary()`, and
`ProcessBeginLine()`.
During analysis, identified a memory leak (missing `delete[] temp`) in
`process_if()`. Fixed on line 2716.
Many of the `process_*()` functions call `AddStore()`, which is now
responsible for reporting its own errors; these functions have not yet
been updated and so if `AddStore()` fails, the compiler will generate
two error messages -- one from `AddStore()` and a second one in the
value returned from the calling `process_*()` function.
The `VERIFY_KEYBOARD_VERSION` macro hides a `return` statement which
returned an error code on failure. This has been split into
`VERIFY_KEYBOARD_VERSION_ret` which causes the caller to returns the
error code, and the existing `VERIFY_KEYBOARD_VERSION` macro now causes
the caller to return `FALSE` on failure. This should be refactored to
move the `return` condition into the caller (removing the `return`
side-effect) in the future.
This is the same change as for `CheckForDuplicateGroup`.
`CheckForDuplicateStore` is one of a few places where parameterized
errors are returned. Moving the error reporting inside the function
makes it cleaner to refactor those parameterized errors later. (This
requires a similar change in `ProcessStoreLine`, also formatted and
rebraced per style guide.)
