[NFC][clang][PCH] ASTStmtReader::VisitStmt(): fixup faulty assert.

[lebedev.ri]

Summary

While this is NFC - NumStmtFields is currently 0, the assert is faulty.

It's not checking that VisitStmt() is called when getIdx() returns NumStmtFields.
It is checking that VisitStmt() is called *first*, when getIdx() returns 0.

I have stumbled into this while changing NumStmtFields to 1.

[riccibruno]

I am not an expert in the serialization code (just did some modifications), but this seems reasonable to me.

[lebedev.ri]

In D59196#1429393, @riccibruno wrote:

I am not an expert in the serialization code (just did some modifications), but this seems reasonable to me.

That's my thoughts too..
Unless of course it is also testing that NumStmtFields is zero.
Anyone with more knowledged opinion?

[riccibruno]

In D59196#1431473, @lebedev.ri wrote:

In D59196#1429393, @riccibruno wrote:

I am not an expert in the serialization code (just did some modifications), but this seems reasonable to me.

That's my thoughts too..
Unless of course it is also testing that NumStmtFields is zero.
Anyone with more knowledged opinion?

On further though I think the assert is fine if you view it as the last thing in VisitStmt, since it is checking that you did not forget a field. Instead just move the deserialization of the new bit you introduce in D59214 so that the assert is last.

[lebedev.ri]

In D59196#1431531, @riccibruno wrote:

In D59196#1431473, @lebedev.ri wrote:

In D59196#1429393, @riccibruno wrote:

I am not an expert in the serialization code (just did some modifications), but this seems reasonable to me.

That's my thoughts too..
Unless of course it is also testing that NumStmtFields is zero.
Anyone with more knowledged opinion?

On further though I think the assert is fine if you view it as the last thing in VisitStmt, since it is checking that you did not forget a field. Instead just move the deserialization of the new bit you introduce in D59214 so that the assert is last.

Oh, that should actually be it.

lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::ReadTemplateKWAndArgsInfo(ASTTemplateKWAndArgsInfo &Args,
   ArgInfo.setLAngleLoc(ReadSourceLocation());
   ArgInfo.setRAngleLoc(ReadSourceLocation());
   for (unsigned i = 0; i != NumTemplateArgs; ++i)
     ArgInfo.addArgument(Record.readTemplateArgumentLoc());
   Args.initializeFrom(TemplateKWLoc, ArgInfo, ArgsLocArray);
 }
 
 void ASTStmtReader::VisitStmt(Stmt *S) {
-  assert(Record.getIdx() == NumStmtFields && "Incorrect statement field count");
+  assert(Record.getIdx() == 0 && "Incorrect statement field count");
 }
 
 void ASTStmtReader::VisitNullStmt(NullStmt *S) {
   VisitStmt(S);
   S->setSemiLoc(ReadSourceLocation());
   S->NullStmtBits.HasLeadingEmptyMacro = Record.readInt();
 }