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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Haven't really checked the code, at the moment thinking about various failure modes.

Cases that aren't tested but I suspect are valid ones:

- empty block, i.e., `requires cplusplus {}`
- nested blocks.

Is it possible to reference external module map from `requires` block? I mean that in one context the module has some extra requirements but in a different context doesn't have them.

It would be nice to have some mechanism to notify developers that includes are still performed regardless of `requires`. For example, in A.h we have

  #include <A_cpp_feature.h>

and in module map

  module A {
    header "A.h"
    requires cplusplus {
      header "A_cpp_feature.h"
    }
    export *
  }

It doesn't mean the header A_cpp_feature.h is used only with cplusplus feature which is not obvious and with big headers can be hard to notice. But I feel like this request goes beyond the scope of your change, so not insisting on it.

Also it can be nice to diagnose strange conditions like `requires cplusplus, !cplusplus` but that's orthogonal to this feature.



================
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 '}'">;
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118311