[clang] [RFC][C++20][Modules] Relax ODR check in unnamed modules (PR #111160)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-modules

Author: Dmitry Polukhin (dmpolukhin)

<details>
<summary>Changes</summary>

Summary:
Option `-fskip-odr-check-in-gmf` is set by default and I think it is what most of C++ developers want. But in header units, Clang ODR checking is too strict, making them hard to use, as seen in the example in the diff. This diff relaxes ODR checks for unnamed modules to match GMF ODR checking.

Alternative solution is to add special handling for CXXFoldExpr to ignore differences in `callee`. In particular, it will treat the following fragments as identical. I think it might be reasonable default behavior instead of `-fskip-odr-check-in-gmf` for header units and GMF. It might be reasonable compromise for checking some ODRs but allow most common issues.
```
CXXFoldExpr 0x55556588b1b8 '<dependent type>'
|-<<<NULL>>>
|-UnresolvedLookupExpr 0x55556588b140 '<dependent type>' lvalue (no ADL) = '__cmp_cat_id' 0x55556588ac18
| `-TemplateArgument type '_Ts'
|   `-TemplateTypeParmType 0x55556588adf0 '_Ts' dependent contains_unexpanded_pack depth 0 index 0 pack
|     `-TemplateTypeParm 0x55556588ad70 '_Ts'
`-<<<NULL>>>

CXXFoldExpr 0x55556588b670 '<dependent type>'
|-UnresolvedLookupExpr 0x55556588b628 '<overloaded function type>' lvalue (ADL) = 'operator|' 0x55556588ae60
|-UnresolvedLookupExpr 0x55556588b5b0 '<dependent type>' lvalue (no ADL) = '__cmp_cat_id' 0x55556588b0d8
| `-TemplateArgument type '_Ts'
|   `-TemplateTypeParmType 0x55556588b260 '_Ts' dependent contains_unexpanded_pack depth 0 index 0 pack
|     `-TemplateTypeParm 0x55556588b1e0 '_Ts'
`-<<<NULL>>>
```

Test Plan: check-clang

---
Full diff: https://github.com/llvm/llvm-project/pull/111160.diff


2 Files Affected:

- (modified) clang/include/clang/Serialization/ASTReader.h (+1-1) 
- (added) clang/test/Headers/header-unit-common-cmp-cat.cpp (+32) 


``````````diff
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index aa88560b259a3b..109c905d2bebb6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2527,7 +2527,7 @@ class BitsUnpacker {
 
 inline bool shouldSkipCheckingODR(const Decl *D) {
   return D->getASTContext().getLangOpts().SkipODRCheckInGMF &&
-         D->isFromGlobalModule();
+         (D->isFromGlobalModule() || !D->isInNamedModule());
 }
 
 } // namespace clang
diff --git a/clang/test/Headers/header-unit-common-cmp-cat.cpp b/clang/test/Headers/header-unit-common-cmp-cat.cpp
new file mode 100644
index 00000000000000..b9d00ba042fda5
--- /dev/null
+++ b/clang/test/Headers/header-unit-common-cmp-cat.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -fR %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -verify -std=c++20 -fskip-odr-check-in-gmf -emit-header-unit -xc++-user-header bz0.h
+// RUN: %clang_cc1 -verify -std=c++20 -fskip-odr-check-in-gmf -emit-header-unit -xc++-user-header bz1.h
+// RUN: %clang_cc1 -verify -std=c++20 -fskip-odr-check-in-gmf -emit-header-unit -xc++-user-header -fmodule-file=bz0.pcm -fmodule-file=bz1.pcm bz.cpp
+
+//--- compare
+template<typename _Tp>
+inline constexpr unsigned __cmp_cat_id = 1;
+
+template<typename... _Ts>
+constexpr auto __common_cmp_cat() {
+  (__cmp_cat_id<_Ts> | ...);
+}
+
+//--- bz0.h
+template <class T>
+int operator|(T, T);
+
+#include "compare"
+// expected-no-diagnostics
+
+//--- bz1.h
+#include "compare"
+// expected-no-diagnostics
+
+//--- bz.cpp
+#include "compare"
+
+import "bz0.h"; // expected-warning {{the implementation of header units is in an experimental phase}}
+import "bz1.h"; // expected-warning {{the implementation of header units is in an experimental phase}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/111160