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