[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: cfe-commits, aaron.ballman.
aaron.ballman added reviewers: aaron.ballman, erichkeane.
aaron.ballman added a comment.

Adding some more reviewers and subscribing the mailing lists.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10842
 def note_prev_module_definition_from_ast_file : Note<"module loaded from '%0'">;
+def err_module_langugae_linkage_no_global : Error <
+  "The declaration %0 appears within a linkage-specification should be "
----------------
aaron.ballman wrote:
> mizvekov wrote:
> > ```
> > def **err_module_language_linkage_no_global** : Error <
> >   "The declaration %0**, which** appears within a linkage-specification**, is not "
> >   "attached to **the** global module.">;
> > ```
> > 
> > My two cents since I think the last sentence did not seem to explain the problem very well.
> Diagnostics in Clang are intentionally ungrammatical, so they should not have leading capitalization or most terminating punctuation. I agree that the diagnostic wording here doesn't really explain what's going wrong.
typo, should be: err_module_language_linkage_no_global 


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10842-10845
+def err_module_langugae_linkage_no_global : Error <
+  "The declaration %0 appears within a linkage-specification should be "
+  "attached to global module. But the compiler failed to search the global "
+  "module.">;
----------------
mizvekov wrote:
> ```
> def **err_module_language_linkage_no_global** : Error <
>   "The declaration %0**, which** appears within a linkage-specification**, is not "
>   "attached to **the** global module.">;
> ```
> 
> My two cents since I think the last sentence did not seem to explain the problem very well.
Diagnostics in Clang are intentionally ungrammatical, so they should not have leading capitalization or most terminating punctuation. I agree that the diagnostic wording here doesn't really explain what's going wrong.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9351
+          (NewFD->isExternCContext() || NewFD->isExternCXXContext())) {
+        if (!getGlobalModule())
+          Diag(NewFD->getLocation(),
----------------
I'm a bit confused here. [module.unit]p7 is describing what module a declaration attached to under which circumstances. I don't see a constraint there which should result in a diagnostic. My reading of https://eel.is/c++draft/module.unit#6 is that the global module always exists, so we should be able to attach declarations to it at any point. Am I misunderstanding?


================
Comment at: clang/test/CXX/module/module.linkage_specification/Inputs/h1.h:7
+}
\ No newline at end of file

----------------
mizvekov wrote:
> Please if possible make your editor set new lines at EOF.
+1 -- please add a newline to the end of each of these files that.


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

https://reviews.llvm.org/D110215