[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:722
+def err_mmap_expected_lbrace_requires : Error<
+  "expected '{' to start rquires block">;
 def err_mmap_expected_rbrace : Error<"expected '}'">;
----------------
Bigcheese wrote:
> vsapsai wrote:
> > s/rquires/requires/
> > 
> > Would it be useful to put `requires` into some quotation marks to show it's not a part of the sentence but used verbatim?
> Possibly, but I don't think we do that anywhere else. '' is always used to refer to user identifiers, "" is only used when referring to headers or strings, and I don't see any usage of ``. I have added a note so it shows up now as:
> 
> ```
> requires-block-errors.m.tmp/missing-opening-brace/module.modulemap:3:1: error: expected '{' to start requires block
> module Pony {
> ^
> requires-block-errors.m.tmp/missing-opening-brace/module.modulemap:2:1: note: for requires block declared here
> requires !cplusplus
> ^
> ```
> 
> Which makes it clear.
The updated text looks better; maybe good enough; but I wonder if it'd be more clear to diagnose as a requires-declaration at global scope. E.g., something like:
```
error: invalid requires declaration outside of a module
note: did you mean to add a '{' to open a block?
```
(maybe my wording isn't great but I hope it indicates the direction I'm suggesting)

(I don't feel strongly either way)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118311