[PATCH] Allow string literals as module identifiers

[Richard Smith]

================
Comment at: docs/Modules.rst:243
@@ -242,3 +242,3 @@
 -----------------
-Module map files use a simplified form of the C99 lexer, with the same rules for identifiers, tokens, string literals, ``/* */`` and ``//`` comments. The module map language has the following reserved words; all other C identifiers are valid identifiers.
+Module map files use a simplified form of the C99 lexer, with the same rules for identifiers, tokens, string literals, ``/* */`` and ``//`` comments. The module map language has the following reserved words; all other C identifiers are valid identifiers. Additionally, string literals can also be used to specify module names.
 
----------------
This seems out of place here.

================
Comment at: docs/Modules.rst:267
@@ -266,3 +266,3 @@
   *module-id*:
     *identifier* ('.' *identifier*)*
 
----------------
Please update the grammar for this feature: change `identifier` to `module-name` here and add a grammar rule:

  *module-name*:
    *identifier*
    *string-literal*

... and some words specifying that `identifier` is equivalent to `"identifier"`.

================
Comment at: docs/Modules.rst:275-279
@@ -274,7 +274,7 @@
 
   *module-declaration*:
     ``explicit``:sub:`opt` ``framework``:sub:`opt` ``module`` *module-id* *attributes*:sub:`opt` '{' *module-member** '}'
     ``extern`` ``module`` *module-id* *string-literal*
 
 The *module-id* should consist of only a single *identifier*, which provides the name of the module being defined. Each module shall have a single definition. 
 
----------------
Change `*module-id*` to `*module-name*` here and remove the redundant "only a single *identifier*" restriction.

================
Comment at: docs/Modules.rst:499-501
@@ -498,5 +498,5 @@
   *wildcard-module-id*:
     *identifier*
     '*'
     *identifier* '.' *wildcard-module-id*
 
----------------
Change `*identifier*` to `*module-name*` here.

================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:617
@@ -616,3 +616,3 @@
 def error_undeclared_use_of_module : Error<
-  "use of a module not declared used: '%0'">;
+  "module %0 does not depend on a module exporting: '%1'">;
 
----------------
Remove colon here, please.

================
Comment at: lib/Lex/ModuleMap.cpp:1065
@@ -1065,2 +1064,3 @@
+    if (Tok.is(MMToken::Identifier) || Tok.is(MMToken::StringLiteral)) {
       Id.push_back(std::make_pair(Tok.getString(), Tok.getLocation()));
       consumeToken();
----------------
Maybe remove quotes here?

================
Comment at: lib/Lex/Preprocessor.cpp:750-757
@@ -751,1 +749,10 @@
+      (Result.is(tok::identifier) || Result.is(tok::string_literal))) {
+    if (Result.is(tok::string_literal)) {
+      // String literals are valid module identifiers. Set token kind and
+      // identifier information appropriately.
+      StringRef WithoutQuotes(Result.getLiteralData(), Result.getLength());
+      WithoutQuotes = WithoutQuotes.substr(1, WithoutQuotes.size() - 2);
+      Result.setIdentifierInfo(&Identifiers.get(WithoutQuotes));
+      Result.setKind(tok::identifier);
+    }
     ModuleImportPath.push_back(std::make_pair(Result.getIdentifierInfo(),
----------------
You should check for, and reject, a ud-suffix here. Should we allow implicit string literal concatenation here?

================
Comment at: lib/Lex/ModuleMap.cpp:978-981
@@ -977,6 +977,6 @@
     // Parse the string literal.
     LangOptions LangOpts;
     StringLiteralParser StringLiteral(&LToken, 1, SourceMgr, LangOpts, *Target);
     if (StringLiteral.hadError)
       goto retry;
     
----------------
Should we perform string literal concatenation here?

================
Comment at: test/Modules/Inputs/declare-use/h.h:4
@@ -3,3 +3,3 @@
 #include "c.h"
-#include "d.h" // expected-error {{use of a module not declared used}}
+#include "d.h" // expected-error {{does not depend on a module exporting:}}
 #include "h1.h"
----------------
Please check the right values are substituted into this diagnostic in at least one test.


http://llvm-reviews.chandlerc.com/D2024