[cfe-commits] r82476 - in /cfe/trunk: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/member-access.cpp

Daniel Dunbar daniel at zuster.org
Sat Sep 26 09:32:03 PDT 2009


Hi Doug,

There is some kind of non-determinism in the member-access.cpp test, I
think something depends on the order of pointers most likely.

On Darwin10 I get this output:
--
operator= : 0 : operator=(<#struct Derived const &#>)
~Base1 : 0 : [#Base1::#]~Base1()
~Base2 : 0 : [#Base2::#]~Base2()
~Base3 : 0 : [#Base3::#]~Base3()
~Derived : 0 : ~Derived()
member1 : 0 : [#Base1::#]member1
member1 : 0 : [#Base2::#]member1
member2 : 0 : [#Base1::#]member2
member3 : 0 : [#Base2::#]member3
member4 : 0
memfun1 : 0 : [#Base3::#]memfun1(<#float#>)
memfun1 : 0 : [#Base3::#]memfun1(<#double#>)
memfun2 : 0 : [#Base3::#]memfun2(<#int#>)
memfun3 : 0 : memfun3(<#int#>)
Base1 : 0 : Base1::
Base2 : 0 : Base2::
Base3 : 0 : Base3::
Derived : 0 : Derived::
operator= : 0 (Hidden) : Base3::operator=(<#struct Base3 const &#>)
operator= : 0 (Hidden) : Base1::operator=(<#struct Base1 const &#>)
operator= : 0 (Hidden) : Base2::operator=(<#struct Base2 const &#>)
memfun1 : 0 (Hidden) : Base2::memfun1(<#int#>)
Proxy : 3 : Proxy::
--

but on Linux ~Base2, ~Base3, and ~Derived appear after
"memfun3(<#int#>)", which breaks the test.

Can you take a look?

 - Daniel

On Mon, Sep 21, 2009 at 1:12 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Mon Sep 21 15:12:40 2009
> New Revision: 82476
>
> URL: http://llvm.org/viewvc/llvm-project?rev=82476&view=rev
> Log:
> When providing a code-completion suggestion for a hidden name, include
> a nested-name-specifier that describes how to refer to that name. For
> example, given:
>
>  struct Base { int member; };
>  struct Derived : Base { int member; };
>
> the code-completion result for a member access into "Derived" will
> provide both "member" to refer to Derived::member (no qualification needed) and
> "Base::member" to refer to Base::member (qualification included).
>
>
> Modified:
>    cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>    cfe/trunk/test/CodeCompletion/member-access.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=82476&r1=82475&r2=82476&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Sep 21 15:12:40 2009
> @@ -86,7 +86,11 @@
>     /// \brief Add a new result to this result set (if it isn't already in one
>     /// of the shadow maps), or replace an existing result (for, e.g., a
>     /// redeclaration).
> -    void MaybeAddResult(Result R);
> +    ///
> +    /// \param R the result to add (if it is unique).
> +    ///
> +    /// \param R the context in which this result will be named.
> +    void MaybeAddResult(Result R, DeclContext *CurContext = 0);
>
>     /// \brief Enter into a new scope.
>     void EnterNewScope();
> @@ -133,14 +137,57 @@
>   if (HiddenCtx->isFunctionOrMethod())
>     return false;
>
> -  // If the hidden and visible declarations are in different name-lookup
> -  // contexts, then we can qualify the name of the hidden declaration.
> -  // FIXME: Optionally compute the string needed to refer to the hidden
> -  // name.
>   return HiddenCtx != Visible->getDeclContext()->getLookupContext();
>  }
>
> -void ResultBuilder::MaybeAddResult(Result R) {
> +/// \brief Compute the qualification required to get from the current context
> +/// (\p CurContext) to the target context (\p TargetContext).
> +///
> +/// \param Context the AST context in which the qualification will be used.
> +///
> +/// \param CurContext the context where an entity is being named, which is
> +/// typically based on the current scope.
> +///
> +/// \param TargetContext the context in which the named entity actually
> +/// resides.
> +///
> +/// \returns a nested name specifier that refers into the target context, or
> +/// NULL if no qualification is needed.
> +static NestedNameSpecifier *
> +getRequiredQualification(ASTContext &Context,
> +                         DeclContext *CurContext,
> +                         DeclContext *TargetContext) {
> +  llvm::SmallVector<DeclContext *, 4> TargetParents;
> +
> +  for (DeclContext *CommonAncestor = TargetContext;
> +       CommonAncestor && !CommonAncestor->Encloses(CurContext);
> +       CommonAncestor = CommonAncestor->getLookupParent()) {
> +    if (CommonAncestor->isTransparentContext() ||
> +        CommonAncestor->isFunctionOrMethod())
> +      continue;
> +
> +    TargetParents.push_back(CommonAncestor);
> +  }
> +
> +  NestedNameSpecifier *Result = 0;
> +  while (!TargetParents.empty()) {
> +    DeclContext *Parent = TargetParents.back();
> +    TargetParents.pop_back();
> +
> +    if (NamespaceDecl *Namespace = dyn_cast<NamespaceDecl>(Parent))
> +      Result = NestedNameSpecifier::Create(Context, Result, Namespace);
> +    else if (TagDecl *TD = dyn_cast<TagDecl>(Parent))
> +      Result = NestedNameSpecifier::Create(Context, Result,
> +                                           false,
> +                                     Context.getTypeDeclType(TD).getTypePtr());
> +    else
> +      assert(Parent->isTranslationUnit());
> +  }
> +
> +  return Result;
> +}
> +
> +void ResultBuilder::MaybeAddResult(Result R, DeclContext *CurContext) {
>   if (R.Kind != Result::RK_Declaration) {
>     // For non-declaration results, just add the result.
>     Results.push_back(R);
> @@ -149,7 +196,8 @@
>
>   // Look through using declarations.
>   if (UsingDecl *Using = dyn_cast<UsingDecl>(R.Declaration))
> -    return MaybeAddResult(Result(Using->getTargetDecl(), R.Rank));
> +    return MaybeAddResult(Result(Using->getTargetDecl(), R.Rank, R.Qualifier),
> +                          CurContext);
>
>   // Handle each declaration in an overload set separately.
>   if (OverloadedFunctionDecl *Ovl
> @@ -157,7 +205,7 @@
>     for (OverloadedFunctionDecl::function_iterator F = Ovl->function_begin(),
>          FEnd = Ovl->function_end();
>          F != FEnd; ++F)
> -      MaybeAddResult(Result(*F, R.Rank));
> +      MaybeAddResult(Result(*F, R.Rank, R.Qualifier), CurContext);
>
>     return;
>   }
> @@ -232,6 +280,11 @@
>                                  I->second.first)) {
>         // Note that this result was hidden.
>         R.Hidden = true;
> +
> +        if (!R.Qualifier)
> +          R.Qualifier = getRequiredQualification(SemaRef.Context,
> +                                                 CurContext,
> +                                              R.Declaration->getDeclContext());
>       } else {
>         // This result was hidden and cannot be found; don't bother adding
>         // it.
> @@ -329,53 +382,6 @@
>   return 0;
>  }
>
> -/// \brief Compute the qualification required to get from the current context
> -/// (\p CurContext) to the target context (\p TargetContext).
> -///
> -/// \param Context the AST context in which the qualification will be used.
> -///
> -/// \param CurContext the context where an entity is being named, which is
> -/// typically based on the current scope.
> -///
> -/// \param TargetContext the context in which the named entity actually
> -/// resides.
> -///
> -/// \returns a nested name specifier that refers into the target context, or
> -/// NULL if no qualification is needed.
> -static NestedNameSpecifier *
> -getRequiredQualification(ASTContext &Context,
> -                         DeclContext *CurContext,
> -                         DeclContext *TargetContext) {
> -  llvm::SmallVector<DeclContext *, 4> TargetParents;
> -
> -  for (DeclContext *CommonAncestor = TargetContext;
> -       CommonAncestor && !CommonAncestor->Encloses(CurContext);
> -       CommonAncestor = CommonAncestor->getLookupParent()) {
> -    if (CommonAncestor->isTransparentContext() ||
> -        CommonAncestor->isFunctionOrMethod())
> -      continue;
> -
> -    TargetParents.push_back(CommonAncestor);
> -  }
> -
> -  NestedNameSpecifier *Result = 0;
> -  while (!TargetParents.empty()) {
> -    DeclContext *Parent = TargetParents.back();
> -    TargetParents.pop_back();
> -
> -    if (NamespaceDecl *Namespace = dyn_cast<NamespaceDecl>(Parent))
> -      Result = NestedNameSpecifier::Create(Context, Result, Namespace);
> -    else if (TagDecl *TD = dyn_cast<TagDecl>(Parent))
> -      Result = NestedNameSpecifier::Create(Context, Result,
> -                                           false,
> -                                    Context.getTypeDeclType(TD).getTypePtr());
> -    else
> -      assert(Parent->isTranslationUnit());
> -  }
> -
> -  return Result;
> -}
> -
>  /// \brief Collect the results of searching for members within the given
>  /// declaration context.
>  ///
> @@ -395,7 +401,8 @@
>  /// names within this declaration context.
>  static unsigned CollectMemberLookupResults(DeclContext *Ctx,
>                                            unsigned InitialRank,
> -                                           llvm::SmallPtrSet<DeclContext *, 16> &Visited,
> +                                           DeclContext *CurContext,
> +                                 llvm::SmallPtrSet<DeclContext *, 16> &Visited,
>                                            ResultBuilder &Results) {
>   // Make sure we don't visit the same context twice.
>   if (!Visited.insert(Ctx->getPrimaryContext()))
> @@ -409,7 +416,8 @@
>          DEnd = CurCtx->decls_end();
>          D != DEnd; ++D) {
>       if (NamedDecl *ND = dyn_cast<NamedDecl>(*D))
> -        Results.MaybeAddResult(CodeCompleteConsumer::Result(ND, InitialRank));
> +        Results.MaybeAddResult(CodeCompleteConsumer::Result(ND, InitialRank),
> +                               CurContext);
>     }
>   }
>
> @@ -452,6 +460,7 @@
>       NextRank = std::max(NextRank,
>                           CollectMemberLookupResults(Record->getDecl(),
>                                                      InitialRank + 1,
> +                                                     CurContext,
>                                                      Visited,
>                                                      Results));
>     }
> @@ -479,9 +488,11 @@
>  /// names within this declaration context.
>  static unsigned CollectMemberLookupResults(DeclContext *Ctx,
>                                            unsigned InitialRank,
> +                                           DeclContext *CurContext,
>                                            ResultBuilder &Results) {
>   llvm::SmallPtrSet<DeclContext *, 16> Visited;
> -  return CollectMemberLookupResults(Ctx, InitialRank, Visited, Results);
> +  return CollectMemberLookupResults(Ctx, InitialRank, CurContext, Visited,
> +                                    Results);
>  }
>
>  /// \brief Collect the results of searching for declarations within the given
> @@ -492,10 +503,13 @@
>  /// \param InitialRank the initial rank given to results in this scope.
>  /// Larger rank values will be used for results found in parent scopes.
>  ///
> +/// \param CurContext the context from which lookup results will be found.
> +///
>  /// \param Results the builder object that will receive each result.
>  static unsigned CollectLookupResults(Scope *S,
>                                      TranslationUnitDecl *TranslationUnit,
>                                      unsigned InitialRank,
> +                                     DeclContext *CurContext,
>                                      ResultBuilder &Results) {
>   if (!S)
>     return InitialRank;
> @@ -517,7 +531,8 @@
>       if (Ctx->isFunctionOrMethod())
>         continue;
>
> -      NextRank = CollectMemberLookupResults(Ctx, NextRank + 1, Results);
> +      NextRank = CollectMemberLookupResults(Ctx, NextRank + 1, CurContext,
> +                                            Results);
>     }
>   } else if (!S->getParent()) {
>     // Look into the translation unit scope. We walk through the translation
> @@ -531,13 +546,14 @@
>     // in DeclContexts unless we have to" optimization), we can eliminate the
>     // TranslationUnit parameter entirely.
>     NextRank = CollectMemberLookupResults(TranslationUnit, NextRank + 1,
> -                                          Results);
> +                                          CurContext, Results);
>   } else {
>     // Walk through the declarations in this Scope.
>     for (Scope::decl_iterator D = S->decl_begin(), DEnd = S->decl_end();
>          D != DEnd; ++D) {
>       if (NamedDecl *ND = dyn_cast<NamedDecl>((Decl *)((*D).get())))
> -        Results.MaybeAddResult(CodeCompleteConsumer::Result(ND, NextRank));
> +        Results.MaybeAddResult(CodeCompleteConsumer::Result(ND, NextRank),
> +                               CurContext);
>     }
>
>     NextRank = NextRank + 1;
> @@ -545,7 +561,7 @@
>
>   // Lookup names in the parent scope.
>   NextRank = CollectLookupResults(S->getParent(), TranslationUnit, NextRank,
> -                                  Results);
> +                                  CurContext, Results);
>   Results.ExitScope();
>
>   return NextRank;
> @@ -882,7 +898,8 @@
>   unsigned NextRank = 0;
>
>   if (const RecordType *Record = BaseType->getAs<RecordType>()) {
> -    NextRank = CollectMemberLookupResults(Record->getDecl(), NextRank, Results);
> +    NextRank = CollectMemberLookupResults(Record->getDecl(), NextRank,
> +                                          Record->getDecl(), Results);
>
>     if (getLangOptions().CPlusPlus) {
>       if (!Results.empty()) {
> @@ -906,7 +923,7 @@
>       // results as well.
>       Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
>       CollectLookupResults(S, Context.getTranslationUnitDecl(), NextRank,
> -                           Results);
> +                           CurContext, Results);
>     }
>
>     // Hand off the results found for code completion.
> @@ -944,14 +961,14 @@
>
>   ResultBuilder Results(*this, Filter);
>   unsigned NextRank = CollectLookupResults(S, Context.getTranslationUnitDecl(),
> -                                           0, Results);
> +                                           0, CurContext, Results);
>
>   if (getLangOptions().CPlusPlus) {
>     // We could have the start of a nested-name-specifier. Add those
>     // results as well.
>     Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
>     CollectLookupResults(S, Context.getTranslationUnitDecl(), NextRank,
> -                         Results);
> +                         CurContext, Results);
>   }
>
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
> @@ -1045,7 +1062,7 @@
>     return;
>
>   ResultBuilder Results(*this);
> -  unsigned NextRank = CollectMemberLookupResults(Ctx, 0, Results);
> +  unsigned NextRank = CollectMemberLookupResults(Ctx, 0, Ctx, Results);
>
>   // The "template" keyword can follow "::" in the grammar, but only
>   // put it into the grammar if the nested-name-specifier is dependent.
> @@ -1068,7 +1085,8 @@
>
>   // After "using", we can see anything that would start a
>   // nested-name-specifier.
> -  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0, Results);
> +  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0,
> +                       CurContext, Results);
>
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
>  }
> @@ -1080,7 +1098,8 @@
>   // After "using namespace", we expect to see a namespace name or namespace
>   // alias.
>   ResultBuilder Results(*this, &ResultBuilder::IsNamespaceOrAlias);
> -  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0, Results);
> +  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0, CurContext,
> +                       Results);
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
>  }
>
> @@ -1109,7 +1128,8 @@
>     for (std::map<NamespaceDecl *, NamespaceDecl *>::iterator
>          NS = OrigToLatest.begin(), NSEnd = OrigToLatest.end();
>          NS != NSEnd; ++NS)
> -      Results.MaybeAddResult(CodeCompleteConsumer::Result(NS->second, 0));
> +      Results.MaybeAddResult(CodeCompleteConsumer::Result(NS->second, 0),
> +                             CurContext);
>   }
>
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
> @@ -1121,7 +1141,8 @@
>
>   // After "namespace", we expect to see a namespace or alias.
>   ResultBuilder Results(*this, &ResultBuilder::IsNamespaceOrAlias);
> -  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0, Results);
> +  CollectLookupResults(S, Context.getTranslationUnitDecl(), 0, CurContext,
> +                       Results);
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
>  }
>
> @@ -1140,7 +1161,7 @@
>
>   // Add any type names visible from the current scope
>   unsigned NextRank = CollectLookupResults(S, Context.getTranslationUnitDecl(),
> -                                           0, Results);
> +                                           0, CurContext, Results);
>
>   // Add any type specifiers
>   AddTypeSpecifierResults(getLangOptions(), 0, Results);
> @@ -1148,7 +1169,7 @@
>   // Add any nested-name-specifiers
>   Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
>   CollectLookupResults(S, Context.getTranslationUnitDecl(), NextRank + 1,
> -                       Results);
> +                       CurContext, Results);
>
>   HandleCodeCompleteResults(CodeCompleter, Results.data(), Results.size());
>  }
>
> Modified: cfe/trunk/test/CodeCompletion/member-access.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/member-access.cpp?rev=82476&r1=82475&r2=82476&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeCompletion/member-access.cpp (original)
> +++ cfe/trunk/test/CodeCompletion/member-access.cpp Mon Sep 21 15:12:40 2009
> @@ -38,5 +38,5 @@
>   // CHECK-CC1: member1 : 2
>   // CHECK-CC1: member2 : 2
>   // CHECK-CC1: member3 : 2
> -  // CHECK-CC1: memfun1 : 2 (Hidden)
> +  // CHECK-CC1: memfun1 : 2 (Hidden) : Base2::memfun1(<#int#>)
>   p->
> \ No newline at end of file
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>




More information about the cfe-commits mailing list