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.)
`CheckForDuplicateGroup` 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, simple change in `ProcessGroupLine`.)
`AddCompileError` would return FALSE only for 'user interruption' (not
used since Developer 17) or for fatal errors. After analysis, it turns
out that this error value was not used anywhere and removing it
simplifies the error management logic, making code paths much easier to
follow.
The return value was used in v17.0 and earlier in the macro for
`AddCompileWarning`, effectively only relevant for 'user interruption'
(`CERR_Break`); this was an ugly code pattern as it meant that any
`AddCompileWarning` could exit a function with failure -- though as
noted above `CERR_Break` was never emitted in v17.0.
Renames kmcmplib error codes to match those in kmc-kmn, and updates the
bitmasks accordingly. Adds a KmnCompilerMessages namespace so a trivial
enum can be declared without risk of clashes with other ERROR_ or
similar messages.
These two codes are not true error or message codes, but are used as
return values extensively throughout kmcmplib. The codes are never
passed to message callbacks. As such, I have renamed them to reflect
usage, but opted not to change their values as there would be some risk
in doing so (e.g. use of `!v` vs `v == CERR_None`).
# 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
# 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
* Removes redundant calls
* Uses C++ classes on the interface side
* Removes global callbacks from the Javascript side
In preparation for cleanup of compiler messages
While it does not cause any problems to have a index() store that is
longer than its corresponding any() store, it often indicates a mistake,
as the trailing characters in the store are ignored. Thus, adding as a
hint (which can be disabled via message suppression) rather than as a
warning or error.
Example code:
store(abc) 'abc'
store(defg) 'defg'
any(abc) + 'x' > index(defg, 1) c generates HINT_IndexStoreLong
Fixes: #10666
# 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
# 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
# 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
