aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:50
+ if (S.LookupQualifiedName(Result, HLSLNamespace)) {
+ NamedDecl *Found = Result.getFoundDecl();
+ if (auto *TD = dyn_cast<ClassTemplateDecl>(Found)) {
----------------
99.9% sure this is safe because we're looking up a tag name and I can't think
of any way that might return multiple results for the same identifier, but
bringing it up just in case anyone else knows of some obscure extension where
that's possible.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:55
+ } else
+ PrevDecl = dyn_cast<CXXRecordDecl>(Found);
+ assert(PrevDecl && "Unexpected lookup result type.");
----------------
Is it possible that this finds a different kind of tag, like an enumeration?
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:85-86
AccessSpecifier Access = AccessSpecifier::AS_private) {
+ if (Record->isCompleteDefinition())
+ return *this;
assert(Record->isBeingDefined() &&
----------------
A downside to this pattern is that we need to repeat the "if it's a complete
definition, don't do anything" predicate in basically any mutating member
function. Getting that wrong will lead to hard-to-debug issues with PCH, as I
understand it. But I don't think there's a cleaner way to do that without doing
far more invasive changes.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:378
+ if (S.LookupQualifiedName(Result, AST.getTranslationUnitDecl()))
+ PrevDecl = Result.getAsSingle<NamespaceDecl>();
+ HLSLNamespace = NamespaceDecl::Create(AST, AST.getTranslationUnitDecl(),
----------------
How certain are you that there's only one result possible here?
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:387
+ // Force external decls in the HLSL namespace to load from the PCH.
+ HLSLNamespace->getCanonicalDecl()->decls_begin();
defineTrivialHLSLTypes();
----------------
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:459-461
+ ASTContext &AST = SemaPtr->getASTContext();
+ IdentifierInfo &II = AST.Idents.get("Resource", tok::TokenKind::identifier);
+ LookupResult Result(*SemaPtr, &II, SourceLocation(), Sema::LookupTagName);
----------------
We made a lookup result but then do nothing with it?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132421/new/
https://reviews.llvm.org/D132421
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
