[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 31 21:43:15 PST 2024


================
@@ -457,6 +457,28 @@ Note that **currently** the compiler doesn't consider inconsistent macro definit
 Currently Clang would accept the above example. But it may produce surprising results if the
 debugging code depends on consistent use of ``NDEBUG`` also in other translation units.
 
+Definitions consistency
+^^^^^^^^^^^^^^^^^^^^^^^
+
+The C++ language defines that same declarations in different translation units should have
+the same definition, as known as ODR (One Definition Rule). Prior to modules, the translation
+units don't dependent on each other and the compiler itself don't and can't perform a strong
+ODR violation check. Sometimes it is the linker does some jobs related to ODR, where the
+higher level semantics are missing. With the introduction of modules, now the compiler have
+the chance to perform ODR violations with language semantics across translation units.
+
+However, in the practice we found the existing ODR checking mechanism may be too aggressive.
+In the many issue reports about ODR violation diagnostics, most of them are false positive
+ODR violations and the true positive ODR violations are rarely reported. Also MSVC don't
+perform ODR check for declarations in the global module fragment.
----------------
ChuanqiXu9 wrote:

> Sure, we can move that part of the discussion over there.

I'll add you as CC if they agree to do so. I remember there is policy to forbid that. But I am not sure.

> Not on the llvm issue tracker yet, I am working on getting issue + patch up soon.

Got it. Not bad. But generally an issue can let us know what's going on and maybe we meet such issues too.

> How are we tracking these issues, is there a special tag for them? We could take a better look, attack this systematically.

No. All of them are falling into the `clang:modules` label. I did a just-in-time search and I found:
- https://github.com/llvm/llvm-project/issues/61317
- https://github.com/llvm/llvm-project/issues/60486
- https://github.com/llvm/llvm-project/issues/63595
- https://github.com/llvm/llvm-project/issues/63544
- https://github.com/llvm/llvm-project/issues/62943 

(Maybe there are more)

Note that not all of them fails due to bugs in ODR checker. Some of them fails due to bugs in other places and the ODR checker just fires it. Although the result is still false positive ODR violation diagnostics.

And there are another camp about, `import-before-include` (https://github.com/llvm/llvm-project/issues/61465). From the perspective of implementors, they are not the same thing. But from the perspective of users, it has a same feelings.

> Right, but reducing these issues takes a lot of effort. On the other hand, it's possible they all reduce to a few simple cases.
> This is my perception so far, working on converting a medium size code base.

Same here.

> So I will go ahead and approve this, even though I feel this might have been too hasty, as long as we are running the clang test suite as before, then we can make progress in fixing all the problems and circle back to always enabling the checker.

Thanks. It is always easy to enable the check by default any time.

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


More information about the cfe-commits mailing list