Open MaximoOliveira opened 3 years ago
Hi @MaximoOliveira
Thanks a lot for this detailed description of the issues.
About error 1:
It seems that the modification point is pointing to a spoon element that has a PartialSourcePosition. I'd say that the modification points should not point to such element. So, I would be necessary to add a check before creating a modif point. We can create a special GH issue to continue discussing about that bug.
About error 2:
I suspect that SpoonClassCompile.java might have a bug that causes this error message to be added
Probably yes, I think so, we can check that by loading that class using spoon without passing over Astor.
The code to improve in Astor would be: to avoid compiling a class contained into the loadClasses collection (i.e., those that are pointed by a modification point) but not affected by any operation. What do you think?
Thanks! Regards Matias
Hi @martinezmatias
We can create a special GH issue to continue discussing about that bug.
Sounds good, ill open a issue on this problem
Probably yes, I think so, we can check that by loading that class using spoon without passing over Astor.
I've created a test case for this error:
@Test
public void testClassDoesntCompile() throws MalformedURLException {
String math_32 = "/home/max/Desktop/tese/fork_max/astor/examples/math_32/src/main/java";
String junitJar = "/home/max/Desktop/tese/original_astor/astor/./examples/libs/junit-4.10.jar";
URL ulrJar = new File(junitJar).toURI().toURL();
String fastMathPath = "/home/max/Desktop/tese/fork_max/astor/examples/math_32/src/main/java/org/apache/commons/math3/util/FastMath.java";
URL urlFastMath = new File(fastMathPath).toURI().toURL();
URL[] cp = new URL[]{ulrJar,urlFastMath};
Launcher launcher = new Launcher();
launcher.addInputResource(math_32);
launcher.buildModel();
CtClass ctClass = (CtClass) launcher.getFactory().getModel().getAllTypes()
.stream().filter(ctType -> ctType.getSimpleName().equals("FastMath")).findFirst().orElse(null);
List<CtClass> classes = Collections.singletonList(ctClass);
SpoonClassCompiler spoonClassCompiler = new SpoonClassCompiler();
List<CtClass> ctClasses = new ArrayList<>(classes);
CompilationResult compilation2 = spoonClassCompiler.compile(ctClasses, cp);
assertTrue(compilation2.getErrorList().stream()
.anyMatch(error -> error.contains("cannot assign a value to final variable")));
}
That replicates the behaviour found at this method (when running on math_32): https://github.com/SpoonLabs/astor/blob/df11be14321b522b8deab115ef902ae8e097e1f5/src/main/java/fr/inria/astor/core/manipulation/bytecode/compiler/SpoonClassCompiler.java#L54
I'm not sure about the origin of this problem but a way of amending it would be to discard any error message that contains the following words: "cannot assign a value to final variable"
Like this (at SpoonClassCompiler.java line 55) :
@Override
public CompilationResult compile(ProgramVariant instance, URL[] cp) {
List<CtClass> ctClasses = new ArrayList<CtClass>(instance.getBuiltClasses().values());
CompilationResult compilation2 = this.compile(ctClasses, cp);
String invalidCompilationError = "cannot assign a value to final variable";
List<String> correctList = compilation2.getErrorList().stream()
.filter(error -> !error.contains(invalidCompilationError)).collect(Collectors.toList());
compilation2.setErrorList(correctList);
return compilation2;
}
This is not optimal but does the trick. Instead, it would be better to understand what is causing that error message to be added and prevent that from happening, but this could be hard to find.
The code to improve in Astor would be: to avoid compiling a class contained into the loadClasses collection (i.e., those that are pointed by a modification point) but not affected by any operation. What do you think?
This is true, but we would still need to prevent the error message "cannot assign a value to final variable" to be added to the error list, in case a loaded class that was modified has a final variable with a value assigned to it (which should be possible).
Do you have any suggestions on how to tackle the problem from "cannot assign a value to final variable", or do you think its ok to simply remove any error that contains these words?
Let me know what you think!
Hi @MaximoOliveira
Instead, it would be better to understand what is causing that error message to be added and prevent that from happening, but this could be hard to find. Do you have any suggestions on how to tackle the problem from "cannot assign a value to final variable", or do you think its ok to simply remove any error that contains these words?
Yes, probably we can upgrade the version of Spoon. If the problem persist, we can open an issue on Spoon. The bug will be exposed by a similar test as that one you have created for this issue.
This is true, but we would still need to prevent the error message "cannot assign a value to final variable" to be added to the error list, in case a loaded class that was modified has a final variable with a value assigned to it (which should be possible).
Yes, in any case, I think that it would be helpful to avoid compiling classes that are in the loaded collection but are not referenced by an Operation, even that is not necessarily related to this issue.
Regards Matias
Hi @martinezmatias
Yes, probably we can upgrade the version of Spoon. If the problem persist, we can open an issue on Spoon. The bug will be exposed by a similar test as that one you have created for this issue.
In this case the problem is not caused by spoon, there is a class named SpoonClassCompiler in Astor but it uses other logic to verify if a class compiles or not. For example it uses ''JavaXToolsCompiler".
If we use spoon then it behaves correctly and says there are no compilation errors:
@Test
public void testClassCompiles() throws MalformedURLException {
String math_32 = "/home/max/Desktop/tese/original_astor/astor/examples/math_32/src/main/java";
Launcher launcher = new Launcher();
launcher.addInputResource(math_32);
launcher.buildModel();
JDTBasedSpoonCompiler compiler = (JDTBasedSpoonCompiler) launcher.getModelBuilder();
List<CategorizedProblem> problems = compiler.getProblems();
assertTrue(problems.isEmpty());
}
Perhaps JDTBasedSpoonCompiler could be used to verify compilation errors in Astor instead of SpoonClassCompiler
What do you think?
Hello again,
I've create the following test case (Cardumen on Math_32):
and found 2 bugs.
1) The first bug is that the program crashes with the following error:
Which can be surpassed by removing the following line : https://github.com/SpoonLabs/astor/blob/df11be14321b522b8deab115ef902ae8e097e1f5/src/main/java/fr/inria/astor/core/solutionsearch/population/ProgramVariantFactory.java#L264
After removing this line the program will run and we are able to find the second bug: 2) The first program variant generated will modify the following part of code: The expression: remainingRegion.isEmpty() From class: AbstractSubHyperplane.java, Line:156
With the fix ingredient: The expression: (hyperplane == null) From class: AbstractRegion.java , Line:180
This program variant loads 23 classes and then checks for any error in one of these classes (FastMath.java happens to be in one of them):
And this modification should not give a compilation error, however it gives 10 errors in the class FastMath.java
The first compilation error it gives is:
And this class (FastMath.java) was not even changed in this program variant.
The thing is that Astor should not give an error at this class, as it was not changed and the error that it gives doesn't make sense as we can assign values to final variables.
Because there is at least 1 error in the list, Astor will discard this program variant as a possible solution when In fact it should not have any error.
This is a concerning bug as Astor will discard program variants that are possible solutions to a bug.
Do you have any idea of what might be causing these wrong errors to be added to the CompilationResult errorlist ? I suspect that SpoonClassCompile.java might have a bug that causes this error message to be added
Let me know if you have any questions or If there is anything that I could clarify better
Thank you!