[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 4 inline comments as done.
iains added inline comments.


================
Comment at: clang/lib/AST/TextNodeDumper.cpp:1723-1725
+  if (!D->isInlineSpecified() && D->isInlined()) {
+    OS << " implicit-inline";
+  }
----------------
ChuanqiXu wrote:
> Is this necessary to this revision?
yes, to test it


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9362
+      //   to the global module.
+      if (getLangOpts().CPlusPlus20 || getLangOpts().CPlusPlus2b) {
+        Module *M = NewFD->getOwningModule();
----------------
ChuanqiXu wrote:
> If I remember correctly, when we run `-std=c++2b`, `getLangOpts().CPlusPlus20` would be true too. So we could check `getLangOpts().CPlusPlus20` only here.
will double-check.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9363-9365
+        Module *M = NewFD->getOwningModule();
+        if (!M || M->isGlobalModule())
+          NewFD->setImplicitlyInline();
----------------
ChuanqiXu wrote:
> nit: this is not  required.
well, the logic is required ;) .. you have suggested a different way to write..


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9657
+      //   in its class definition, it is inline.
+      if (getLangOpts().CPlusPlus20 || getLangOpts().CPlusPlus2b) {
+        Module *M = NewFD->getOwningModule();
----------------
ChuanqiXu wrote:
> ditto
likewise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045