Bugzilla – Full Text Bug Listing
| Summary: | NPE in CPPVisitor using Generate Getters and Setters | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Marc-André Laperle <malaperle> | ||||
| Component: | cdt-core | Assignee: | Markus Schorn <mschorn.eclipse> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Doug Schaefer <cdtdoug> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | ||||||
| Version: | 8.0 | ||||||
| Target Milestone: | 8.0.1 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Marc-André Laperle
Found an easier way to reproduce it:
#ifndef TESTNPE_H_
#define TESTNPE_H_
namespace NS {
template <class T>
class Test {
};
}
class Test{
::NS::Test<int> test;
};
#endif
Although it is problematic to call name-resolution on copied AST, I have fixed the NPE. Created attachment 199057 [details]
Set parent node to fix NPE
Hi Markus. From what I gather, the problem here is that getTranslationUnit will go through all parents until it finds a IASTranslationUnit but Generate Getters ans Setters (and other refactorings) don't set the parent node on the node to insert so this produces a NPE. Returning null will result in a CPPScopeProblem, no? So I think one potential solution would be to set the parent on the node that's being inserted by the ASTWriter (node.setParent). Since the ASTWriter has access to the parent node and the node being inserted, I think it makes sense to do it in ASTWriter instead of having the clients do it and then asserting to make sure they do. But doing that makes the AST become mix of frozen and not frozen AST. Is that why returning null is the better solution here?
Looks like this got committed to a 8_0 branch and that branch got pushed instead of cdt_8_0. Markus, can you double check your push settings? (In reply to comment #4) > Looks like this got committed to a 8_0 branch and that branch got pushed > instead of cdt_8_0. Markus, can you double check your push settings? Thanks for pointing this out, I have corrected my commit. (In reply to comment #3) > Created attachment 199057 [details] > Set parent node to fix NPE > Hi Markus. From what I gather, the problem here is that getTranslationUnit will > go through all parents until it finds a IASTranslationUnit but Generate Getters > ans Setters (and other refactorings) don't set the parent node on the node to > insert so this produces a NPE. Returning null will result in a CPPScopeProblem, > no? So I think one potential solution would be to set the parent on the node > that's being inserted by the ASTWriter (node.setParent). Since the ASTWriter > has access to the parent node and the node being inserted, I think it makes > sense to do it in ASTWriter instead of having the clients do it and then > asserting to make sure they do. But doing that makes the AST become mix of > frozen and not frozen AST. Is that why returning null is the better solution > here? Setting the parent node to the original AST creates a mess, we should not do that. For me, it does not make sense to resolve names in the copied AST. The semantics for such an operation are unclear, because the name resolution depends on how the copied AST will be used. We should probably explicitly prevent users from doing that (by throwing an exception). In the particular case the name resolution is done (as a hack), because the ICPPASTQualifiedName does not store the information whether the template keyword has been used for one of its segments. --> We should probably open two new bugs: One for preventing name-resolution on copied AST and another one for extending ICPPASTQualifiedName. Comment on attachment 199057 [details]
Set parent node to fix NPE
Thanks for the explanation, makes sense. I will open the two bugs.
|