Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
Several improvements to Code Completion plugin
Huki:
--- Quote from: ollydbg on June 21, 2014, 01:28:07 pm ---@Huki.
About parsing the target in a project. See an example.
A project have two targets: TargetA, TargetB.
TargetA is the active target, which contains two cpp file (A1.cpp, A2.cpp), TargetB contains two cpp file (B1.cpp, B2.cpp).
Current method: parse all the four files with the preprecessor directive from TargetA, see bool NativeParser::DoFullParsing(cbProject* project, ParserBase* parser).
Is this OK?
Or, is it better to only parse the files(A1.cpp, A2.cpp) belong to the active target? I don't have an idea.
--- End quote ---
Hmm, it would be a good optimization indeed. If a certain file is not going to be compiled then there isn't much point in parsing it. It would mean for example, if I have selected the Code Completion target, then with code such as project->GetBuildTarget(), I can only use "go to declaration".. "go to implementation" would fail. Anyway, I think it's the expected behavior since we are not supposed to modify the sdk code when in CC target, as per our example.
One caveat is, what happens when a non-parsed file is activated in the editor?
- If the file does not belong to any project, it is added to a special parser for standalone files and parsed.
- If the file belongs to the current project nothing would happen (currently it is assumed that a file belonging to a project will always be parsed, so this case isn't covered).
Do you think this behavior needs to be changed? i.e., If a file belongs to the project but not part of the active target (and hence not parsed), should it be added to the standalone parser file's list if it is opened in the editor?
Anyway, if we want to try this optimization it appears to be simple: in DoFullParsing() we can replace project->GetFilesList() with:
1) project->GetBuildTarget(active_target)
2) Check if it's a real or virtual target
2a) If real, target->GetFilesList()
2b) If virtual, do according to the patch I submitted for collecting defines.
PS: I'm setting up git as we speak.. :)
ollydbg:
--- Quote from: Huki on June 21, 2014, 04:20:47 pm ---Hmm, it would be a good optimization indeed. If a certain file is not going to be compiled then there isn't much point in parsing it. It would mean for example, if I have selected the Code Completion target, then with code such as project->GetBuildTarget(), I can only use "go to declaration".. "go to implementation" would fail.
--- End quote ---
Yes, "go to implementation" would fail, if we only parse files belong to a single target(like Code Completion plugin target).
--- Quote ---Anyway, I think it's the expected behavior since we are not supposed to modify the sdk code when in CC target, as per our example.
--- End quote ---
But some user may ask that ""go to implementation" should still work. It's a good feature if we can quickly find the implementation.
Note, we already have an option: one parser for the whole workspace, which means one token tree holds all the tokens from every file in the workspace (a workspace may containing many projects), I can guess the reason is that we want a whole declaration/implementation cross reference between all the projects.
--- Quote ---One caveat is, what happens when a non-parsed file is activated in the editor?
- If the file does not belong to any project, it is added to a special parser for standalone files and parsed.
--- End quote ---
Yes, those files will be put in a special parser named "NON" parser.
--- Quote ---- If the file belongs to the current project nothing would happen (currently it is assumed that a file belonging to a project will always be parsed, so this case isn't covered).
--- End quote ---
Yes, if the new opened file is in the token tree, it won't be parsed, it may switch the parser if necessary.
--- Quote ---Do you think this behavior needs to be changed? i.e., If a file belongs to the project but not part of the active target (and hence not parsed), should it be added to the standalone parser file's list if it is opened in the editor?
--- End quote ---
I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.
--- Quote ---Anyway, if we want to try this optimization it appears to be simple: in DoFullParsing() we can replace project->GetFilesList() with:
1) project->GetBuildTarget(active_target)
2) Check if it's a real or virtual target
2a) If real, target->GetFilesList()
2b) If virtual, do according to the patch I submitted for collecting defines.
--- End quote ---
Yes, this is the way if we want to implement the "per target parsing".
Huki:
--- Quote from: ollydbg on June 21, 2014, 05:05:30 pm ---I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.
--- End quote ---
If the cross reference feature is needed, then I think it's better to keep the current system of full project reparse. Your new idea is almost good enough, but it assumes that the change in project macros list only affects the new target. Most of the time this is not true, eg., switching from Scintilla target to CC target: the macros used by scintilla target (such as SCI_LEXER) will be removed, so we need to reparse the old target too.
ollydbg:
--- Quote from: Huki on June 21, 2014, 07:41:34 pm ---
--- Quote from: ollydbg on June 21, 2014, 05:05:30 pm ---I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.
--- End quote ---
If the cross reference feature is needed, then I think it's better to keep the current system of full project reparse. Your new idea is almost good enough, but it assumes that the change in project macros list only affects the new target. Most of the time this is not true, eg., switching from Scintilla target to CC target: the macros used by scintilla target (such as SCI_LEXER) will be removed, so we need to reparse the old target too.
--- End quote ---
Agree, in some project, files were shared by several targets, this way, if a file is already parsed in targetA, it will never be parsed in targetB.
So, your two patches in
--- Quote from: Huki on June 18, 2014, 09:10:27 pm ---Thanks for the commit.
Two patches for build targets parsing (i.e., project defined macros):
...
--- End quote ---
are not needed, right? Unless we find a better way.
Huki:
--- Quote from: ollydbg on June 23, 2014, 03:53:44 am ---So, your two patches in
--- Quote from: Huki on June 18, 2014, 09:10:27 pm ---Thanks for the commit.
Two patches for build targets parsing (i.e., project defined macros):
...
--- End quote ---
are not needed, right? Unless we find a better way.
--- End quote ---
Hmm, the first patch is needed IMO (i.e, "When a virtual target is selected, collect the defines for all child targets."). In the CB project for example, check the parts covered by SCI_LEXER macro in ScintillaBase.cxx (sdk\wxscintilla\src\scintilla\src\ScintillaBase.cxx). With the "All" virtual target, SCI_LEXER won't be defined, so the code covered by it won't be parsed by CC. I think when a virtual target is chosen, it is better to collect the defines set for the virtual target, and also all the defines set for its child targets.
The second patch is more of a performance concern. Most projects have a Debug and Release target (where a DEBUG macro is defined for the Debug target). In this case when the target is changed, the user expects that the DEBUG code is enabled or disabled accordingly, which means the full project needs to be reparsed with the new macro state. Personally I don't mind the time taken for full project reparse, so I'd suggest committing the second patch too. :)
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version