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

Apparently dead code in LoggerFactory #371

Closed
TWiStErRob opened this issue Dec 4, 2023 · 3 comments
Closed

Apparently dead code in LoggerFactory #371

TWiStErRob opened this issue Dec 4, 2023 · 3 comments

Comments

@TWiStErRob
Copy link

TWiStErRob commented Dec 4, 2023

While reading slf4j2 code to figure out how bindingsproviders work, I came across a line that confused me: catch(NoSuchFieldError) in LoggerFactory

private final static void versionSanityCheck() {
try {
String requested = PROVIDER.getRequestedApiVersion();
boolean match = false;
for (String aAPI_COMPATIBILITY_LIST : API_COMPATIBILITY_LIST) {
if (requested.startsWith(aAPI_COMPATIBILITY_LIST)) {
match = true;
}
}
if (!match) {
Util.report("The requested version " + requested + " by your slf4j provider is not compatible with "
+ Arrays.asList(API_COMPATIBILITY_LIST).toString());
Util.report("See " + VERSION_MISMATCH + " for further details.");
}
} catch (java.lang.NoSuchFieldError nsfe) {
// given our large user base and SLF4J's commitment to backward
// compatibility, we cannot cry here. Only for implementations
// which willingly declare a REQUESTED_API_VERSION field do we
// emit compatibility warnings.
} catch (Throwable e) {
// we should never reach here
Util.report("Unexpected problem occurred during version sanity check", e);
}
}

I was wondering why the comment references REQUESTED_API_VERSION, while there's no such thing in the try.

I found the change that invalidated that catch, and I'm still baffled as to why Java does not complain about a checked exception being not used I see why javac didn't alert: it's an Error, not checked!.

@ceki
Copy link
Member

ceki commented Dec 5, 2023

Hi @TWiStErRob

I think the NoSuchFieldError can no longer occur and it makes no sense to check for it. As for the compiler not complaining, it is a good question but beyond the scope of SLF4J. ☺️

@TWiStErRob
Copy link
Author

See edit: Error is not a checked exception.

@ceki
Copy link
Member

ceki commented Dec 5, 2023

Unreachable code removed in commit 5ccbe20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants