python3kgae added inline comments.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:297
+ }
+
IdentifierInfo &HLSL = AST.Idents.get("hlsl", tok::TokenKind::identifier);
----------------
beanz wrote:
> I think the core of what you'e doing here is not far off, but I think this
> pattern doesn't scale or cover all the use cases that matter.
>
> The cases that come to my mind are:
> 1) PCH has a forward declaration, source doesn't use the type so we shouldn't
> complete the type.
> 2) PCH has a forward declaration, source uses the type, so we need to
> generate a complete decl.
> 3) PCH has a complete declaration, so we should re-use the declaration and
> not need to do anything.
>
> We also need to keep in mind that we're going to keep adding more and more
> types, so this pattern where each new type needs to add code to lookup is
> going to be a challenge. It is also going to be a challenge to manage the
> complexity of PCH defines one thing but not another.
>
> One thing you should look at is how clang handles the following C++ code:
>
> ```
> template<typename T>
> class Foo;
>
> template<typename T>
> class Foo {
> T Val;
> };
> ```
>
> This results in two decls. One for the forward decl and the second for the
> definition. The definition lists the forward declaration as the previous
> decl. We should do that here.
>
> When we generate the HLSL namespace, we can do a lookup and if we find a
> previous decl, set the previous decl.
>
> When we generate any of the builtin types, we can lookup the previous decl,
> and annotate as the previous decl. We'll can also handle the case where a
> decl from the PCH is complete. If the PCH provides a complete decl, we can
> skip generating any decls for it, and the lookups should just work.
Set previous decl not work.
It goes to "For most kinds of declaration, it doesn't really matter which one
we pick." before checking PreviousDecl.
```
// For most kinds of declaration, it doesn't really matter which one we pick.
if (!isa<FunctionDecl>(DUnderlying) && !isa<VarDecl>(DUnderlying)) {
// If the existing declaration is hidden, prefer the new one. Otherwise,
// keep what we've got.
return !S.isVisible(Existing);
}
// Pick the newer declaration; it might have a more precise type.
for (Decl *Prev = DUnderlying->getPreviousDecl(); Prev;
Prev = Prev->getPreviousDecl())
if (Prev == EUnderlying)
return true;
return false;
```
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
