Community
Participate
Working Groups
For refactoring and AST-rewriting additional information about the preprocessor directives are needed. This was part of my C++ Refactoring enhancement report. https://bugs.eclipse.org/bugs/show_bug.cgi?id=181493 As recommended by Markus Schorn I split the original patch into smaller pieces. I also replaced all the Strings and Vectors.
Created attachment 64142 [details] changes for org.eclipse.cdt.core
Created attachment 64143 [details] Additional Tests
Emanuel, I had a look at the patch and see two issues with it: * it is legal to use spaces in directives: '# ifdef x', therefore the changes in DOMScanner.java are not correct. * the patch adds the method IASTPreprocessorMacroDefinition.getExpansions(): There is already a way to obtain the expansion locations for a macro def. You should use IBinding binding= macro.getName().resolveBinding(); IASTName[] ref= ast.getReferences(binding); IASTNodeLocation[] locs= ref[0].getNodeLocations(); If that does not work correctly we should fix it, rather than providing a second possibility to find the same information.
Created attachment 64303 [details] revised patch Markus, I didn't now that spaces are allowed there. I improved the patch and removed the Expansionslocations.
Created attachment 64304 [details] Unit Tests I added test for the "spaces issue".
Emanuel, thanks for the update! Please update the copyright notices, such that they show the year 2007, also you should add your name to the contributor list. I have an issue with the equals() method added somewhere in LocationMap.java. If you implement it, you'd need to implement hashCode() as well. None of the other classes in there implement equals(), so I'd remove it. Other than that I am willing to apply the patch.
Created attachment 64435 [details] patch I updated the copyright headers and removed the equals method.
Created attachment 64436 [details] Unit Tests Updated the copyright headers.
Thanks Emanuel, I have applied the patch.
Ouch, this patch broke my integration between the new C99 preprocessor and LocationMap. Its no big deal, I just have to pass the new parameters that were added to the methods in IScannerPreprocessorLog. What is the new Object[] array parameter that was added to IScannerPreprocessorLog.startFunctionStyleExpansion() supposed to be? The type is just Object and there are no documentation comments so I'm not sure what to pass here. Also, several of the non-Javadoc comments in LocationMap are now out of sync, the comments do not mention the new parameters. What is the point of these non-Javadoc comments anyways? They add no information and they are prone to going out of sync.
The array contains the actual parameter. The type Object[] is a product of the CharArrayObjectMap used in the BaseScanner to store those values.
Created attachment 64644 [details] adds a test to the test suite for macro expansion arguments > The array contains the actual parameter. The type Object[] is a product of the > CharArrayObjectMap used in the BaseScanner to store those values. Ok, I see now how this works. I have attached a small patch that adds a test for this. Please verify that it is correct. I was able to make the necessary changes to the C99 preprocessor quite easily. At some point someone is going to have to cast Object[] back to char[][] in order to get at the data. I would argue that this is better done inside BaseScanner than by a client, especially since casting multidimensional arrays is so ugly in Java. What do other people think?
> At some point someone is going to have to cast Object[] back to char[][] in > order to get at the data. I would argue that this is better done inside > BaseScanner than by a client, especially since casting multidimensional arrays > is so ugly in Java. What do other people think? Mike you are right, I have changed the API, such that it requires to provide char[][] and gives you back the char[][]. The affected methods: * void IScannerPreprocessorLog.startFunctionStyleExpansion( IMacroDefinition macro, char[][] parameters, int startOffset, int endOffset, char[][] actualArguments); * char[][] MacroExpansionLocation.getActualParameters(); Thanks for the testcase.
(In reply to comment #12) > At some point someone is going to have to cast Object[] back to char[][] in > order to get at the data. I would argue that this is better done inside > BaseScanner than by a client, especially since casting multidimensional arrays > is so ugly in Java. What do other people think? > Your right it's not the best solution char[][] would be better. I can do a patch for this. But Markus was faster. :)
Thanks Markus for making the changes.
