[Investigation] The relationship between the compile policies and the stop policies

[Maurizio Cimadamore]

I think your analysis is correct.

The issue started to appear when we removed and consolidated unused 
hidden option (JDK-8148808).

But the general issue is that policies such as ATTR_ONLY and CHECK_ONLY 
_predate_ the shouldStop options, with which they do overlap significantly.

I believe that policies such ATTR_ONLY (stops after Attr) and CHECK_ONLY 
(stops after Flow) should be removed - and shouldStop should be used 
instead, since it's equally expressive.

In the future, we might want to evaluate whether there is a need for a 
"dry-run" option, to run javac, emit all possible checks but don't 
generate any classfiles - but for now shouldStop should be usable in 
that respect.

Maurizio


On 09/05/2021 12:02, Guoxiong Li wrote:
> Hi all,
>
> When I read the code of the 
> `com.sun.tools.javac.main.JavaCompiler#JavaCompiler(Context context)`, 
> I found the following strange code.
>
> ```
>         if (options.isSet("should-stop.at <http://should-stop.at>") &&
>             CompileState.valueOf(options.get("should-stop.at 
> <http://should-stop.at>")) == CompileState.ATTR)
>             compilePolicy = CompilePolicy.ATTR_ONLY;
>         else
>             compilePolicy = 
> CompilePolicy.decode(options.get("compilePolicy"));
> ```
>
> It means that if the `should-stop.at <http://should-stop.at>` is 
> `ATTR`, the compile policy will be `ATTR_ONLY`. It may be not right. 
> Because `should-stop.at <http://should-stop.at>` is the same as 
> `should-stop.ifError`. If no error occurs, the compilation should 
> continue. But the `ATTR_ONLY` means that the compilation always 
> finishes after `ATTR`. Therefore, I think the `should-stop.at 
> <http://should-stop.at>=ATTR` is not equal to `compilePolicy=ATTR_ONLY`.
>
> I searched the submit log of these code. I found these codes were 
> submitted at JDK-8148808 [1]. Please see the following snippet of the 
> patch[2] of JDK-8148808.
>
> ```
> - attrParseOnly = options.isSet("-attrparseonly");
> encoding = options.get(ENCODING);
> lineDebugInfo = options.isUnset(G_CUSTOM) ||
> options.isSet(G_CUSTOM, "lines");
> @@ -405,7 +403,8 @@
>
> verboseCompilePolicy = options.isSet("verboseCompilePolicy");
>
> - if (attrParseOnly)
> + if (options.isSet("shouldStopPolicy") &&
> + CompileState.valueOf(options.get("shouldStopPolicy")) == 
> CompileState.ATTR)
> compilePolicy = CompilePolicy.ATTR_ONLY;
> else
> compilePolicy = CompilePolicy.decode(options.get("compilePolicy"));
> ```
>
> It removed the option `-attrparseonly` and used `shouldStopPolicy` to 
> replace `-attrparseonly`.(FYI: `shouldStopPolicy` was changed to 
> `should-stop.at <http://should-stop.at>` in another patch.)
> Based on the logic of the code above, the original developer might 
> think that the following two expressions are the same:
>
> *1. options.isSet("-attrparseonly")*
> *2. options.isSet("shouldStopPolicy") *
> *    && CompileState.valueOf(options.get("shouldStopPolicy")) == 
> CompileState.ATTR)*
>
> Actually it is not the same. The first expression may be same as the 
> following expression:
>
> *options.isSet("should-stop.ifError") *
> *&& CompileState.valueOf(options.get("should-stop.ifError")) == 
> CompileState.ATTR)*
> *&& options.isSet("should-stop.ifNoError") *
> *&& CompileState.valueOf(options.get("should-stop.ifNoError")) == 
> CompileState.ATTR)*
>
> Anyway, I don't think it is a good idea to mix the compile policies 
> and the stop policies. So we should only keep this line:
>
> ```
>             compilePolicy = 
> CompilePolicy.decode(options.get("compilePolicy"));
> ```
>
> and remove the following codes:
>
> ```
>         if (options.isSet("should-stop.at <http://should-stop.at>") &&
>             CompileState.valueOf(options.get("should-stop.at 
> <http://should-stop.at>")) == CompileState.ATTR)
>             compilePolicy = CompilePolicy.ATTR_ONLY;
>         else
> ```
>
> What's your opinion? Especially, should we mix the compile policies 
> and the stop policies? Should there be a relationship between the 
> compile policies and the stop policies?
> Any idea is appreciated. Thank you.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8148808 
> <https://bugs.openjdk.java.net/browse/JDK-8148808>
> [2] http://hg.openjdk.java.net/jdk9/jdk9/langtools/rev/645b5debcb07 
> <http://hg.openjdk.java.net/jdk9/jdk9/langtools/rev/645b5debcb07>
>
> Best Regards,
> -- Guoxiong
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210510/d716f7bb/attachment.htm>