beanz added inline comments.
================
Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:58
+/// them before we initialize the ExternalSemaSource base class.
+struct ChainedHLSLExternalSemaSourceMembers {
+ ChainedHLSLExternalSemaSourceMembers(ExternalSemaSource *ExtSema)
----------------
python3kgae wrote:
> beanz wrote:
> > IIUC, this code just exists to make sure that the `ASTReader` deserializes
> > before the external sema source starts adding things. Is that correct?
> >
> > If so, you don't need to do this, instead you can just add this code to
> > `InitializeSema()` to force the `ASTReader` to de-serialize the decls:
> >
> > ```
> > // If the translation unit has external storage force external decls to
> > load.
> > if (AST.getTranslationUnitDecl()->hasExternalLexicalStorage())
> > (void)AST.getTranslationUnitDecl()->decls_begin();
> > ```
> Still need this to make sure HLSLSema and ExternalSema are initialized before
> MultiplexExternalSemaSource.
In FrontendAction, where you are creating the Chained source, you can instead
create a Multiplex source, and put the PCH external source in as the first
source and the HLSL one second. The PCH will get initialized first, the HLSL
one can force the PCH one to populate the decls.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:314
+
+ if (!HLSLNamespace) {
+ HLSLNamespace = NamespaceDecl::Create(AST, AST.getTranslationUnitDecl(),
----------------
Rather than reusing the namespace you can set the PCH one as the previous decl
for the one you're declaring here. That will result in them basically being
merged because Namespaces can have multiple decls.
The advantage to this is that the PCH namespace will be marked in the AST as a
deserialized decl but the HLSL one won't. It will make debugging and AST
visualization easier.
================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:411
+ Completions.insert(std::make_pair(
+ Decl, std::bind(&HLSLExternalSemaSource::completeBufferType, this,
+ std::placeholders::_1)));
----------------
If the decl you put in here is the canonical decl, and you do the lookup by
canonical decl. The conflicting decls shouldn't be an issue. That does require
that you mark the previous decl so that they are known to be the same
declaration.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132421/new/
https://reviews.llvm.org/D132421
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
