Bump cel-spec + googleapi submodules #476
Merged
Conversation
This file contains 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
* add `compileAll` task * add IntelliJ-sync-task * bump jandex version to make it work with Java 21
snazy
force-pushed
the
bump-cel-spec-googleapis
branch
3 times, most recently
from
da8d5c9 to
a46e549
Compare
Bump cel-spec submodule to [v0.13.0](https://github.com/google/cel-spec/releases/tag/v0.13.0), googleapi submodule to [f2d78630d2c1d5e20041dfff963e093de9298e4d](googleapis/googleapis@f2d7863). This was necessary due to recent build errors caused by missing (old) artifacts for the conformance bazel build. The submodule bumps bring a bunch of new conformance tests as well, which unveiled that some functionality that was not present in CEL-Go when CEL-Java was created, is required by the CEl-Spec. Summary of changes: * Bugfix in `o.p.c.checker.Standard` that fixes a type resolution error for equals and not-equals functions, when both parameters need to be up-casted. * Changes in many types (in `o.p.c.common.types.*T` classes) to automatically convert between numeric and null types. * equal and compare with a null and a non-null type no longer fail, but return `False` * equal and compare between different numeric types no longer fail, but return "the right" result * this includes that numeric CEL map keys can be heterogenous, e.g. an `int` can be retrieved using an `uint` or `double` key * Fix retrieval of milliseconds from `Duration` - must only return the milliseconds within the second * Un-ignore a bunch of conformance tests that pass fine now A few conformance tests have been added to `InterpreterTest`, but the majority of tests is left in the conformance tests.
snazy
force-pushed
the
bump-cel-spec-googleapis
branch
2 times, most recently
from
a71269c to
5a815f7
Compare
snazy
commented
Nov 17, 2023
| @@ -58,6 +61,10 @@ configure<ProtobufExtension> { | |||
| // Download from repositories | |||
| artifact = "com.google.protobuf:protoc:${libs.versions.protobuf.get()}" | |||
| } | |||
| plugins { | |||
There was a problem hiding this comment.
The conformance-tests proto/gRPC definitions have been moved in the googleapis repo - finally separated. This includes a change in the package names in the related classes.
| @@ -162,13 +162,13 @@ Overloads.GreaterEqualsBytes, asList(Decls.Bytes, Decls.Bytes), Decls.Bool), | |||
| Decls.newFunction( | |||
| Operator.Equals.id, | |||
| Decls.newParameterizedOverload( | |||
| Overloads.Equals, asList(paramA, paramA), Decls.Bool, typeParamAList))); | |||
| Overloads.Equals, asList(paramA, paramB), Decls.Bool, typeParamABList))); | |||
There was a problem hiding this comment.
This fixes automatic up-casts for expressions like list(string) == list(double), which is translated a call to equals(list(string), list(double)), but before this change the (general) parameter type definition for A would have been memoized for the 2nd parameter as well, resulting in a "no such overload" error.
| @@ -299,8 +313,6 @@ public static Val timeGetSeconds(Duration duration) { | |||
| } | |||
|
|
|||
| public static Val timeGetMilliseconds(Duration duration) { | |||
| return IntT.intOf( | |||
| TimeUnit.SECONDS.toMillis(duration.getSeconds()) | |||
There was a problem hiding this comment.
This was according to the CEL-Go implementation, but the conformance-tests proved it wrong.
| @@ -282,6 +313,19 @@ public Val negate() { | |||
| } | |||
| } | |||
|
|
|||
| /** Add implements traits.Adder.Add. */ | |||
| @Override | |||
| public Val add(Val other) { | |||
There was a problem hiding this comment.
No change, only moved to the related function.
| @@ -228,6 +278,19 @@ public Val multiply(Val other) { | |||
| } | |||
| } | |||
|
|
|||
| /** Add implements traits.Adder.Add. */ | |||
| @Override | |||
| public Val add(Val other) { | |||
There was a problem hiding this comment.
No change, only moved to the related function.
dimas-b
approved these changes
Nov 17, 2023
| // the OTHER uint is > Integer.MAX_VALUE, so THIS int MUST be smaller | ||
| return IntNegOne; | ||
| } | ||
| return intOfCompare(Long.compareUnsigned(i, other.intValue())); |
There was a problem hiding this comment.
nit: At this point the values are within the [0; Integer.MAX_VALUE] range... Do we really need to use Long.compareUnsigned()?
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.
Bump cel-spec submodule to v0.13.0, googleapi submodule to f2d78630d2c1d5e20041dfff963e093de9298e4d.
This was necessary due to recent build errors caused by missing (old) artifacts for the conformance bazel build.
The submodule bumps bring a bunch of new conformance tests as well, which unveiled that some functionality that was not present in CEL-Go when CEL-Java was created, is required by the CEl-Spec.
Summary of changes:
o.p.c.checker.Standardthat fixes a type resolution error for equals and not-equals functions, when both parameters need to be up-casted.o.p.c.common.types.*Tclasses) to automatically convert between numeric and null types.Falseintcan be retrieved using anuintordoublekeyDuration- must only return the milliseconds within the secondA few conformance tests have been added to
InterpreterTest, but the majority of tests is left in the conformance tests.