[cfe-dev] DeclContext in matcher results

[Manuel Klimek]

On Wed, Jul 18, 2012 at 2:52 AM, Sam Panzer <panzer at google.com> wrote:
> It turns out that I'm only looking for conflicts in the unqualified name,
> since I will never introduce a qualified variable in the Init portion of a
> for loop.
>
> My current strategy is to grab the DeclContext from the loop's index or
> iterator variable and use that as a starting point to check for naming
> conflicts with existing variables. The difficult part is identifying when my
> own changes would conflict with themselves, which I accomplish by building
> up a reversed AST tree containing only Stmt nodes, so that I can ask for the
> parent of any Stmt. Ideally, this would be done by the AST or some other
> utility, though the data structure is only computed once per run of the
> migration tool.

Another use case for hasAncestor I guess...

>
> -Sam
>
>
> On Mon, Jul 16, 2012 at 1:48 AM, Manuel Klimek <klimek at google.com> wrote:
>>
>>
>> On Sat, Jul 14, 2012 at 3:13 AM, Sam Panzer <panzer at google.com> wrote:
>> > Hi everyone,
>> >
>> > When a tool introduces a new declaration or renames an old one, it will
>> > need
>> > to make sure that the existing semantics are preserved. In particular,
>> > it is
>> > possible for the new declaration to conflict with or shadow an existing
>> > one,
>> > which means the tool needs to be particularly careful.
>> >
>> > My first guess would be to check all enclosing and enclosed DeclContexts
>> > for
>> > declarations with the same identifier. Is this a reasonable strategy,
>> > and if
>> > so, would matchers be able to track the DeclContexts of their results?
>> > It
>> > gets even more interesting when the refactoring tool wants to introduce
>> > two
>> > separate declarations that might conflict with each other.
>>
>> If you have a declaration, you can traverse the decl-contexts from the
>> ast context. We have some snippets of code that we haven't upstreamed
>> yet, which do traversal of the decl-contexts, and are probably not
>> perfect yet. See below.
>>
>> I'd rather not have the matcher layer try to implement pulling
>> together additional information. I'd rather try to either get Sema
>> involved (like you suggested earlier) or add stuff somewhere into the
>> AST so we can retrieve it later; perhaps make that optional.
>>
>> Thoughts?
>> /Manuel
>>
>> inline bool HasSymbol(
>>     const clang::ASTContext& ast_context,
>>     const clang::DeclContext* context,
>>     const llvm::SmallVectorImpl<llvm::StringRef>& components,
>>     int component) {
>>   if (context == NULL) {
>>     return false;
>>   }
>>   if (component == components.size()) {
>>     // Found the symbol.
>>     return true;
>>   }
>>   clang::IdentifierInfo& identifier =
>>       ast_context.Idents.get(components[component]);
>>   clang::DeclContext::lookup_const_result result =
>> context->lookup(&identifier);
>>   for (clang::NamedDecl* const * it = result.first; it != result.second;
>> ++it) {
>>     if (HasSymbol(ast_context, clang::NamedDecl::castToDeclContext(*it),
>>                   components, component+1)) {
>>       return true;
>>     }
>>   }
>>   return false;
>> }
>> inline bool HasGlobalSymbol(
>>     const clang::ASTContext& ast_context, const string& symbol) {
>>   llvm::StringRef symbol_ref(symbol.c_str(), symbol.size());
>>   llvm::SmallVector<llvm::StringRef, 4> components;
>>   symbol_ref.split(components, "::");
>>   clang::DeclContext* decl = ast_context.getTranslationUnitDecl();
>>   return HasSymbol(ast_context, decl, components, 0);
>> }
>>
>> >
>> > Thanks!
>> >
>> > -Sam
>
>