Handle variant conversions more accurately #388
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cirras
commented
May 23, 2025
delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/VariantUtils.java
Outdated
Show resolved
Hide resolved
delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationResolver.java
Outdated
Show resolved
Hide resolved
|
Oops. Repeated word in the commit message.
|
Cirras
force-pushed
the
variants_are_deeply_upsetting
branch
from
feee515 to
fdffacf
Compare
This one was a real mess. The initial implementation of variant conversions for invocation resolution was based on the FPC `is_better_candidate_single_variant` routine because their documentation indicated it was reverse-engineered from Delphi's behavior. Unfortunately, it was inaccurate in many ways. Some interesting problems in the previous implementation: - Comparisons of integer types were based on type identity, while Delphi bases these comparisons on signedness and value range. - All real types (as well as `Currency` and `Comp`) were preferred over `Extended`, while Delphi always prefers it over other real types (plus `Currency` and `Comp`, but only when the`Extended` type is larger). - `Currency` and `Comp` didn't conflict with `Single` or `Extended`, while in Delphi they will always conflict with real types of the same size or smaller. - Character types (`WideChar`, `AnsiChar`) parameters were considered valid targets for variant conversions, while in Delphi they are not. - Mixing-and-matching candidates between string, integer, boolean, and real types was allowed, while in Delphi such cases are ambiguous.
Cirras
force-pushed
the
variants_are_deeply_upsetting
branch
from
fdffacf to
6996d20
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one was a real mess.
The initial implementation of variant conversions for invocation resolution was based on the FPC
is_better_candidate_single_variantroutine because their documentation indicated it was reverse-engineered from Delphi's behavior. Unfortunately, it was inaccurate in many ways.Some interesting problems in the previous implementation:
CurrencyandComp) were preferred overExtended, while Delphi always prefers it over other real types (plusCurrencyandComp, but only when theExtendedtype is larger).CurrencyandCompdidn't conflict withSingleorExtended, while in Delphi they will always conflict with real types of the same size or smaller.WideChar,AnsiChar) parameters were considered valid targets for variant conversions, while in Delphi they are not.