[cfe-commits] r142634 - in /cfe/trunk: include/clang/AST/DeclBase.h lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp
Argyrios Kyrtzidis
kyrtzidis at apple.com
Mon Oct 24 18:49:48 PDT 2011
On Oct 20, 2011, at 7:57 PM, Sean Callanan wrote:
> Author: spyffe
> Date: Thu Oct 20 21:57:43 2011
> New Revision: 142634
>
> URL: http://llvm.org/viewvc/llvm-project?rev=142634&view=rev
> Log:
> I added a new function to DeclContext called
> addDeclInternal(). This function suppresses any
> calls to FindExternalVisibleDeclsByName() while
> a Decl is added to a DeclContext. This behavior
> is required for the ASTImporter, because in the
> case of the LLDB client the ASTImporter would be
> called recursively to import the visible decls,
> which leads to assertions because the recursive
> call is seeing partially-formed types.
>
> I also modified the ASTImporter to use
> addDeclInternal() in all places where it would
> otherwise use addDecl(). This fix should not
> affect the rest of Clang, passes Clang's
> testsuite, and fixes several serious LLDB bugs.
Could we go in another direction here, making ASTImporter simpler instead of complicating DeclContext ?
How about, the ASTImporter does not modify DeclContexts at all (maybe if 'MinimalImport' is true) in which case it turns into a simpler "Decl-creation-factory".
The clients would be responsible to do the right thing with the Decls that they get from ASTImporter, which would either mean adding them to a DeclContext, or, most of the cases involving lldb, returning them via the 'ExternaASTSource' so the DeclContext that already called into ExternaASTSource can add them itself.
In general, this will get lldb somewhat "closer" to how PCH works instead of diverging even more.
-Argyrios
>
> Modified:
> cfe/trunk/include/clang/AST/DeclBase.h
> cfe/trunk/lib/AST/ASTImporter.cpp
> cfe/trunk/lib/AST/DeclBase.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=142634&r1=142633&r2=142634&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Thu Oct 20 21:57:43 2011
> @@ -1231,6 +1231,8 @@
> /// If D is also a NamedDecl, it will be made visible within its
> /// semantic context via makeDeclVisibleInContext.
> void addDecl(Decl *D);
> +
> + void addDeclInternal(Decl *D);
>
> /// @brief Add the declaration D to this context without modifying
> /// any lookup tables.
> @@ -1290,6 +1292,9 @@
> /// the lookup tables because it can be easily recovered by walking
> /// the declaration chains.
> void makeDeclVisibleInContext(NamedDecl *D, bool Recoverable = true);
> +
> + void makeDeclVisibleInContextInternal(NamedDecl *D,
> + bool Recoverable = true);
>
> /// udir_iterator - Iterates through the using-directives stored
> /// within this context.
> @@ -1359,7 +1364,8 @@
> StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;
>
> void buildLookup(DeclContext *DCtx);
> - void makeDeclVisibleInContextImpl(NamedDecl *D);
> + void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal, bool Recoverable);
> + void makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal);
> };
>
> inline bool Decl::isTemplateParameter() const {
>
> Modified: cfe/trunk/lib/AST/ASTImporter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=142634&r1=142633&r2=142634&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ASTImporter.cpp (original)
> +++ cfe/trunk/lib/AST/ASTImporter.cpp Thu Oct 20 21:57:43 2011
> @@ -2038,7 +2038,7 @@
> Importer.Import(D->getLocStart()),
> Loc, Name.getAsIdentifierInfo());
> ToNamespace->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToNamespace);
> + LexicalDC->addDeclInternal(ToNamespace);
>
> // If this is an anonymous namespace, register it as the anonymous
> // namespace within its context.
> @@ -2117,7 +2117,7 @@
> ToTypedef->setAccess(D->getAccess());
> ToTypedef->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToTypedef);
> - LexicalDC->addDecl(ToTypedef);
> + LexicalDC->addDeclInternal(ToTypedef);
>
> return ToTypedef;
> }
> @@ -2188,7 +2188,7 @@
> D2->setAccess(D->getAccess());
> D2->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, D2);
> - LexicalDC->addDecl(D2);
> + LexicalDC->addDeclInternal(D2);
>
> // Import the integer type.
> QualType ToIntegerType = Importer.Import(D->getIntegerType());
> @@ -2293,7 +2293,7 @@
>
> D2->setQualifierInfo(Importer.Import(D->getQualifierLoc()));
> D2->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(D2);
> + LexicalDC->addDeclInternal(D2);
> }
>
> Importer.Imported(D, D2);
> @@ -2350,7 +2350,7 @@
> ToEnumerator->setAccess(D->getAccess());
> ToEnumerator->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToEnumerator);
> - LexicalDC->addDecl(ToEnumerator);
> + LexicalDC->addDeclInternal(ToEnumerator);
> return ToEnumerator;
> }
>
> @@ -2491,14 +2491,14 @@
> // Set the parameters.
> for (unsigned I = 0, N = Parameters.size(); I != N; ++I) {
> Parameters[I]->setOwningFunction(ToFunction);
> - ToFunction->addDecl(Parameters[I]);
> + ToFunction->addDeclInternal(Parameters[I]);
> }
> ToFunction->setParams(Parameters);
>
> // FIXME: Other bits to merge?
>
> // Add this function to the lexical context.
> - LexicalDC->addDecl(ToFunction);
> + LexicalDC->addDeclInternal(ToFunction);
>
> return ToFunction;
> }
> @@ -2566,7 +2566,7 @@
> if (ToField->hasInClassInitializer())
> ToField->setInClassInitializer(D->getInClassInitializer());
> Importer.Imported(D, ToField);
> - LexicalDC->addDecl(ToField);
> + LexicalDC->addDeclInternal(ToField);
> return ToField;
> }
>
> @@ -2622,7 +2622,7 @@
> ToIndirectField->setAccess(D->getAccess());
> ToIndirectField->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToIndirectField);
> - LexicalDC->addDecl(ToIndirectField);
> + LexicalDC->addDeclInternal(ToIndirectField);
> return ToIndirectField;
> }
>
> @@ -2671,7 +2671,7 @@
> BitWidth, D->getSynthesize());
> ToIvar->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToIvar);
> - LexicalDC->addDecl(ToIvar);
> + LexicalDC->addDeclInternal(ToIvar);
> return ToIvar;
>
> }
> @@ -2784,7 +2784,7 @@
> ToVar->setAccess(D->getAccess());
> ToVar->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToVar);
> - LexicalDC->addDecl(ToVar);
> + LexicalDC->addDeclInternal(ToVar);
>
> // Merge the initializer.
> // FIXME: Can we really import any initializer? Alternatively, we could force
> @@ -2963,7 +2963,7 @@
> // Set the parameters.
> for (unsigned I = 0, N = ToParams.size(); I != N; ++I) {
> ToParams[I]->setOwningFunction(ToMethod);
> - ToMethod->addDecl(ToParams[I]);
> + ToMethod->addDeclInternal(ToParams[I]);
> }
> SmallVector<SourceLocation, 12> SelLocs;
> D->getSelectorLocs(SelLocs);
> @@ -2971,7 +2971,7 @@
>
> ToMethod->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToMethod);
> - LexicalDC->addDecl(ToMethod);
> + LexicalDC->addDeclInternal(ToMethod);
> return ToMethod;
> }
>
> @@ -3000,7 +3000,7 @@
> Name.getAsIdentifierInfo(),
> ToInterface);
> ToCategory->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToCategory);
> + LexicalDC->addDeclInternal(ToCategory);
> Importer.Imported(D, ToCategory);
>
> // Import protocols
> @@ -3073,7 +3073,7 @@
> D->isInitiallyForwardDecl());
> ToProto->setForwardDecl(D->isForwardDecl());
> ToProto->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToProto);
> + LexicalDC->addDeclInternal(ToProto);
> }
> Importer.Imported(D, ToProto);
>
> @@ -3136,7 +3136,7 @@
> D->isImplicitInterfaceDecl());
> ToIface->setForwardDecl(D->isForwardDecl());
> ToIface->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToIface);
> + LexicalDC->addDeclInternal(ToIface);
> }
> Importer.Imported(D, ToIface);
>
> @@ -3255,7 +3255,7 @@
> ToImpl->setLexicalDeclContext(LexicalDC);
> }
>
> - LexicalDC->addDecl(ToImpl);
> + LexicalDC->addDeclInternal(ToImpl);
> Category->setImplementation(ToImpl);
> }
>
> @@ -3384,7 +3384,7 @@
> D->getPropertyImplementation());
> Importer.Imported(D, ToProperty);
> ToProperty->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToProperty);
> + LexicalDC->addDeclInternal(ToProperty);
>
> ToProperty->setPropertyAttributes(D->getPropertyAttributes());
> ToProperty->setPropertyAttributesAsWritten(
> @@ -3443,7 +3443,7 @@
> Importer.Import(D->getPropertyIvarDeclLoc()));
> ToImpl->setLexicalDeclContext(LexicalDC);
> Importer.Imported(D, ToImpl);
> - LexicalDC->addDecl(ToImpl);
> + LexicalDC->addDeclInternal(ToImpl);
> } else {
> // Check that we have the same kind of property implementation (@synthesize
> // vs. @dynamic).
> @@ -3520,7 +3520,7 @@
> Protocols.data(), Protocols.size(),
> Locations.data());
> ToForward->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToForward);
> + LexicalDC->addDeclInternal(ToForward);
> Importer.Imported(D, ToForward);
> return ToForward;
> }
> @@ -3549,7 +3549,7 @@
> Importer.Import(From->getLocation()));
>
> ToClass->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(ToClass);
> + LexicalDC->addDeclInternal(ToClass);
> Importer.Imported(D, ToClass);
> return ToClass;
> }
> @@ -3711,7 +3711,7 @@
>
> D2->setAccess(D->getAccess());
> D2->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(D2);
> + LexicalDC->addDeclInternal(D2);
>
> // Note the relationship between the class templates.
> Importer.Imported(D, D2);
> @@ -3806,7 +3806,7 @@
>
> // Add the specialization to this context.
> D2->setLexicalDeclContext(LexicalDC);
> - LexicalDC->addDecl(D2);
> + LexicalDC->addDeclInternal(D2);
> }
> Importer.Imported(D, D2);
>
>
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=142634&r1=142633&r2=142634&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Thu Oct 20 21:57:43 2011
> @@ -1013,6 +1013,13 @@
> ND->getDeclContext()->makeDeclVisibleInContext(ND);
> }
>
> +void DeclContext::addDeclInternal(Decl *D) {
> + addHiddenDecl(D);
> +
> + if (NamedDecl *ND = dyn_cast<NamedDecl>(D))
> + ND->getDeclContext()->makeDeclVisibleInContextInternal(ND);
> +}
> +
> /// buildLookup - Build the lookup data structure with all of the
> /// declarations in DCtx (and any other contexts linked to it or
> /// transparent contexts nested within it).
> @@ -1026,12 +1033,12 @@
> // lookup building, this is implicitly enforced by addDecl.
> if (NamedDecl *ND = dyn_cast<NamedDecl>(*D))
> if (D->getDeclContext() == DCtx)
> - makeDeclVisibleInContextImpl(ND);
> + makeDeclVisibleInContextImpl(ND, false);
>
> // Insert any forward-declared Objective-C interface into the lookup
> // data structure.
> if (ObjCClassDecl *Class = dyn_cast<ObjCClassDecl>(*D))
> - makeDeclVisibleInContextImpl(Class->getForwardInterfaceDecl());
> + makeDeclVisibleInContextImpl(Class->getForwardInterfaceDecl(), false);
>
> // If this declaration is itself a transparent declaration context or
> // inline namespace, add its members (recursively).
> @@ -1147,7 +1154,17 @@
> return false;
> }
>
> -void DeclContext::makeDeclVisibleInContext(NamedDecl *D, bool Recoverable) {
> +void DeclContext::makeDeclVisibleInContext(NamedDecl *D, bool Recoverable)
> +{
> + makeDeclVisibleInContextWithFlags(D, false, Recoverable);
> +}
> +
> +void DeclContext::makeDeclVisibleInContextInternal(NamedDecl *D, bool Recoverable)
> +{
> + makeDeclVisibleInContextWithFlags(D, true, Recoverable);
> +}
> +
> +void DeclContext::makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal, bool Recoverable) {
> // FIXME: This feels like a hack. Should DeclarationName support
> // template-ids, or is there a better way to keep specializations
> // from being visible?
> @@ -1159,7 +1176,7 @@
>
> DeclContext *PrimaryContext = getPrimaryContext();
> if (PrimaryContext != this) {
> - PrimaryContext->makeDeclVisibleInContext(D, Recoverable);
> + PrimaryContext->makeDeclVisibleInContextWithFlags(D, Internal, Recoverable);
> return;
> }
>
> @@ -1168,12 +1185,12 @@
> // them so we can add the decl. Otherwise, be lazy and don't build that
> // structure until someone asks for it.
> if (LookupPtr || !Recoverable || hasExternalVisibleStorage())
> - makeDeclVisibleInContextImpl(D);
> + makeDeclVisibleInContextImpl(D, Internal);
>
> // If we are a transparent context or inline namespace, insert into our
> // parent context, too. This operation is recursive.
> if (isTransparentContext() || isInlineNamespace())
> - getParent()->makeDeclVisibleInContext(D, Recoverable);
> + getParent()->makeDeclVisibleInContextWithFlags(D, Internal, Recoverable);
>
> Decl *DCAsDecl = cast<Decl>(this);
> // Notify that a decl was made visible unless it's a Tag being defined.
> @@ -1182,7 +1199,7 @@
> L->AddedVisibleDecl(this, D);
> }
>
> -void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D) {
> +void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal) {
> // Skip unnamed declarations.
> if (!D->getDeclName())
> return;
> @@ -1203,10 +1220,11 @@
> // with this declaration's name.
> // If the lookup table contains an entry about this name it means that we
> // have already checked the external source.
> - if (ExternalASTSource *Source = getParentASTContext().getExternalSource())
> - if (hasExternalVisibleStorage() &&
> - LookupPtr->find(D->getDeclName()) == LookupPtr->end())
> - Source->FindExternalVisibleDeclsByName(this, D->getDeclName());
> + if (!Internal)
> + if (ExternalASTSource *Source = getParentASTContext().getExternalSource())
> + if (hasExternalVisibleStorage() &&
> + LookupPtr->find(D->getDeclName()) == LookupPtr->end())
> + Source->FindExternalVisibleDeclsByName(this, D->getDeclName());
>
> // Insert this declaration into the map.
> StoredDeclsList &DeclNameEntries = (*LookupPtr)[D->getDeclName()];
>
>
> _______________________________________________
> 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