Dead code warning in try-with-resources, but not i

2019-06-24 15:07发布

问题:

The following code uses the try-with-resources construction introduced in Java 8. The occasionallyThrow() method is declared to throw an OccasionalException, the Resource's close() method to throw a CloseException. Eclipse (Version: Neon Release (4.6.0), Build id: 20160613-1800) adds a warning on the line marked with // dead code that the branch is dead code. Implicitly, Eclipse affirms that the line marked with // alive code is not dead code.

Object tryWithResources() throws OccasionalException {
    Object value = null;
    try (Resource resource = new Resource()) {
        occasionallyThrow();
        value = new Object();
    }
    catch (CloseException e) {
        if (value == null) {
            // alive code
        }
        else {
            // dead code
        }
    }
    return value;
}

I'm confused by this. If occasionallyThrow() throws its OccasionalException, then the try-with-resources should catch that as the primary exception and then attempt to close the resource. If closing the resource throws a CloseException, then it will be suppressed under the OccasionalException, so there won't be the CloseException to catch. So, the only time there should be CloseException to catch is when the block within the try completed successfully which means that value is non-null. So it seems like the "dead code" is actually alive, and the "alive code" is actually dead. I'm not sure what a compiler is actually expected to recognize here, but at the very least, it seems like the "dead code" here should not be called dead.

What makes this more complicated is that the translated form without using the try-with-resources form doesn't get marked with any dead code warnings at all. (I'm fairly confident that I got this translation right, based on 14.20.3.2. Extended try-with-resources, but I wouldn't be completely surprised if there's bug here…)

Object expandedTry() throws OccasionalException {
    Object value = null;
    try {
        Resource resource = new Resource();
        Throwable $primary = null;
        try {
            occasionallyThrow();
            value = new Object();
        }
        catch (Throwable t) {
            $primary = t;
            throw t;
        }
        finally {
            if (resource != null) {
                if ($primary != null) {
                    try {
                        resource.close();
                    }
                    catch (Throwable $suppressed) {
                        $primary.addSuppressed($suppressed);
                    }
                }
                else {
                    resource.close();
                }
            }
        }
    }
    catch (CloseException e) {
        if (value == null) {
            // alive (not dead!)
        }
        else {
            // alive
        }
    }
    return value;
}

Am I missing something that would make either branch in the if-else dead in one of these, but not in the other?

Full Code

Here's the full code with definitions of the auxiliary exception types, the Resource class, and the top-level class.

public class TestTryWithResources {

    /** Exception thrown by Resource's close() method */
    @SuppressWarnings("serial")
    static class CloseException extends Exception {}

    /** AutoCloseable declared to throw a CloseException */ 
    static class Resource implements AutoCloseable {
        @Override
        public void close() throws CloseException {}
    }

    /** An occasionally thrown exception */
    @SuppressWarnings("serial")
    static class OccasionalException extends Exception {}

    /** Method declared to throw an occasional exception */
    void occasionallyThrow() throws OccasionalException {}

    /*
     * Method using try-with-resources.  Eclipse warns that the 
     * portion marked with "// dead code" is Dead code.
     */
    Object tryWithResources() throws OccasionalException {
        Object value = null;
        try (Resource resource = new Resource()) {
            occasionallyThrow();
            value = new Object();
        }
        catch (CloseException e) {
            if (value == null) {
                // alive code
            }
            else {
                // dead code
            }
        }
        return value;
    }

    /*
     * Method not using try-with-resources.  This is the translation
     * of the try-with-resources in tryWithResources, according to 
     * [14.20.3 try-with-resources][1].  Eclipse does not warn about 
     * any of the code being Dead code.
     * 
     * [1]: https://docs.oracle.com/javase/specs/jls/se7/html/jls-14.html#jls-14.20.3 
     */
    Object expandedTry() throws OccasionalException {
        Object value = null;
        try {
            Resource resource = new Resource();
            Throwable $primary = null;
            try {
                occasionallyThrow();
                value = new Object();
            }
            catch (Throwable t) {
                $primary = t;
                throw t;
            }
            finally {
                if (resource != null) {
                    if ($primary != null) {
                        try {
                            resource.close();
                        }
                        catch (Throwable $suppressed) {
                            $primary.addSuppressed($suppressed);
                        }
                    }
                    else {
                        resource.close();
                    }
                }
            }
        }
        catch (CloseException e) {
            if (value == null) {
                // alive
            }
            else {
                // alive
            }
        }
        return value;
    }
}

Responses to Comments

Amin J's answer suggested a workaround of using the resource after setting value to change Eclipse's code analysis. This doesn't work though. After using the resource, e.g., by printing it, the dead code warning is still present in both Luna and Neon:

回答1:

For some reason the static code analyzer thinks that that the resource is going to be closed right after the try declaration while according to this tutorial the resource is closed after the statement.

The try-with-resources statement ensures that each resource is closed at the end of the statement.

So for instance if you change your code to use the resource after value is is (code below) it won't warn you about the dead code (tested on Eclipse Luna however).

Object tryWithResources() throws OccasionalException {
    Object value = null;
    try (Resource resource = new Resource()) {
        occasionallyThrow();
        value = new Object();
        resource.someMethod(); // using the resource, so eclipse thinks it's not closed yet (correctly)
    }
    catch (CloseException e) {
        if (value == null) {
            // alive code
        }
        else {
            // dead code
        }
    }
    return value;
}

UPDATE

This is the actual code I tested with usage of resource (in this case reader) after setting the value.

        Object val = null;

        try (BufferedReader reader = new BufferedReader(new FileReader("C:\\file.txt"))) {
            val = new Object();
            System.out.println("got here");
            reader.readLine();
        }
        catch(IOException e){
            System.out.println("io ex");
            if ( val == null){

            }
            else{

            }
        }


回答2:

This doesn't answer the question of why Eclipse is generating the warning that it is, or whether it should be or not, but this is a workaround that at least eliminates the warning for the time being. Rather than putting the conditional in the catch block, you can call another method with the value being tested as well as a the exception, and from that method test whether the object is null and then do whatever needs to be done:

Object tryWithResources() throws OccasionalException {
    Object value = null;
    try (Resource resource = new Resource()) {
        occasionallyThrow();
        value = new Object();
        System.out.println(resource.toString());
    }
    catch (CloseException e) {
        catchBlock(value, e);  // call auxiliary method
    }
    return value;
}

void catchBlock(Object value, CloseException e) {
    if (value == null) {
        // then
    }
    else {
        // else
    }
}