add source locations to evaluation errors

[cdisselkoen]

Description of changes

Source locations for evaluation errors.

I think the required changes to EvaluationError manage to be non-breaking -- EvaluationError is a public type, but its fields are private, this PR doesn't change the EvaluationErrorKind enum, and the constructors I changed are all marked pub(crate). But someone should check me on this.

(I did remove two From impls, which can be added back if necessary. But they should be deprecated in favor of the new constructors which also take a source location.)

Issue #, if available

Towards resolving #485. (Just evaluation errors, not the entity-parsing errors or other errors also mentioned in #485.)

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

• A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

I confirm that this PR (choose one, and delete the other options):

• Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

• Does not require updates because my change does not impact the Cedar Dafny model or DRT infrastructure.

Disclaimer

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[john-h-kastner-aws]

Looks good. I don't see anything that looks like a breaking change

[andrewmwells-amazon]

This seems error-prone to me. Maybe create an issue to fix it?

[cdisselkoen]

Happy to create an issue, curious what you find to be error-prone about this?

Are you concerned that the source location in this field might disagree with the source location embedded in error_kind? If it makes you feel better, the vast majority of the EvaluationErrorKinds do not have a source loc field -- the only cases that do have a source loc are InvalidRestrictedExpression and NonValue, and those just because they store an Expr as part of the error. We could make an issue to remove these somehow, so that error_kind never contains a source loc; is that what you're suggesting?

Going the other way -- having all the source locs in error_kind so that we never need this field -- is backwards; we established recently that we prefer the convention to use the *Kind pattern to have a single place the source loc field is defined, rather than adding a source loc field in N places, once for each enum variant. The struct here conforms to that practice.

[andrewmwells-amazon]

Yeah, it's just a general concern about "code smell," not anything concrete. Even if we always remember to keep them in sync, I'd worry about that complexity hiding other bugs. E.g., I think the bug we had where it was possible to add a link and a template with colliding names wouldn't have happened if we didn't have duplicate information in our lists of policies+templates and links+templates. Even though it wasn't caused by forgetting to keep data in sync, the added complexity (e.g., rolling back changes to one representation if modifying the other failed) distracts code reviewers from the obvious "you forgot to check that no link with this ID exists."