[PATCH] D132421: [HLSL] Support PCH for cc1 mode

Xiang Li via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 31 12:14:08 PDT 2022


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



More information about the cfe-commits mailing list