[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

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 '&lt;dependent type&gt;'
|-&lt;&lt;&lt;NULL&gt;&gt;&gt;
|-UnresolvedLookupExpr 0x55556588b140 '&lt;dependent type&gt;' 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'
`-&lt;&lt;&lt;NULL&gt;&gt;&gt;

CXXFoldExpr 0x55556588b670 '&lt;dependent type&gt;'
|-UnresolvedLookupExpr 0x55556588b628 '&lt;overloaded function type&gt;' lvalue 
(ADL) = 'operator|' 0x55556588ae60
|-UnresolvedLookupExpr 0x55556588b5b0 '&lt;dependent type&gt;' 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'
`-&lt;&lt;&lt;NULL&gt;&gt;&gt;
```

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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits