FunctionDepsFinder: Check for declarations inside another declaration #46
Conversation
libcextract/FunctionDepsFinder.hh
Outdated
| * Check if the same Decl was already added into the closure but for a | ||
| * different typename. | ||
| */ | ||
| TypedefDecl *insideRangeOfDecl(TypedefDecl *decl) |
There was a problem hiding this comment.
Please use Decl and not only TypedefDecl.
There was a problem hiding this comment.
There were some complaints from the compiler when I was using Decl, so I used the more specialized type. I'll check it tomorrow.
| { | ||
| SourceRange range = decl->getSourceRange(); | ||
|
|
||
| for (auto it = Dependencies.begin(); it != Dependencies.end(); ++it) { |
There was a problem hiding this comment.
This algorithm is O(n ²). If the closure is small, I think this is acceptable, but if we want to do this extensively we may be in a better shape to sort by SourceLocation something to reduce it to O(n log n).
There was a problem hiding this comment.
The idea in this case was to submit the solution to check with our CI. Also, for my current LP, it worked.
I'll think how this could be improved.
libcextract/FunctionDepsFinder.hh
Outdated
| if (cur_range == range) | ||
| continue; | ||
|
|
||
| if (cur_range.getBegin() == range.getBegin() && |
There was a problem hiding this comment.
You can use Contains_From_LineCol and get the Decl with larger Range.
There was a problem hiding this comment.
Cool, I'll use it
libcextract/FunctionDepsFinder.hh
Outdated
| SourceRange range = decl->getSourceRange(); | ||
|
|
||
| for (auto it = Dependencies.begin(); it != Dependencies.end(); ++it) { | ||
| if (TypedefDecl *cur_decl = dynamic_cast<TypedefDecl *>(*it)) { |
There was a problem hiding this comment.
Please avoid using dynamic_cast<type*> and use dyn_cast. This works on clang builds with RTTI disabled.
There was a problem hiding this comment.
I just copied this line from the FunctionDepsFinder :P
Thanks for the suggestion
|
So I addressed all your comments besides the algorithm. Also, I added a testcase, which was simple to craft. Tomorrow I plans to rethink the algorithm so we can have this merged. |
Closes: SUSE#44 Signed-off-by: Marcos Paulo de Souza <[email protected]>
Closes: #44