Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change constexpr to hardcoded number to avoid out of range error #389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Change constexpr to hardcoded number to avoid out of range error
PiperOrigin-RevId: 623840664
  • Loading branch information
FHIR Team authored and Copybara-Service committed Apr 18, 2024
commit 22574f43a27d15264dd2cd4990cf2e5fb152708a
5 changes: 3 additions & 2 deletions bazel/antlr4_cc.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Build rules to create C++ code from an Antlr4 grammar."""

load("@rules_java//java:defs.bzl", "java_binary")

def antlr4_cc_lexer(name, src, namespaces = None, imports = None, deps = None, lib_import = None):
"""Generates the C++ source corresponding to an antlr4 lexer definition.

Expand All @@ -25,8 +27,7 @@ def antlr4_cc_lexer(name, src, namespaces = None, imports = None, deps = None, l
"%sLexer.h" % base_file_prefix,
"%sLexer.cpp" % base_file_prefix,
]

native.java_binary(
java_binary(
name = "antlr_tool",
jvm_flags = ["-Xmx256m"],
main_class = "org.antlr.v4.Tool",
Expand Down
8 changes: 4 additions & 4 deletions bazel/protogen.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Rules for generating Protos from FHIR definitions"""

load("@rules_java//java:defs.bzl", "java_binary", "java_test")

R4_PACKAGE_DEP = "@com_google_fhir//spec:fhir_r4"
PROTO_GENERATOR = "@com_google_fhir//java/com/google/fhir/protogen:ProtoGenerator"
PROFILE_GENERATOR = "@com_google_fhir//java/com/google/fhir/protogen:ProfileGenerator"
Expand Down Expand Up @@ -151,8 +152,7 @@ def gen_fhir_protos(
"-Dgolden_dir=" + src_dir,
"-Xmx4096M",
]

native.java_test(
java_test(
name = "GeneratedProtoTest_" + name,
size = "medium",
srcs = ["//external:GeneratedProtoTest.java"],
Expand Down Expand Up @@ -294,7 +294,7 @@ def _get_tar_for_pkg(pkg):
return pkg + "_package.tgz"

def _proto_generator_with_runtime_deps_on_existing_protos(name, golden_java_protos_rules):
native.java_binary(
java_binary(
name = name,
main_class = "com.google.fhir.protogen.ProtoGeneratorMain",
runtime_deps = golden_java_protos_rules +
Expand Down
1 change: 1 addition & 0 deletions cc/google/fhir/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ cc_library(
":util",
"//cc/google/fhir/json:fhir_json",
"//cc/google/fhir/status",
"//cc/google/fhir/status:statusor",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
Expand Down
68 changes: 34 additions & 34 deletions cc/google/fhir/error_reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ class ErrorHandler {
// * field_path: Path to the field where the error occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirFatal(const absl::Status& status,
std::string_view element_path,
std::string_view field_path) = 0;
absl::string_view element_path,
absl::string_view field_path) = 0;

// Handles an "Error" issue encountered.
// This should be used when user data violates a precondition. For instance,
Expand All @@ -105,9 +105,9 @@ class ErrorHandler {
// (including index into repeated fields).
// * field_path: Path to the field where the error occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirError(std::string_view msg,
std::string_view element_path,
std::string_view field_path) = 0;
virtual absl::Status HandleFhirError(absl::string_view msg,
absl::string_view element_path,
absl::string_view field_path) = 0;

// Handles a "Warning" issue encountered.
// This should be used when user data violates a precondition at "Warning"
Expand All @@ -122,9 +122,9 @@ class ErrorHandler {
// (including index into repeated fields).
// * field_path: Path to the field where the warning occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirWarning(std::string_view msg,
std::string_view element_path,
std::string_view field_path) = 0;
virtual absl::Status HandleFhirWarning(absl::string_view msg,
absl::string_view element_path,
absl::string_view field_path) = 0;

// Handles a "Fatal" issue encountered during FHIRPath evaluation.
// This indicates a process encountered a code error (e.g., a function call
Expand All @@ -142,9 +142,9 @@ class ErrorHandler {
// * field_path: Path to the field where the error occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirPathFatal(const absl::Status& status,
std::string_view expression,
std::string_view element_path,
std::string_view field_path) = 0;
absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) = 0;

// Handles an "Error" issue encountered during FHIRPath evaluation.
// This indicates an Error-level FHIRPath requirement that was not met by a
Expand All @@ -157,9 +157,9 @@ class ErrorHandler {
// (including index into repeated fields).
// * field_path: Path to the field where the error occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirPathError(std::string_view expression,
std::string_view element_path,
std::string_view field_path) = 0;
virtual absl::Status HandleFhirPathError(absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) = 0;

// Handles a "Warning" issue encountered during FHIRPath evaluation.
// This indicates a Warning-level FHIRPath requirement that was not met by a
Expand All @@ -172,9 +172,9 @@ class ErrorHandler {
// (including index into repeated fields).
// * field_path: Path to the field where the error occurred. This will be
// identical to `element_path`, but without the field index.
virtual absl::Status HandleFhirPathWarning(std::string_view expression,
std::string_view element_path,
std::string_view field_path) = 0;
virtual absl::Status HandleFhirPathWarning(absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) = 0;

// Returns true if any issues have been reported at WARNING level.
virtual bool HasWarnings() const = 0;
Expand Down Expand Up @@ -343,18 +343,18 @@ class FailFastErrorHandler : public ErrorHandler {
bool HasFatals() const override { return false; }

absl::Status HandleFhirFatal(const absl::Status& status,
std::string_view element_path,
std::string_view field_path) override {
absl::string_view element_path,
absl::string_view field_path) override {
return element_path.empty()
? status
: absl::Status(
status.code(),
absl::StrCat(status.message(), " at ", element_path));
}

absl::Status HandleFhirError(std::string_view msg,
std::string_view element_path,
std::string_view field_path) override {
absl::Status HandleFhirError(absl::string_view msg,
absl::string_view element_path,
absl::string_view field_path) override {
if (behavior_ != FAIL_ON_ERROR_OR_FATAL) {
return absl::OkStatus();
}
Expand All @@ -363,34 +363,34 @@ class FailFastErrorHandler : public ErrorHandler {
absl::StrCat(msg, " at ", element_path));
}

absl::Status HandleFhirWarning(std::string_view msg,
std::string_view element_path,
std::string_view field_path) override {
absl::Status HandleFhirWarning(absl::string_view msg,
absl::string_view element_path,
absl::string_view field_path) override {
return absl::OkStatus();
}

absl::Status HandleFhirPathFatal(const absl::Status& status,
std::string_view expression,
std::string_view element_path,
std::string_view field_path) override {
absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) override {
return absl::Status(
status.code(),
absl::Substitute("Error evaluating FHIRPath expression `$0`: $1 at $2",
expression, status.message(), element_path));
}

absl::Status HandleFhirPathError(std::string_view expression,
std::string_view element_path,
std::string_view field_path) override {
absl::Status HandleFhirPathError(absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) override {
return behavior_ == FAIL_ON_ERROR_OR_FATAL
? absl::InvalidArgumentError(absl::Substitute(
"Failed expression `$0` at $1", expression, element_path))
: absl::OkStatus();
}

absl::Status HandleFhirPathWarning(std::string_view expression,
std::string_view element_path,
std::string_view field_path) override {
absl::Status HandleFhirPathWarning(absl::string_view expression,
absl::string_view element_path,
absl::string_view field_path) override {
return absl::OkStatus();
}

Expand Down
8 changes: 2 additions & 6 deletions cc/google/fhir/fhir_path/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,7 @@ cc_library(
"fhir_path_types.h",
],
strip_include_prefix = "//cc/",
visibility = [
"//visibility:private", # Only private by automation, not intent. Owner may accept CLs adding visibility. See go/scheuklappen#explicit-private.
],
visibility = ["//visibility:private"],
deps = [
"//cc/google/fhir:annotations",
"//cc/google/fhir:fhir_types",
Expand All @@ -143,9 +141,7 @@ cc_library(
"utils.h",
],
strip_include_prefix = "//cc/",
visibility = [
"//visibility:private", # Only private by automation, not intent. Owner may accept CLs adding visibility. See go/scheuklappen#explicit-private.
],
visibility = ["//visibility:private"],
deps = [
"//cc/google/fhir:annotations",
"//cc/google/fhir:references",
Expand Down
42 changes: 33 additions & 9 deletions cc/google/fhir/fhir_path/fhir_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@
#include "google/fhir/fhir_path/fhir_path_types.h"
#include "google/fhir/fhir_path/utils.h"
#include "google/fhir/fhir_types.h"
#include "google/fhir/primitive_handler.h"
#include "google/fhir/proto_util.h"
#include "google/fhir/status/status.h"
#include "google/fhir/status/statusor.h"
#include "google/fhir/util.h"
#include "proto/google/fhir/proto/r4/core/datatypes.pb.h"
#include "icu4c/source/common/unicode/unistr.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/message.h"

namespace google {
namespace fhir {
Expand Down Expand Up @@ -3039,7 +3041,8 @@ class ComparisonOperator : public BinaryOperator {

} else if (IsSystemString(*left_result) && IsSystemString(*right_result)) {
return EvalStringComparison(primitive_handler, left, right);
} else if (IsDateTime(*left_result) && IsDateTime(*right_result)) {
} else if ((IsInstant(*left_result) || IsDateTime(*left_result)) &&
(IsInstant(*right_result) || IsDateTime(*right_result))) {
return EvalDateTimeComparison(primitive_handler, *left_result,
*right_result);
} else if (IsSimpleQuantity(*left_result) &&
Expand Down Expand Up @@ -3147,12 +3150,33 @@ class ComparisonOperator : public BinaryOperator {
absl::StatusOr<absl::optional<bool>> EvalDateTimeComparison(
const PrimitiveHandler* primitive_handler, const Message& left_message,
const Message& right_message) const {
FHIR_ASSIGN_OR_RETURN(
DateTimePrecision left_precision,
primitive_handler->GetDateTimePrecision(left_message));
FHIR_ASSIGN_OR_RETURN(
DateTimePrecision right_precision,
primitive_handler->GetDateTimePrecision(right_message));
const Message* left = &left_message;
const Message* right = &right_message;
std::vector<std::unique_ptr<const Message>> to_delete;
if (IsInstant(left_message)) {
FHIR_ASSIGN_OR_RETURN(
JsonPrimitive json_primitive,
primitive_handler->WrapPrimitiveProto(left_message));
json_primitive.value = absl::StripPrefix(json_primitive.value, "\"");
json_primitive.value = absl::StripSuffix(json_primitive.value, "\"");
FHIR_ASSIGN_OR_RETURN(
left, primitive_handler->NewDateTime(json_primitive.value));
to_delete.push_back(std::unique_ptr<const Message>(left));
}
if (IsInstant(right_message)) {
FHIR_ASSIGN_OR_RETURN(
JsonPrimitive json_primitive,
primitive_handler->WrapPrimitiveProto(right_message));
json_primitive.value = absl::StripPrefix(json_primitive.value, "\"");
json_primitive.value = absl::StripSuffix(json_primitive.value, "\"");
FHIR_ASSIGN_OR_RETURN(
right, primitive_handler->NewDateTime(json_primitive.value));
to_delete.push_back(std::unique_ptr<const Message>(right));
}
FHIR_ASSIGN_OR_RETURN(DateTimePrecision left_precision,
primitive_handler->GetDateTimePrecision(*left));
FHIR_ASSIGN_OR_RETURN(DateTimePrecision right_precision,
primitive_handler->GetDateTimePrecision(*right));

// The FHIRPath spec (http:https://hl7.org/fhirpath/#comparison) states that "If
// one value is specified to a different level of precision than the other,
Expand All @@ -3164,9 +3188,9 @@ class ComparisonOperator : public BinaryOperator {
}

FHIR_ASSIGN_OR_RETURN(absl::Time left_time,
primitive_handler->GetDateTimeValue(left_message));
primitive_handler->GetDateTimeValue(*left));
FHIR_ASSIGN_OR_RETURN(absl::Time right_time,
primitive_handler->GetDateTimeValue(right_message));
primitive_handler->GetDateTimeValue(*right));

// negative if left < right, positive if left > right, 0 if equal
absl::civil_diff_t time_difference =
Expand Down
24 changes: 24 additions & 0 deletions cc/google/fhir/fhir_path/fhir_path_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2407,6 +2407,30 @@ TYPED_TEST(FhirPathTest, TimeComparison) {
EvalsToTrue());
}

TYPED_TEST(FhirPathTest, TimeComparison_Instant) {
auto observation = ParseFromString<typename TypeParam::Observation>(R"pb(
# 2001-10-28T01:59:00Z
meta: {
last_updated: {
value_us: 1004234340000000
timezone: "UTC"
precision: MICROSECOND
}
}
)pb");
EXPECT_THAT(
TestFixture::Evaluate(observation, "meta.lastUpdated = meta.lastUpdated"),
EvalsToTrue());
EXPECT_THAT(
TestFixture::Evaluate(observation, "meta.lastUpdated < meta.lastUpdated"),
EvalsToFalse());
EXPECT_THAT(
TestFixture::Evaluate(observation,
"meta.lastUpdated < @2001-10-28T01:59:10Z and "
"@2001-10-28T01:58:50 < meta.lastUpdated"),
EvalsToTrue());
}

TYPED_TEST(FhirPathTest, TimeCompareDifferentPrecision) {
using DateTime = typename TypeParam::DateTime;
using DateTimePrecision = typename TypeParam::DateTime::Precision;
Expand Down
Loading