[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.

The test case is for Objective-C but it is likely other languages like
C++ are affected as well, just haven't created appropriate test cases.

The test case fails with the following error

  clang/test/Modules/Output/ambiguous-enum-lookup.m.tmp/test.m:7:11: error: reference to 'kEnumValue' is ambiguous
      s.x = kEnumValue;
            ^
  clang/test/Modules/Output/ambiguous-enum-lookup.m.tmp/Frameworks/NonModular.framework/Headers/NonModular.h:8:5: note: candidate found by name lookup is 'kEnumValue'
      kEnumValue = 1,
      ^
  clang/test/Modules/Output/ambiguous-enum-lookup.m.tmp/Frameworks/NonModular.framework/Headers/NonModular.h:8:5: note: candidate found by name lookup is 'kEnumValue'
      kEnumValue = 1,
      ^

It happens because we have 2 EnumConstantDecl for 'kEnumValue' - one from
PiecewiseVisible module, another non-modular. And the the name lookup is
ambiguous because these decls have different canonical decls.
Non-modular decl believes it is the canonical decl because that's the
behavior for Mergeable and in the source code non-modular decl comes
before other EnumConstantDecl. Modular EnumConstantDecl has no intention
of being canonical decl but it is deserialized *before* non-modular decl
is created, so as the only decl it becomes the canonical decl.

The idea for the fix is to skip deserialization of decls from hidden
(sub)modules, thus allowing to establish correct canonical decl. This is
a proof of concept, the implementation isn't optimal, and there are
failing tests. I mostly wanted to share the approach to discuss it.

Another high-level approach I've considered is to enhance handling decls
from hidden modules and to delay connecting them with redecl chains and
canonical decls till the moment they become visible. I've decided to go
with "avoid deserialization" approach because this way we should be
doing less work and it seems less error-prone compared to juggling with
existing decls.

rdar://82908206


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114411

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/ambiguous-enum-lookup.m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114411.389088.patch
Type: text/x-patch
Size: 6498 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211123/b6fec786/attachment.bin>