RecordDecl::LoadFieldsFromExternalStorage() expels existing decls from the DeclContext linked list

[Quuxplusone]

Bugzilla Link	PR24420
Status	NEW
Importance	P normal
Reported by	Elie Morisse (syniurge@gmail.com)
Reported on	2015-08-10 14:33:43 -0700
Last modified on	2015-08-10 18:26:14 -0700
Version	trunk
Hardware	All All
CC	llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, syniurge@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Unlike DeclContext::LoadLexicalDeclsFromExternalStorage() which preserve
existing decls with:

  // Splice the newly-read declarations into the beginning of the list
  // of declarations.
  Decl *ExternalFirst, *ExternalLast;
  std::tie(ExternalFirst, ExternalLast) =
      BuildDeclChain(Decls, FieldsAlreadyLoaded);
  ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
  FirstDecl = ExternalFirst;
  if (!LastDecl)
    LastDecl = ExternalLast;

RecordDecl::LoadFieldsFromExternalStorage() resets the linked list:

  std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls, /*FieldsAlreadyLoaded=*/false);

Assuming it to be empty may be correct most of the time but in my particular
usage of Clang this becomes a serious hassle because implicit
constructors/operators/etc. may be declared before
RecordDecl::LoadFieldsFromExternalStorage() gets called, there's no way to
control this and the only workaround I've found is to traverse the entire PCH's
AST before doing anything to ensure the fields get deserialized before implicit
methods get declared by Sema, which defeats the benefits of lazy
deserialization.

(The implicit methods are still returned by Sema::LookupXXXX after being
expelled, and they still believe their lexical decl context to be the record,
despite missing from a dump())

[Quuxplusone]

Yes, this is wrong. How did you manage to get an implicit special member function declared without loading the fields? (Is there some way to write a unit test for this against an unmodified clang binary?)