BasicAnnotationProcessor:: Changing deferral policy by AriaAdibi · Pull Request #1348 · google/auto

AriaAdibi · 2022-07-11T13:23:25Z

Following the discussion at #1340, I believe changing the defer policy to individual rejected elements as opposed to their enclosing TypeElement (if not PackageElement) is a sound and beneficial choice for the following reasons.

For my convenience whenever I refer to an element, please assume that I am talking about a non-package element.

Before I explain I want to note that in my solution superficial validation for each element is still done starting from its enclosing TypeElement; therefore, the deferral policy when it comes to well-informedness has not changed. Deferral policy is changed only for those elements that are rejected by a processor which are guaranteed to be well-informed.

Is it backward compatible?

I believe it should be. Unless someone uses some hacks to overcome similar (rare) difficulties like that of the discussion.

Why is it a sound choice?

The main problem in our discussion was that according to process() in Processor Javadoc (and only here) the input is TypeElements originating from the prior round and not any Elements. (Oddly enough, as it is verified in the discussion in practicality this is not the behavior of the checked Java compilers, more on this later.) I believe this is still the case especially if the current behavior of BasicAnnotationProcessor is acceptable.

Presently, the aforementioned process() is overridden, and within it another process() method Is called. To distinguish between them let's call the latter process2(). The input of process2() is not restricted to TypeElements and in fact it gets any Element in general. With the current policy. the step deferred elements are only considered again in process2() and have nothing to do with process(). The only exception is when in process() the missing elements are to be reported, where now only enclosing types are reported but with my solution, the exact problematic elements are reported. I can only report the violating TypeElements as well if required. For this reason, I believe the solution does not violate the aforementioned Javadoc.
As hinted before, the checked processing tools (like OpenJDK) do not adhere to only sending the TypeElements and in that case, the new solution is the more appropriate behavior to have.

Why beneficial?

As discussed in the discussion because of the current behavior of common processing tools (like OpenJDK) if the annotation target something other than Types, it may happen that some elements correctly get processed and just because one other element within the same enclosing TypeElement gets rejected, all of these elements will be sent to be processed again. This is not optimized and also causes some headaches for the users (including me :)) of Google's BasicAnnotationProcessor to handle multiple processing scenarios.

Asking for some favors :)

I would greatly appreciate it if you check the Javadoc of ElementName and let me know if I am correct and give me your opinion. I think the main purpose of ElementName instead of Element is the information presented here.
For the unit test I could not figure out the API to say more than 1 file is created with the processor. I would appreciate your help in this regard as well.

Edits:

Now the retrieval of overloaded executable elements is done correctly. For now to gain access to erasedParameterTypes() I have added an instance of ExplicitOverrides in ExecutableElementName. While this is not bad it is subjective and for me is less than ideal, if this proposal gets accepted please let me know your opinion about a slight change in structure so that this method is more generally accessible.

I would still greatly appreciate your comments about the questions that I have asked before.

Thank you!

eamonnmcmanus · 2022-09-05T21:16:16Z

Thanks for this! I'm finally getting around to it.

To answer some of the questions above:

I think that rather than calling ExplicitOverrides.erasedParameterTypes, it might be better to do something like MoreTypes.asExecutable(executableElement.asType()).getParameterTypes() and then erase those yourself with Types.erasure.
I'm not sure I understand the question about the unit test, but you should be able to assert about each generated file in its own statement. You should be using the more recent compile-testing API, though, like this.

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

AriaAdibi · 2022-09-25T00:54:52Z

Thank you for your thorough comments. Sorry for the delayed response, I have a relatively busy schedule.

I have made the changes requested and responded. I have also made additional changes and I want to mention a few of them and provide the reason(s). The change has got a little big and therefore, I have tried to organize my commits relatively isolated and meaningful to help the reviewer(s) to see the changes better.

I have added support for TypeParameter which was missing. I am not sure why before it was decided to ignore TypeParameter. For example, if class A<@Annotation T> gets deferred then T would not have been processed at all. (I believe I have noticed a bug in deferral when it comes to TypeParameter as well, but I will leave this topic for a later pull request.)
8d05d4f is an unrelated fix. However, because the fix was very small I included it here. Please let me know if you want it in a separate pull request.
I believe ElementBluePrint is a more suitable name than ElementName therefore I have changed the name. I can revert the change if desired.
Please let me know your thoughts on UnsupportedElementBluePrint.
b1eb35e To support RECORD_COMPONENT I have implemented getEnclosingType() differently, and moved it to MoreElements.

As always I would appreciate your comments,
Thank you.

In the following example, class ClassA{ @annotation void overloadedMethod(SomeGeneratedClass c) {} @annotation void overloadedMethod(int c){} } at least open-jdk does not report the second method if `SomeGeneratedClass` references a `TypeKind#ERROR` when `RoundEnvironment#getElementsAnnotatedWith` is called, or even when `TypeElement#getEnclosedElements()` is called. Therefore, our implementation should be vigilant that the second method is captured for processing at some point. Note that for a method to get "hidden" like this, it should reside after the `ERROR` referencing method, and it should not have any distinguishing characteristic like different name, different number of parameter, or a clear parameter type mismatch.

eamonnmcmanus

(By the way, I ran the previous version through Google's internal tests and everything passed.)

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

eamonnmcmanus · 2023-12-20T22:06:19Z

common/src/main/java/com/google/auto/common/MoreElements.java

+   * Returns the nearest enclosing {@link TypeElement} to the current element, throwing an {@link
+   * IllegalArgumentException} if the provided {@link Element} is not enclosed by a type.
+   */
+  public static TypeElement getEnclosingType(Element element) {


OK. Usually for a public API change like this we have an internal API review process. But I think this one is very straightforward and we can probably dispense with that.

eamonnmcmanus · 2023-12-20T22:10:00Z

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

+      TypeElement enclosingTypeElement = getEnclosingType(element);
+      this.enclosingTypeElementBluePrint = new TypeElementBluePrint(enclosingTypeElement);
+      this.simpleName = element.getSimpleName();
+      int ordinalOverloadPosition = 1;


I see, yes. Although we could imagine that the different processing rounds could return getEnclosedElements() in a different order, I think that would be pretty surprising given the specification that you quote. So indeed just storing the position of the element among all enclosed elements might be sufficient? Without needing to have any specific logic for overloading, except perhaps a verification check to ensure that the simple name is still the one we expect.

Alternatively, if you want to be safe and keep the logic as it stands, I think it would be good to get rid of that Iterator. I always wince a bit when I see an Iterator. 🙂 What about just streaming over the enclosed elements here too, to produce a List of elements with the same name, and then using indexOf to get ordinalOverloadPosition. Perhaps rename that, too.

AriaAdibi · 2024-01-06T04:32:02Z

Hello @eamonnmcmanus,
I hope this message finds you well.

I wanted to gently remind you about the pending review, in case it got buried in the midst of your busy schedule.

Your attention to this matter is greatly appreciated.
Thank you!

eamonnmcmanus

Sorry I'm only finding time for this now!

I think at this point we can take these changes and put them through Google's internal review process. I'll probably make a few cosmetic tweaks, and the review may result in further changes. There'll be a separate PR, where you will be duly credited, which will close this one.

I left a comment about the new getEnclosingType method.

eamonnmcmanus · 2024-01-27T19:47:57Z

common/src/main/java/com/google/auto/common/MoreElements.java

+   * Returns the nearest enclosing {@link TypeElement} to the current element, throwing an {@link
+   * IllegalArgumentException} if the provided {@link Element} is not enclosed by a type.
+   */
+  public static TypeElement getEnclosingType(Element element) {


I'm still a bit unsure about this. It seems to me that if element is already a type, the name and specification of the method suggest that it should nevertheless return that type's enclosing type. But in fact the implementation returns element itself. I know the private method in BasicAnnotationProcessor was the same, but I think we should change either the behaviour or the name.

I think this would be a good candidate for a follow-on change, with this change retaining the private getEnclosingType for now.

@AriaAdibi

…rencing missing types. See #1348 for details. If a processor is triggered by a method annotation, for example, and the method references as-yet-undefined types, then we should defer processing of just that method to the next processing round. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of as-yet-undefined types in method and field signatures. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

@AriaAdibi

…rencing missing types. See #1348 for details. If a processor is triggered by a method annotation, for example, and the method references as-yet-undefined types, then we should defer processing of just that method to the next processing round. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of as-yet-undefined types in method and field signatures. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

@AriaAdibi

…rencing missing types. See #1348 for details. If a processor is triggered by a method annotation, for example, and the method references as-yet-undefined types, then we should defer processing of just that method to the next processing round. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of as-yet-undefined types in method and field signatures. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

eamonnmcmanus

So you don't need to do anything, but just so you know we're discovering that there are some bugs that didn't show up because of gaps in the test coverage.

eamonnmcmanus · 2024-01-31T01:25:56Z

common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java

+  }
+
+  @Test
+  public void properlyDefersProcessing_recordComponent() {


FYI we're discovering that these tests aren't quite complete. This is testing that a record will be deferred until the next round if it contains a component whose type is not yet defined. However what's really new with this PR is the notion of rejecting elements in a round even if they don't reference undefined types. So I think there need to be test methods for that too. If I add them I discover some issues with the code, because the various FooElementBlueprint classes weren't actually being tested.

You are absolutely right, the testing is subpar. However, I am not sure I understand the potential problem here. I look forward to your tests. If I could do anything please don't hesitate to ask.

Well, here's what happened. During the internal review, my teammate @netdpb asked about type parameters of methods as opposed to classes. It looked as if the code in TypeParameterElementBlueprint wouldn't work in that case. So I added a test method with

<@TypeParameterRequiresGeneratedCode T extends SomeGeneratedClass> void foo(T t) {}

That test method didn't fail, so I added traces and discovered that TypeParameterElementBlueprint wasn't being invoked. That's because the missing type SomeGeneratedClass causes the whole containing class to be deferred to the next round. At that point I realized that I had not been distinguishing clearly between classes being deferred because some element in them references an undefined type, and arbitrary elements being deferred because a Step rejects them. That second thing is where the new factories (blueprints) come into play. It only happens when there are no undefined types, and it's where we were missing some test coverage.

The updated #1702, still work-in-progress, includes new test methods.

eamonnmcmanus · 2024-01-31T01:27:00Z

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

+    }
+
+    @Override
+    Optional<VariableElement> getElement(Elements elementUtils) {


This isn't correct for a RecordComponentElement, which isn't a VariableElement.

Well spotted, thank you. I must admit at the time I was writing this in 2022, I just started working with Java 11 and did not know about records.

eamonnmcmanus · 2024-01-31T01:27:44Z

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

+
+    private TypeParameterElementBlueprint(Element element) {
+      super(element);
+      this.enclosingTypeElementBlueprint = new TypeElementBlueprint(element.getEnclosingElement());


This isn't correct for the type parameter of a method.

@AriaAdibi

…cted by processing steps. See #1348 for details. If a processor is triggered by a method annotation, for example, and the processor rejects the method until the next round, then we should defer processing of just that method, not the entire class containing it. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of deferring processing of methods and other non-type elements. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

@AriaAdibi

…cted by processing steps. See #1348 for details. If a processor is triggered by a method annotation, for example, and the processor rejects the method until the next round, then we should defer processing of just that method, not the entire class containing it. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of deferring processing of methods and other non-type elements. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

@AriaAdibi

…cted by processing steps. See #1348 for details. If a processor is triggered by a method annotation, for example, and the processor rejects the method until the next round, then we should defer processing of just that method, not the entire class containing it. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of deferring processing of methods and other non-type elements. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

@AriaAdibi

…cted by processing steps. See #1348 for details. If a processor is triggered by a method annotation, for example, and the processor rejects the method until the next round, then we should defer processing of just that method, not the entire class containing it. Closes #1348. RELNOTES=`BasicAnnotationProcessor` has improved handling of deferring processing of methods and other non-type elements. Thanks to @AriaAdibi for this contribution! PiperOrigin-RevId: 602032802

Currently, the documentation references some Javadoc about how a given lowercase-e element may be represented by different `Element` instances over time. However, I think that's a red herring for the problem that we're solving: - The doc doesn't necessarily imply that older instance become "invalid," only that they should be compared with `equals` instead of `==`. (And I suspect that we've done as much all along. As a bonus, using `equals` probably isn't even technically necessary today, at least if we depend on implementation details of javac—though of course we shouldn't.) - The doc doesn't necessarily imply that there's anything special about the boundary between rounds. Thus, as far as the Javadoc for `Element` is concerned, the compiler could switch which instance it returns *during* a round just as easily as it could use different instances across rounds. And thus, the doc isn't explicit on whether we should treat a given `Element` as having a lifetime of exactly the round in which we looked it up. But this code does appear to be a response to a real behavior in javac—and perhaps especially [in Eclipse](https://bugs.eclipse.org/bugs/show_bug.cgi?id=480936). (I haven't looked into Turbine.) There does in fact appear to be an undocumented(?) restriction on the the lifetime of an `Element` instance: - [JDK-6191665](https://bugs.openjdk.org/browse/JDK-6191665) - [JDK-7026845](https://bugs.openjdk.org/browse/JDK-7026845?focusedId=12082299&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12082299) (The behavior of `Element` instances across rounds [might have changed for Java 7](https://bugs.openjdk.org/browse/JDK-8144105), but the behavior seen in Java 7 appears to have persisted despite [some changes to reuse state across rounds in Java 9](https://bugs.openjdk.org/browse/JDK-8038455).) Notably, while `Element` documents that callers should use `Element.equals` instead of identity comparison, it appears that even `Element.equals` does not consider two distinct `Element` instances for "the same declaration" to be equal. (As of this writing, javac does not appear to override `equals` in [its `Element` implementations](https://github.com/openjdk/jdk/blob/2a71f89bc8d72be8095113695e541f4f38acdeca/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java) at all.) Concretely, we've seen that an `ExecutableElement` from one round may have an enclosing element whose enclosed-elements list does not contain anything equal to that `ExecutableElement`. That specific quirk may or may not be a problem, but it's further evidence that such usage is unsupported. (For what it's worth, I edited our `ElementFactory` implementations to always return the original `Element`, and I got no failures in a TAP Global Presubmit run except for those in `BasicAnnotationProcessorTest`, which depends on the quirk discussed just above. That quirk comes up only because `ElementFactory` looks up the enclosing element as part of its equality logic, so I'd expect all tests to pass if we were to replace `ElementFactory` with `Element`. It would be nice to have test coverage that demonstrated a clearer need for `ElementFactory`. We could add some by directly performing an equality comparison on an `ExecutableElement` from a different round, but it would be nice to have a better sense of how that might matter in practice and how widespread the problem is—e.g., whether it also affects `TypeMirror` instances. But given that we know Eclipse behaves differently from javac, we might well see the problems when testing under ejc, which I think we might have some setup for?) (One use case that probably works for modern versions of javac (but quite likely still not Eclipse) is that of storing only `ClassSymbol`/`TypeElement` instances, which [are currently reused across rounds](https://bugs.openjdk.org/browse/JDK-6191665?focusedId=13519728&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13519728).) (followup to e20db25 / #1348) RELNOTES=n/a PiperOrigin-RevId: 872557496

Currently, the documentation references some Javadoc about how a given lowercase-e element may be represented by different `Element` instances over time. However, I think that's a red herring for the problem that we're solving: - The doc doesn't necessarily imply that older instance become "invalid," only that they should be compared with `equals` instead of `==`. (And I suspect that we've done as much all along. As a bonus, using `equals` probably isn't even technically necessary today, at least if we depend on implementation details of javac—though of course we shouldn't.) - The doc doesn't necessarily imply that there's anything special about the boundary between rounds. Thus, as far as the Javadoc for `Element` is concerned, the compiler could switch which instance it returns *during* a round just as easily as it could use different instances across rounds. And thus, the doc isn't explicit on whether we should treat a given `Element` as having a lifetime of exactly the round in which we looked it up. But this code does appear to be a response to a real behavior in javac—and perhaps especially [in Eclipse](https://bugs.eclipse.org/bugs/show_bug.cgi?id=480936). (I haven't looked into Turbine.) There does in fact appear to be an undocumented(?) restriction on the the lifetime of an `Element` instance: - [JDK-6191665](https://bugs.openjdk.org/browse/JDK-6191665) - [JDK-7026845](https://bugs.openjdk.org/browse/JDK-7026845?focusedId=12082299&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12082299) (The behavior of `Element` instances across rounds [might have changed for Java 7](https://bugs.openjdk.org/browse/JDK-8144105), but the behavior seen in Java 7 appears to have persisted despite [some changes to reuse state across rounds in Java 9](https://bugs.openjdk.org/browse/JDK-8038455).) Notably, while `Element` documents that callers should use `Element.equals` instead of identity comparison, it appears that even `Element.equals` does not consider two distinct `Element` instances for "the same declaration" to be equal. (As of this writing, javac does not appear to override `equals` in [its `Element` implementations](https://github.com/openjdk/jdk/blob/2a71f89bc8d72be8095113695e541f4f38acdeca/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java) at all.) Concretely, we've seen that an `ExecutableElement` from one round may have an enclosing element whose enclosed-elements list does not contain anything equal to that `ExecutableElement`. That specific quirk may or may not be a problem, but it's further evidence that such usage is unsupported. (For what it's worth, I edited our `ElementFactory` implementations to always return the original `Element`, and I got no failures in a TAP Global Presubmit run except for those in `BasicAnnotationProcessorTest`, which depends on the quirk discussed just above. That quirk comes up only because `ElementFactory` looks up the enclosing element as part of its equality logic, so I'd expect all tests to pass if we were to replace `ElementFactory` with `Element`. It would be nice to have test coverage that demonstrated a clearer need for `ElementFactory`. We could add some by directly performing an equality comparison on an `ExecutableElement` from a different round, but it would be nice to have a better sense of how that might matter in practice and how widespread the problem is—e.g., whether it also affects `TypeMirror` instances. But given that we know Eclipse behaves differently from javac, we might well see the problems when testing under ejc, which I think we might have some setup for?) (One use case that probably works for modern versions of javac (but quite likely still not Eclipse) is that of storing only `ClassSymbol`/`TypeElement` instances, which [are currently reused across rounds](https://bugs.openjdk.org/browse/JDK-6191665?focusedId=13519728&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13519728).) (followup to e20db25 / #1348) RELNOTES=n/a PiperOrigin-RevId: 872912877

AriaAdibi force-pushed the dev/change_defer_policy branch 2 times, most recently from 0174584 to 1e30f1a Compare July 11, 2022 14:28

cgdecker requested a review from eamonnmcmanus July 13, 2022 16:11

cgdecker added Component: common P3 labels Jul 13, 2022

AriaAdibi changed the title ~~BasicAnnotationProcessor:: Changing defer policy~~ Draft:BasicAnnotationProcessor:: Changing defer policy Jul 20, 2022

AriaAdibi changed the title ~~Draft:BasicAnnotationProcessor:: Changing defer policy~~ BasicAnnotationProcessor:: Changing defer policy Jul 20, 2022

AriaAdibi marked this pull request as draft July 20, 2022 00:37

eamonnmcmanus self-assigned this Jul 21, 2022

AriaAdibi force-pushed the dev/change_defer_policy branch from 29665e4 to e9ff8ce Compare August 29, 2022 12:53

AriaAdibi marked this pull request as ready for review August 29, 2022 12:56

AriaAdibi force-pushed the dev/change_defer_policy branch 3 times, most recently from 702fc9d to 9682796 Compare September 1, 2022 13:44

AriaAdibi changed the title ~~BasicAnnotationProcessor:: Changing defer policy~~ BasicAnnotationProcessor:: Changing deferral policy Sep 1, 2022

eamonnmcmanus requested changes Sep 5, 2022

View reviewed changes

AriaAdibi force-pushed the dev/change_defer_policy branch from ffd9e5a to 541078f Compare September 9, 2022 21:46

AriaAdibi marked this pull request as draft September 15, 2022 14:53

AriaAdibi force-pushed the dev/change_defer_policy branch from 541078f to a9500fd Compare September 15, 2022 14:54

AriaAdibi force-pushed the dev/change_defer_policy branch 3 times, most recently from 49e044f to 816b662 Compare September 24, 2022 12:08

AriaAdibi marked this pull request as ready for review September 25, 2022 00:55

AriaAdibi requested a review from eamonnmcmanus September 25, 2022 01:15

AriaAdibi marked this pull request as draft September 26, 2022 18:22

AriaAdibi marked this pull request as ready for review September 27, 2022 06:14

AriaAdibi force-pushed the dev/change_defer_policy branch from d71240c to e3b3178 Compare September 27, 2022 13:22

AriaAdibi force-pushed the dev/change_defer_policy branch from e3b3178 to 7c7587d Compare December 13, 2023 19:37

AriaAdibi added 7 commits December 20, 2023 15:54

unit-test, proper deferral of executable elements

fa15c13

Corrected and optimized the deferral behaviour for ill-formed elements

13c33c9

Typo: "blue print" -> "blueprint"

ec2f8d6

Enhanced the naming and comment

e110dda

ElementBlueprint: toString. public -> protected

b70ebdb

getExecutableElement -> used less efficient but more readable method

c91b8a5

AriaAdibi force-pushed the dev/change_defer_policy branch from 3a7a25b to c91b8a5 Compare December 20, 2023 20:56

Variable order changed for readability

a709294

AriaAdibi requested a review from eamonnmcmanus December 20, 2023 21:28

eamonnmcmanus reviewed Dec 20, 2023

View reviewed changes

Simple While loop instead of Iterator

75722fb

AriaAdibi requested a review from eamonnmcmanus December 22, 2023 22:28

findAnnotatedElements: type conversion done only once if necessary

79d1cef

eamonnmcmanus requested changes Jan 27, 2024

View reviewed changes

copybara-service bot mentioned this pull request Jan 27, 2024

In BasicAnnotationProcessor, correctly defer non-type elements rejected by processing steps. #1702

Closed

eamonnmcmanus reviewed Jan 31, 2024

View reviewed changes

copybara-service bot closed this in e20db25 Jan 31, 2024

copybara-service bot mentioned this pull request Feb 20, 2026

Improve the documentation of ElementFactory. #2027

Merged

Comments

Conversation

AriaAdibi commented Jul 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Is it backward compatible?

Why is it a sound choice?

Why beneficial?

Asking for some favors :)

Edits:

Uh oh!

eamonnmcmanus commented Sep 5, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AriaAdibi commented Sep 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eamonnmcmanus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AriaAdibi commented Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eamonnmcmanus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eamonnmcmanus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AriaAdibi commented Jul 11, 2022 •

edited

Loading

AriaAdibi commented Sep 25, 2022 •

edited

Loading

AriaAdibi commented Jan 6, 2024 •

edited

Loading