[PATCH] D111543: [clang][modules] Stop creating `IdentifierInfo` for names of explicit modules

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D111543#3062095 <https://reviews.llvm.org/D111543#3062095>, @rsmith wrote:

> I'm concerned that this is adding complexity to paper over a bug elsewhere. Creating an `IdentifierInfo` should never cause unrelated uses of that identifier to change their meaning or behavior. My guess is that there's a bug in how we resolve or merge identifier  information in the AST reader.

Thanks for the information. You're right that the issue is most likely somewhere around `ASTReader`, not name resolution.

My understanding is that `Preprocessor::getIdentifierInfo` may trigger AST deserialization. The problem here seems to be that we're calling `getIdentifierInfo` before the preprocessor's `IdentifierTable` is set up with "external identifier info lookup" (aka `ASTReader`). So the call to `IdentifierTable::get` creates new `IdentifierInfo` for `"Interface"` instead of looking it up in the AST. Further calls to `getIdentifierInfo` resolve to this new `IdentifierInfo` and the `ObjCInterfaceDecl` never gets deserialized.

@rsmith, is my understanding correct? If so, I'm not sure there's another bug hiding here: it's just an unexpected series of events that should probably be prevented (hence this patch).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111543/new/

https://reviews.llvm.org/D111543