DeclContext in matcher results

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.

Thanks!

-Sam

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);
}

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.

-Sam

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...