This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 262167 - Convert "ifs" to "switch" incorrectly generated code
Summary: Convert "ifs" to "switch" incorrectly generated code
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 8.1
Hardware: PC Windows 8.1
: P2 normal with 1 vote (vote)
Assignee: Svata Dedic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-20 14:57 UTC by ibeaumont
Modified: 2016-05-26 01:58 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ibeaumont 2016-05-20 14:57:07 UTC
Using the Java hint in the code below (marked *** HINT ***), generates the wrong code.



 private DBEntry getCurrentBatchParams(DataSection ds,SQLInsertModuleProperties sqlInsertProps, Map<String, String> javaScriptVariableToFieldNameMap, Map<Integer, SQLInsertParamInterface.EVALUATE_STATUS> evaluateExpressions, Map<Integer, Object> resolvedExpressions,Map<Integer, CompiledScript> compiledScripts,Map<Integer, Bindings> bindings, Integer[] dataSectionColumnIndexPosition) throws SQLException, ModuleException {
        List<SQLInsertParamInterface> params = sqlInsertProps.getParams();
        List<Object> realData = new ArrayList<>(params.size());
        for (int i = 0; i < params.size(); i++) {
            SQLInsertParam p = (SQLInsertParam)params.get(i);
            try {
                if (evaluateExpressions.get(i) != null) {
*****HINT****                    if (evaluateExpressions.get(i) == SQLInsertParamInterface.EVALUATE_STATUS.LITERAL) {
                        realData.add(resolvedExpressions.get(i));
                    } else if (evaluateExpressions.get(i) == SQLInsertParamInterface.EVALUATE_STATUS.ONLY_RESOLVE) {
                        Integer index = dataSectionColumnIndexPosition[i];
                        switch (p.getDatatype()) {
                            case String:
                                realData.add(ds.getData().getString(index));
                                break;
                            case Boolean:
                                realData.add(ds.getData().getBoolean(index));
                                break;
                            case Byte:
                                realData.add(ds.getData().getByte(index));
                                break;
                            case Double:
                                realData.add(ds.getData().getDouble(index));
                                break;
                            case Float:
                                realData.add(ds.getData().getFloat(index));
                                break;
                            case Int:
                                realData.add(ds.getData().getInt(index));
                                break;
                            case Long:
                               realData.add(ds.getData().getLong(index));
                                break;
                            case BinaryStream:
                               realData.add(ds.getData().getBinaryStream(index));
                                break;
                            case Object:
                                realData.add(ds.getData().getObject(index));
                                break;
                            case CharacterStream:
                                //DataSection getCharacterStream is not implemented
                                realData.add(ds.getData().getString(index));
                                break;
                            case Short:
                              realData.add(ds.getData().getShort(index));
                                break;
                            default:
                                Logger.logAlertStateEvent(StdDebugLogMsg.sqlUnsupportedDatatype, new String[]{p.getDatatype().toString()});
                                throw new ModuleException("Unsupported Data Type for: " + p.getDatatype().toString());
                        }  
                    } else if (evaluateExpressions.get(i) == SQLInsertParamInterface.EVALUATE_STATUS.SCRIPT_EVALUATE) {
                        realData.add(compiledScripts.get(i).eval());
                    } else if (evaluateExpressions.get(i) == SQLInsertParamInterface.EVALUATE_STATUS.RESOLVE_AND_SCRIPT_EVALUATE) {
                        for (String s : p.getVariableList()) {
                            bindings.get(i).put(javaScriptVariableToFieldNameMap.get(s), ds.getData().getObject(s));
                        }
                        Object o = compiledScripts.get(i).eval();
                        for (String s : p.getVariableList()) {
                            bindings.get(i).remove(javaScriptVariableToFieldNameMap.get(s));
                        }
                        realData.add(o);
                    } 
                }
            } catch (ModuleException | SQLException | ScriptException e) {
                throw new ModuleException("Error processing column:\n\t" + p.toString(), e);
            }           
        }
        return new DBEntry(realData,ds.getData().getRow());
    }




The code generated is as follows.  There is a "break" missing - indicated by ***** MISSING BREAK ******
This code will compile fine, but behaviour in a totally different way.  So it is hard to pick up without good testing.


private DBEntry getCurrentBatchParams(DataSection ds,SQLInsertModuleProperties sqlInsertProps, Map<String, String> javaScriptVariableToFieldNameMap, Map<Integer, SQLInsertParamInterface.EVALUATE_STATUS> evaluateExpressions, Map<Integer, Object> resolvedExpressions,Map<Integer, CompiledScript> compiledScripts,Map<Integer, Bindings> bindings, Integer[] dataSectionColumnIndexPosition) throws SQLException, ModuleException {
        List<SQLInsertParamInterface> params = sqlInsertProps.getParams();
        List<Object> realData = new ArrayList<>(params.size());
        for (int i = 0; i < params.size(); i++) {
            SQLInsertParam p = (SQLInsertParam)params.get(i);
            try {
                if (evaluateExpressions.get(i) != null) {
                    if (null != evaluateExpressions.get(i)) switch (evaluateExpressions.get(i)) {
                        case LITERAL:
                            realData.add(resolvedExpressions.get(i));
                            break;
                        case ONLY_RESOLVE:
                            Integer index = dataSectionColumnIndexPosition[i];
                            switch (p.getDatatype()) {
                                case String:
                                    realData.add(ds.getData().getString(index));
                                    break;
                                case Boolean:
                                    realData.add(ds.getData().getBoolean(index));
                                    break;
                                case Byte:
                                    realData.add(ds.getData().getByte(index));
                                    break;
                                case Double:
                                    realData.add(ds.getData().getDouble(index));
                                    break;
                                case Float:
                                    realData.add(ds.getData().getFloat(index));
                                    break;
                                case Int:
                                    realData.add(ds.getData().getInt(index));
                                    break;
                                case Long:
                                    realData.add(ds.getData().getLong(index));
                                    break;
                                case BinaryStream:
                                    realData.add(ds.getData().getBinaryStream(index));
                                    break;
                                case Object:
                                    realData.add(ds.getData().getObject(index));
                                    break;
                                case CharacterStream:
                                    //DataSection getCharacterStream is not implemented
                                    realData.add(ds.getData().getString(index));
                                    break;
                                case Short:
                                    realData.add(ds.getData().getShort(index));
                                    break;
                                default:
                                    Logger.logAlertStateEvent(StdDebugLogMsg.sqlUnsupportedDatatype, new String[]{p.getDatatype().toString()});
                                    throw new ModuleException("Unsupported Data Type for: " + p.getDatatype().toString());
                            }
***** MISSING BREAK ******
                        case SCRIPT_EVALUATE:
                            realData.add(compiledScripts.get(i).eval());
                            break;
                        case RESOLVE_AND_SCRIPT_EVALUATE:
                            for (String s : p.getVariableList()) {
                                bindings.get(i).put(javaScriptVariableToFieldNameMap.get(s), ds.getData().getObject(s));
                            }   Object o = compiledScripts.get(i).eval();
                            for (String s : p.getVariableList()) {
                                bindings.get(i).remove(javaScriptVariableToFieldNameMap.get(s));
                            }   realData.add(o);
                            break;
                        default:
                            break;
                    } 
                }
            } catch (ModuleException | SQLException | ScriptException e) {
                throw new ModuleException("Error processing column:\n\t" + p.toString(), e);
            }           
        }
        return new DBEntry(realData,ds.getData().getRow());
    }
Comment 1 Svata Dedic 2016-05-23 15:59:53 UTC
Couldn't you prepare some fragment of code to test ? I've tried an +- equivalent structure in dev trunk and it seems to work, although the break (which is missing in your case) is wrongly indented.
Comment 2 ibeaumont 2016-05-24 09:25:57 UTC
Ok, here is simplified test that shows the problem.




package test;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class FormatIssue {

    public enum Day {
        SUNDAY, MONDAY, TUESDAY, WEDNESDAY,
        THURSDAY, FRIDAY, SATURDAY
    }

    public class DayHolder {

        Day day;

        Day getDay() {
            return day;
        }
    }

    public void test(Map<Integer, Day> evaluateExpressions) throws Exception {
        List<DayHolder> params = new ArrayList<>();
        for (int i = 0; i < params.size(); i++) {
            DayHolder p = params.get(i);
            if (evaluateExpressions.get(i) != null) {
                if (evaluateExpressions.get(i) == Day.SUNDAY) {
                    // Do something 1
                } else if (evaluateExpressions.get(i) == Day.MONDAY) {
                    switch (p.getDay()) {
                        case SUNDAY:
                            break;
                        default:
                            throw new Exception("xxx");
                    }
                } else if (evaluateExpressions.get(i) == Day.TUESDAY) {
                    // Do something 2
                }
            }
        }
    }
}
Comment 3 Svata Dedic 2016-05-24 14:14:28 UTC
Great thanks for the example. The issue is a little more serious just that one hint - the IDE's idea of whether the execution escapes (throws, returns or breaks out to an outer statement) from a given piece of code produces incorrect results. Possibly affects refactoring or other hints.
Comment 4 Svata Dedic 2016-05-24 16:33:08 UTC
Fixed in jet-main#56de4b373b04
Comment 5 Quality Engineering 2016-05-26 01:58:10 UTC
Integrated into 'main-silver', will be available in build *201605260002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/56de4b373b04
User: Svata Dedic <sdedic@netbeans.org>
Log: #262167: fixed bad escape analysis causing if-to-switch not generate break. Added testcases