Bugzilla – Full Text Bug Listing
| Summary: | NullPointerException when inserting pragma into CDT AST | ||
|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | pl |
| Component: | cdt-core | Assignee: | Project Inbox <cdt-core-inbox> |
| Status: | NEW --- | QA Contact: | Jonah Graham <jonah> |
| Severity: | minor | ||
| Priority: | P3 | CC: | eclipse.sprigogin, thomas.corbat |
| Version: | 7.0 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Mac OS X - Carbon (unsup.) | ||
| See Also: | https://git.eclipse.org/r/52955 | ||
| Whiteboard: | |||
|
Description
pl
I could not reproduce the bug as reported. The NullPointerException has been fixed in the following commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=ca61147d16a313e1ea8a8376f97626d9475d7b95 But the modification still does not work as expected - it is just ignored. Because only IASTStatements are expected as children of a compound statement the ASTLiteralNode gets filtered when assembling the modified child array. A possible workaround for the user of this infrastructure was to derive his own ASTNodeLiteral type, which satisfies the required interface. In the case at hand IASTStatement. I don't think this would be a very intuitive solution. The alternative I see was add the required interfaces for such cases to the ASTLiteralNode (not possible due to copy() signature). For the assambling of modified child arrays we would need the following interfaces for the known calls in CDT: - IASTDeclaration - IASTStatement - IASTArrayModifier - ICPPASTConstructorChainInitializer - IASTTypeId - IASTExpression - IASTName - IASTParameterDeclaration - IASTPointerOperator On the other hand I think it is highly unlikely that we need other cases than IASTStatements and IASTDeclaration, which actually is (at least in the cases here) just a lack of support for preprocessor directives in the rewrite infrastructure. New Gerrit change created: https://git.eclipse.org/r/52955 (In reply to Thomas Corbat from comment #1) > I could not reproduce the bug as reported. The NullPointerException has been > fixed in the following commit: > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > ?id=ca61147d16a313e1ea8a8376f97626d9475d7b95 > > But the modification still does not work as expected - it is just ignored. > Because only IASTStatements are expected as children of a compound statement > the ASTLiteralNode gets filtered when assembling the modified child array. > > A possible workaround for the user of this infrastructure was to derive his > own ASTNodeLiteral type, which satisfies the required interface. In the case > at hand IASTStatement. I don't think this would be a very intuitive solution. > > The alternative I see was add the required interfaces for such cases to the > ASTLiteralNode (not possible due to copy() signature). For the assambling of > modified child arrays we would need the following interfaces for the known > calls in CDT: > - IASTDeclaration > - IASTStatement > - IASTArrayModifier > - ICPPASTConstructorChainInitializer > - IASTTypeId > - IASTExpression > - IASTName > - IASTParameterDeclaration > - IASTPointerOperator > > On the other hand I think it is highly unlikely that we need other cases > than IASTStatements and IASTDeclaration, which actually is (at least in the > cases here) just a lack of support for preprocessor directives in the > rewrite infrastructure. The proposed change looks like a huge hack. If we want to support preprocessor directives in ASTRewrite, we should do so explicitly, not by trying to impersonate regular AST nodes. Since the preprocessor directives no not participate in regular node hierarchy, they should be inserted by file offset, not into the node hierarchy. (In reply to Sergey Prigogin from comment #3) > (In reply to Thomas Corbat from comment #1) > > ... > The proposed change looks like a huge hack. If we want to support > preprocessor directives in ASTRewrite, we should do so explicitly, not by > trying to impersonate regular AST nodes. Since the preprocessor directives > no not participate in regular node hierarchy, they should be inserted by > file offset, not into the node hierarchy. I see the objection and I'm not completely happy with that approach either. We actually have two problems here: - Lacking support for insertion of preprocessor directives. If I recall correctly this has come up more than once regarding include directives. Pragmas is just another case. It would be nice if we had an approach which works with the ASTRewrite infrastructure. While they are not part of the AST the ASTRewrite is still the proposed way of performing code transformation. An additional offset-based change might be a hassle for the transformation developer to cope with - at least if the preprocessor directive modification is part of a wider transformation. - Support of ASTLiteralNode as container for arbitrary code. Using this node for pragmas is just a case where we currently cannot offer an alternative solution (beside TextEditChanges). From its description I derive that it should be usable for all kind of code. Insertion into compound statements is still not possible. It's arguable whether we really need that. I guess we do. A transformation implementer might want to insert an object-macro expansion which does not have a corresponding representation in the AST: { MACRO // Insert MACRO } Eventually, the cases which a literal node is really required might always be preprocessor related. If we can come up with a solution for all preprocessor stuff I'm really happy. I'm just a bit stuck in finding a feasible solution. This bug was assigned and targeted at a now released milestone. As that milestone has now passed, the milestone field has been cleared. If this bug has been fixed, please set the milestone to the version it was fixed in and marked the bug as resolved. |