[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/DeclBase.h:616
 
+  bool isInExportDeclContext() const;
+
----------------
I think it would be good to add some comments to document the function (as done with surrounding code).


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7793-7794
   "because namespace %1 does not enclose namespace %2">;
+def err_export_non_namespace_scope_name : Error<"cannot export %0 as it is "
+  "not at namespace scope.">;
+def err_redeclaration_non_exported : Error <"cannot export redeclaration %0 here "
----------------



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7795-7796
+  "not at namespace scope.">;
+def err_redeclaration_non_exported : Error <"cannot export redeclaration %0 here "
+  "since the previous declaration is not exported.">;
 def err_invalid_declarator_global_scope : Error<
----------------
Both of these tweak the formatting somewhat, but the important thing is dropping the trailing punctuation from the diagnostic text.


================
Comment at: clang/lib/AST/DeclBase.cpp:999
+bool Decl::isInExportDeclContext() const {
+  auto *DC = getLexicalDeclContext();
+
----------------



================
Comment at: clang/lib/Sema/SemaDecl.cpp:1622
 
+// [module.interface]p6:
+// A redeclaration of an entity X is implicitly exported if X was introduced by
----------------



================
Comment at: clang/lib/Sema/SemaDecl.cpp:1643-1645
+// A simple wrapper function to ease the call for
+// CheckRedeclarationModuleOwnership and CheckRedeclarationExported and any
+// other thing needed.
----------------



================
Comment at: clang/lib/Sema/SemaDecl.cpp:5789
+      if (!isa<NamespaceDecl>(DC))
+        Diag(Loc, diag::err_export_non_namespace_scope_name) << Name;
+      else
----------------



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

https://reviews.llvm.org/D112903