jdoerfert marked 4 inline comments as done.
jdoerfert added a comment.
@kiranchandramohan @JonChesterfield I will address the comments just made, if
you think this is otherwise fine it would be good if you accept the patch (it's
been around for weeks so it's not really people didn't have a chance to look).
================
Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272
+
+ template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); }
+
----------------
kiranchandramohan wrote:
> jdoerfert wrote:
> > jdoerfert wrote:
> > > kiranchandramohan wrote:
> > > > jdoerfert wrote:
> > > > > kiranchandramohan wrote:
> > > > > > jdoerfert wrote:
> > > > > > > kiranchandramohan wrote:
> > > > > > > > Compiler throws up this error.
> > > > > > > > error: explicit specialization in non-namespace scope ‘class
> > > > > > > > clang::ASTRecordReader’
> > > > > > > Oh, my compiler was happy. Let me rebase and see what the
> > > > > > > pre-merge bots say so I might get some insight into the problem.
> > > > > > Were you able to reproduce the error? I was using gcc 9.2 compiler.
> > > > > I have not seen the error yet. I build with clang. Do you happen to
> > > > > have an idea how to refactor this? I will look into it with a new gcc
> > > > > asap.
> > > > Moving the specialization to the source file seems to fix the issue.
> > > I will do that then. Did you find the time to look over the rest?
> > > Moving the specialization to the source file seems to fix the issue.
> >
> > Like this?
> You can remove the following declaration from the header file and just define
> it in the cpp file.
> /// Specialization for OMPTraitInfo*.
> template <> OMPTraitInfo *readUserType();
Right, will do that, thx!
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:6575
+template <> void ASTRecordWriter::writeUserType(OMPTraitInfo *TI) {
+ writeUInt32(TI->Sets.size());
----------------
kiranchandramohan wrote:
> Had to also move this up in the file to avoid an instantiation after use
> error.
I didn't get that but I have it in the header, I'll figure it out.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:417
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+ S.pop_back();
+ return S;
----------------
kiranchandramohan wrote:
> If there is no match? Or is it always guaranteed to have a match?
We always have selector sets (the outermost category).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71830/new/
https://reviews.llvm.org/D71830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
