Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-13 Thread via GitHub
ChrisHegarty merged PR #14633: URL: https://github.com/apache/lucene/pull/14633 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucen

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-12 Thread via GitHub
thecoop commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2873415595 I definitely agree that having to have a separate `throw t` as part of the exception handling is trappy (although generally ok due to flow-control changes breaking the compile without it

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-10 Thread via GitHub
rmuir commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2869000472 > > I thought it did not work with multi-catch, but that it works here. Of course you could make a more horrible hack, but some people don't like this in the Lucene community (its know

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
mikemccand commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2867787360 +1 to eliminate the crazy `success` pattern! I hope we can also eliminate even the `success2` cases :) Thank you @thecoop for fixing this! -- This is an automated message fr

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866618642 The reason for that problem is in the JLS: https://docs.oracle.com/javase/specs/jls/se24/html/jls-11.html#jls-11.2.2 > A throw statement whose thrown expression is a final or ef

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
thecoop commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866538943 Unfortunately that doesn't work here - Java forgets the specific exception types thrown in the try when the exception goes in & out of the generic method - it infers `T` to `Throwable`,

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866593107 Indeed that does not work. I was under the impression that the compiler should be able to see that the method parameter has same type as return type by this. Looks like that's not ful

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866550770 > Unfortunately that doesn't work here - Java forgets the specific exception types thrown in the try when the exception goes in & out of the generic method - it infers `T` to `Throwab

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866499124 I have a suggestion. I used the same approach in the expressions module to patch the stack trace and rethrowing exceptions (to include the source code of the exception into the stack

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866504083 P.S.: Here is the bytecode of the Expression patching: https://github.com/apache/lucene/pull/14602#issuecomment-2845328503 Small note: As this is generated bytecode the generics

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
thecoop commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866484934 @uschindler PR updated. I've applied this refactor to the most recent set of formats, as those are most likely to be copied in future format changes. That seems like a good place to leav

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
thecoop commented on code in PR #14633: URL: https://github.com/apache/lucene/pull/14633#discussion_r2081616824 ## lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene101/Lucene101PostingsReader.java: ## @@ -120,23 +119,21 @@ public Lucene101PostingsReader(Se

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on code in PR #14633: URL: https://github.com/apache/lucene/pull/14633#discussion_r2081556092 ## lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene101/Lucene101PostingsReader.java: ## @@ -120,23 +119,21 @@ public Lucene101PostingsReader

Re: [PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
uschindler commented on PR #14633: URL: https://github.com/apache/lucene/pull/14633#issuecomment-2866336273 Hi, In general at the time when the success code was written a catch/rethrow of Throwable would not compile without an inappropriate throws clause on the method. Because the java c

[PR] Catch and re-throw Throwable rather than using a success boolean [lucene]

2025-05-09 Thread via GitHub
thecoop opened a new pull request, #14633: URL: https://github.com/apache/lucene/pull/14633 The use of a boolean `success` parameter is common in the Lucene codebase. This can be replaced with a `catch (Throwable t) {...; throw t}` pattern that means a boolean doesn't need to be used at all