assume unfound classes to be in the same package as the current class #57
Conversation
|
LGTM |
src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java
Outdated
Show resolved
Hide resolved
|
Professor, I have updated the codes. |
| // package as the current class | ||
| String packageName = classAndPackageMap.getOrDefault(type, currentPackage); | ||
| UnsolvedClass newClass = new UnsolvedClass(type, packageName); | ||
| newClass.addMethod(creationMethod); |
There was a problem hiding this comment.
This code should also call createUnsolvedClass (and then call addMethod on the result)
There was a problem hiding this comment.
as we've added updateMissingClass to the updateUnsolvedClass method, I think we can't call updateUnsolvedClass here because we need to add the method to the UnsolvedClass instance before updating the list of missing classes.
There was a problem hiding this comment.
we need to add the method to the UnsolvedClass instance before updating the list of missing classes
Why? The pointer is still valid, and I don't think there's any reason to believe that any computation will be done with the list of missing classes before the call to addMethod is executed (assuming a sequential Java semantics, but I'm okay with that assumption here given the way the JVM works).
There was a problem hiding this comment.
No, I am wrong: updateMissingClass does more computation than I expected. I was incorrectly thinking that it just updates a data structure; it also does computation over the list of methods.
Can you modify updateUnsolvedClass to take a varargs array of methods and add them all to the class before it calls updateMissingClass?
There was a problem hiding this comment.
Professor, I'm afraid that might make updateUnsolvedClass more complicated than it should be. The reason is that it will require an additional array as an argument whenever it is called, even if there is no methods added (such as in line 529 and 674). And even at line 704, while there is a method added, there is only one method added. So modifying updateUnsolvedClass to take an array of methods as an argument seems to be unnecessary.
There was a problem hiding this comment.
it will require an additional array as an argument whenever it is called
It will not, if you use the varargs feature as I suggested. It is designed for exactly this situation, and the resulting code should be simpler than what you currently have.
src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java
Outdated
Show resolved
Hide resolved
|
Professor, I have updated the codes based on your suggestions. Please take a look. |
There was a problem hiding this comment.
I implemented my suggestion. I think this is ready to merge, but I'd like @LoiNguyenCS to take one more look. Please do, and then if this looks good to you please merge.
No description provided.