[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Aaron Ballman via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

AaronBallman wrote:

> I am not sure I under why we would actually consume ATTRS in either case, but 
> I think I need to think about it more / play with the PR.

I'd like to understand this better, too. I would have (naively and without 
checking in a debugger) that we would have peeked at `ATTRS` rather than 
consumed it.

> @AaronBallman Any opinion?

I think your suggestion makes sense to me; that's a good use of annotation 
tokens to squirrel information from the preprocessor into the parser, and 
keeping the logic consolidated as much as possible is preferred.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

cor3ntin wrote:

> We try to consume all module name tokens and return a token::cxx_module_name, 
> which includes {‘a’, ‘.’, ‘FUNC_LIKE’, ‘.’, ‘:’, ‘c’}, and we also will 
> consumed and stop at ‘ATTRS’, but it’s not a part of module name, and need to 
> be put back to token stream. Current a Lex action cannot both return a token 
> and cache tokens (doing so would corrupt the token cache if the call to Lex 
> comes from CachingLex / PeekAhead).

I've been thinking about that.
I think _both_ the annotation and the peeked token need to be re-entered
and then you can call PP.Lex(Result) again and return.

Is that something you would be willing to explore?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -862,6 +862,15 @@ bool Preprocessor::HandleIdentifier(Token ) {
 ModuleImportExpectsIdentifier = true;
 CurLexerCallback = CLK_LexAfterModuleImport;
   }
+
+  if ((II.isModulesDeclaration() || Identifier.is(tok::kw_module)) &&
+  !InMacroArgs && !DisableMacroExpansion &&
+  (getLangOpts().Modules || getLangOpts().DebuggerSupport) &&

yronglin wrote:

Good catch! This is definitely a mistake. It should be `CPlusPlusModules `. And 
the `DebuggerSupport` was used for debug mode, I followed what `import` doing.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }

yronglin wrote:

That's makes sense. Excessive consumed tokens may be wrong.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

cor3ntin wrote:

I am not sure I under why we would actually consume `ATTRS` in either case, but 
I think I need to think about it more / play with the PR.

@AaronBallman Any opinion?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -862,6 +862,15 @@ bool Preprocessor::HandleIdentifier(Token ) {
 ModuleImportExpectsIdentifier = true;
 CurLexerCallback = CLK_LexAfterModuleImport;
   }
+
+  if ((II.isModulesDeclaration() || Identifier.is(tok::kw_module)) &&
+  !InMacroArgs && !DisableMacroExpansion &&
+  (getLangOpts().Modules || getLangOpts().DebuggerSupport) &&

cor3ntin wrote:

Can you explain these two options? I would expect that to be `CPlusPlusModules`

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }

cor3ntin wrote:

I'm not sure we can read that many tea leaves. I would eat everything until the 
next semi colon (or let the parser do that)

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

yronglin wrote:

Friendly ping~

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin deleted 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

yronglin wrote:

friendly ping~

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

yronglin wrote:

Sorry for the very late reply! I’ve tried to implement this approach. But I’m 
fall into trouble. 

```
#ifndef VERSION_H
#define VERSION_H

#define VERSION libv5
#define A a
#define B b
#define C c
#define FUNC_LIKE(X) function_like_##X
#define ATTR [[]]
#define SEMICOLON ;

#endif


export module a.FUNC_LIKE:c ATTRS; // OK, FUNC_LIKE would not be treated as a 
macro name.
```

*The 1st approach*:

We try to consume all module name tokens and return a token::cxx_module_name, 
which includes {‘a’, ‘.’, ‘FUNC_LIKE’, ‘.’, ‘:’, ‘c’}, and we also will 
consumed and stop at ‘ATTRS’, but it’s not a part of module name, and need to 
be put back to token stream. Current a Lex action cannot both return a token 
and cache tokens (doing so would corrupt the token cache if the call to Lex 
comes from CachingLex / PeekAhead).

*The 2nd approach*:

We try to consume all module name tokens and got {‘a’, ‘.’, ‘FUNC_LIKE’, ‘.’, 
‘:’, ‘c’} we need to put this token array back to token stream with macro 
expansion disabled. Also, we have consumed and stop at ‘ATTRS’, but it’s not a 
part of module name, and need to be put back to token stream(enable macro 
expansion). The two EnterTokenStreans are conflict.

I also tried to eliminate the complex state machine in LexAfterModuleImport.

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

cor3ntin wrote:

> Introduce a new cxx_module_name also looks good to me, it's looks like the 
> approach processing #pragma.

Let's try to do that then

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 01/11] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20?=
 =?UTF-8?q?Declarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f4..40c6bd63e9948 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d95..0d4b526ec6d15 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78..ef66348a83125 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 0..b439366db3fba
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 0..4608934290950
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571..aa4bb52a57fac 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and known module. 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 01/11] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20?=
 =?UTF-8?q?Declarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f4..40c6bd63e9948 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d95..0d4b526ec6d15 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78..ef66348a83125 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 0..b439366db3fba
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 0..4608934290950
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571..aa4bb52a57fac 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and known module. 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -520,6 +524,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   RecomputeNeedsHandleIdentifier();
   }
 
+  /// Determine whether this is the contextual keyword \c module.
+  bool isModulesDecl() const { return IsModulesDecl; }
+
+  /// Set whether this identifier is the contextual keyword \c module.
+  void setModulesDecl(bool I) {

yronglin wrote:

It seems that the setter methods of other flags in `IdentifierInfo` have a bool 
parameter, so I kept the same format.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -932,6 +932,12 @@ def warn_module_conflict : Warning<
   InGroup;
 
 // C++20 modules
+def err_module_decl_cannot_be_macros : Error<
+  "the module name in a module%select{| partition}0 declaration cannot contain 
"
+  "an object-like macro %1, it's consists of one or more identifiers "
+  "separated by '.'">;

yronglin wrote:

Agree, I want to add a more help information into diagnostic to address 
@Bigcheese 's comments before. I don’t know if we need to add a link to clang’s 
documentation(or other helpful informations). I don’t know if there is any 
precedent before.

> It would be nice in cases like this if we just linked to some documentation 
> saying what to do instead.



https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }

yronglin wrote:

This recovery was used to handle cases like the following:
```
export module A.FUNC_LIKE(B c:C ATTRS
export module A.FUNC_LIKE(B,). c:C ATTRS
export module A.FUNC_LIKE(B,) c:C ATTRS
```
I want to recover from the error as much as possible, jumping to the next '.' 
or ':' when `(` occurs.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

yronglin wrote:

> What is the motivation to put the logic in the preprocessor ? It does adds 
> quite a bit of complexity to the state machine.

There are two reasons. The 1st is to be consistent with the way that `import` 
was processed. The 2nd reason is that some steps in Parser will look ahead of 
the token before ParseModuleDecl. In ParseModuleName, causing the first 
identifier was macro expanded token.  So once we saw a `module` keyword, The 
next token and the token conforming to the form `identifier(.identifier)*` must 
be processed in `Preprocessor::LexAfterModuleDecl`.

Introduce a new `cxx_module_name` also looks good to me, it's looks like the 
approach processing `#pragma`.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -520,6 +524,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   RecomputeNeedsHandleIdentifier();
   }
 
+  /// Determine whether this is the contextual keyword \c module.
+  bool isModulesDecl() const { return IsModulesDecl; }

yronglin wrote:

Agree!

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -932,6 +932,12 @@ def warn_module_conflict : Warning<
   InGroup;
 
 // C++20 modules
+def err_module_decl_cannot_be_macros : Error<
+  "the module name in a module%select{| partition}0 declaration cannot contain 
"
+  "an object-like macro %1, it's consists of one or more identifiers "
+  "separated by '.'">;

cor3ntin wrote:

```suggestion
  "an object-like macro %1">;
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -520,6 +524,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   RecomputeNeedsHandleIdentifier();
   }
 
+  /// Determine whether this is the contextual keyword \c module.
+  bool isModulesDecl() const { return IsModulesDecl; }

cor3ntin wrote:

can you say `Declaration`? (to avoid confusing with the Decl` class?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }

cor3ntin wrote:

I'm not sure there is any value in trying to recover here.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -520,6 +524,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   RecomputeNeedsHandleIdentifier();
   }
 
+  /// Determine whether this is the contextual keyword \c module.
+  bool isModulesDecl() const { return IsModulesDecl; }
+
+  /// Set whether this identifier is the contextual keyword \c module.
+  void setModulesDecl(bool I) {

cor3ntin wrote:

Why I?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we don't expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  Diag(Result, diag::err_module_decl_cannot_be_macros)
+  << Result.getLocation() << ModuleDeclLexingPartitionName
+  << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait 
until
+  // we see the next identifier.
+  if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon)) 
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+  }
+
+  // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+  // preprocessing token [...]
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+  LexUnexpandedToken(Tok);
+  if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon)) 
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+  }
+  if (Tok.is(tok::l_paren))
+NumParens++;
+  else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+  }
+
+  return true;
+}
+

cor3ntin wrote:

What is the motivation to put the logic in the preprocessor ? It does adds 
quite a bit of complexity to the state machine.
But I struggle to make up my mind on what the better approach is.

I kept coming back to the idea of using an annotation

- Introduce a new `cxx_module_name` token annotation in `TokenKind.defs`
- Parse the full module name in the preprocessor
- Store it in  a `Preprocessor` or `Lexer` member (we need 2 
`SmallVectorImpl>`, one for the 
partition, one for the name)
- The value of that annotation would be a pointer to that pair of vector
- `ParseModuleName` would handle `cxx_module_name` by moving the content of the 
vector.
(It w

- That way `LexAfterModuleDecl` can just produce a single token with a fully 
parsed module name and we avoid having the logic spread in multiple places with 
complex state machines.


---

Alternatively, and perhaps much simpler,  lex the whole module name + partition 
at once here do all the checks and diagnostics and reinject all of these tokens 
all at once.

... actually, that the second solution should work nicely, right?


WDYT?


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/cor3ntin commented:

I'm starting to find the state machine

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

Should we continue build a module declaration even if error occurred when 
parsing module names?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

Thanks for your review @Bigcheese !

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -932,6 +932,12 @@ def warn_module_conflict : Warning<
   InGroup;
 
 // C++20 modules
+def err_module_decl_cannot_be_macros : Error<
+  "the name of a module%select{| partition}0 declaration cannot contains "
+  "an object-like macro %1, and the macro will not expand"
+  "%select{|; did you mean '%3'?}2">;

yronglin wrote:

Agree, with a little improve ` it's consists of one or more identifiers 
separated by '.'`.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  auto BuildFixItHint =
+  [&](std::string ) -> std::optional {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+if (!CurTokenLexer)
+  return std::nullopt;
+Token Tok;
+bool HasUnknownToken = false;
+llvm::raw_string_ostream OS(Replacement);
+do {
+  Lex(Tok);
+  if (const char *Punc = tok::getPunctuatorSpelling(Tok.getKind()))
+OS << Punc;
+  else if (Tok.isLiteral() && Tok.getLiteralData())
+OS << StringRef(Tok.getLiteralData(), Tok.getLength());
+  else if (auto *II = Tok.getIdentifierInfo())
+OS << II->getName();
+  else
+HasUnknownToken = true;
+} while (CurTokenLexer && !CurTokenLexer->isAtEnd());
+if (HasUnknownToken)
+  return std::nullopt;

yronglin wrote:

I've improve diagnostic message by add `it's consists of one or more 
identifiers separated by '.'`, WDYT?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,87 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/D.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/E.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/F.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/G.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/H.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/I.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/J.cppm -triple x86_64-linux-gnu -verify
+
+//--- version.h
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+#define C c
+#define FUNC_LIKE(X) function_like_##X
+#define ATTRS [[]]
+#define SEMICOLON ;
+
+#endif
+
+//--- A.cppm
+#include "version.h"

yronglin wrote:

Thanks, looks like clang needs fixing, and I've added a `module;` 

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  auto BuildFixItHint =
+  [&](std::string ) -> std::optional {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+if (!CurTokenLexer)
+  return std::nullopt;
+Token Tok;
+bool HasUnknownToken = false;
+llvm::raw_string_ostream OS(Replacement);
+do {
+  Lex(Tok);
+  if (const char *Punc = tok::getPunctuatorSpelling(Tok.getKind()))
+OS << Punc;
+  else if (Tok.isLiteral() && Tok.getLiteralData())
+OS << StringRef(Tok.getLiteralData(), Tok.getLength());
+  else if (auto *II = Tok.getIdentifierInfo())
+OS << II->getName();
+  else
+HasUnknownToken = true;
+} while (CurTokenLexer && !CurTokenLexer->isAtEnd());

yronglin wrote:

I've remove this FixItHint here, I'd like to add a single API in a separate 
patch.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.

yronglin wrote:

Done!

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 01/10] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20?=
 =?UTF-8?q?Declarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f4..40c6bd63e9948 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d95..0d4b526ec6d15 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78..ef66348a83125 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 0..b439366db3fba
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 0..4608934290950
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571..aa4bb52a57fac 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and known module. 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

https://github.com/Bigcheese edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -932,6 +932,12 @@ def warn_module_conflict : Warning<
   InGroup;
 
 // C++20 modules
+def err_module_decl_cannot_be_macros : Error<
+  "the name of a module%select{| partition}0 declaration cannot contains "
+  "an object-like macro %1, and the macro will not expand"
+  "%select{|; did you mean '%3'?}2">;

Bigcheese wrote:

Not sure if you need "and the macro will not expand" here. Per the spec you 
don't even get to if expansion happens or not, it's just invalid being a macro.

```suggestion
def err_module_decl_cannot_be_macros : Error<
  "the module name in a module%select{| partition}0 declaration cannot contain "
  "an object-like macro %1%select{|; did you mean '%3'?}2">;
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  auto BuildFixItHint =
+  [&](std::string ) -> std::optional {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+if (!CurTokenLexer)
+  return std::nullopt;
+Token Tok;
+bool HasUnknownToken = false;
+llvm::raw_string_ostream OS(Replacement);
+do {
+  Lex(Tok);
+  if (const char *Punc = tok::getPunctuatorSpelling(Tok.getKind()))
+OS << Punc;
+  else if (Tok.isLiteral() && Tok.getLiteralData())
+OS << StringRef(Tok.getLiteralData(), Tok.getLength());
+  else if (auto *II = Tok.getIdentifierInfo())
+OS << II->getName();
+  else
+HasUnknownToken = true;
+} while (CurTokenLexer && !CurTokenLexer->isAtEnd());

Bigcheese wrote:

I forget now where else I made this comment, but this is probably the 5th or 
6th case of expanding a macro into a string in Clang. I'd really like to have a 
single API for doing this, as they are all slightly different.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -932,6 +932,12 @@ def warn_module_conflict : Warning<
   InGroup;
 
 // C++20 modules
+def err_module_decl_cannot_be_macros : Error<
+  "the name of a module%select{| partition}0 declaration cannot contains "
+  "an object-like macro %1, and the macro will not expand"
+  "%select{|; did you mean '%3'?}2">;
+def err_unxepected_paren_in_module_decl : Error<
+  "unexpected '(' after the name of a module%select{| partition}0 
declaration">;

Bigcheese wrote:

"name of a module declaration" is a bit odd. I think "module name in a module 
declaration" is clearer.

```suggestion
def err_unxepected_paren_in_module_decl : Error<
  "unexpected '(' after the module name in a module%select{| partition}0 
declaration">;
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  auto BuildFixItHint =
+  [&](std::string ) -> std::optional {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+if (!CurTokenLexer)
+  return std::nullopt;
+Token Tok;
+bool HasUnknownToken = false;
+llvm::raw_string_ostream OS(Replacement);
+do {
+  Lex(Tok);
+  if (const char *Punc = tok::getPunctuatorSpelling(Tok.getKind()))
+OS << Punc;
+  else if (Tok.isLiteral() && Tok.getLiteralData())
+OS << StringRef(Tok.getLiteralData(), Tok.getLength());
+  else if (auto *II = Tok.getIdentifierInfo())
+OS << II->getName();
+  else
+HasUnknownToken = true;
+} while (CurTokenLexer && !CurTokenLexer->isAtEnd());
+if (HasUnknownToken)
+  return std::nullopt;

Bigcheese wrote:

I believe this still allows a lot of invalid cases where applying the fixit 
produces an invalid module name. I expect two different cases of people hitting 
this.

1. Accidentally use the name of a macro
Here what the macro expands to doesn't matter, as they weren't trying to 
expand it.
2. Intentionally trying to use the preprocessor to set the module name
Here the expansion is probably valid, but they went out of their way to use 
the preprocessor, so they probably don't want to put in what the macro expands 
to.

In either case I'm not sure if the fixit is actually helpful here. My 
expectation is that Clang will either tell you to replace it with something 
like `export module static inline __attribute__((always_inline))`, or something 
you don't want to replace it with anyway. It would be nice in cases like this 
if we just linked to some documentation saying what to do instead.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.

Bigcheese wrote:

```suggestion
  // If we don't expect an identifier but got an identifier, it's not a part of
  // module name.
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -0,0 +1,87 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/D.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/E.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/F.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/G.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/H.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/I.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/J.cppm -triple x86_64-linux-gnu -verify
+
+//--- version.h
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+#define C c
+#define FUNC_LIKE(X) function_like_##X
+#define ATTRS [[]]
+#define SEMICOLON ;
+
+#endif
+
+//--- A.cppm
+#include "version.h"

Bigcheese wrote:

The module-file grammar prohibits this, but clang doesn't implement this 
restriction yet. https://eel.is/c++draft/cpp#nt:module-file

You need to have a `module;` decl before any preprocessor directives.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

@@ -1329,6 +1341,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'

Bigcheese wrote:

```suggestion
  // indicates a module declaration. We already saw the 'module'
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Michael Spencer via cfe-commits]

Bigcheese wrote:

> The paper does not clearly says whether disallow function-like macro is also 
> needed, but I think disallow function-like macro has the same goal as the 
> paper. WDYT? @cor3ntin @ChuanqiXu9
> 
> The wording in the paper said: _No identifier in the pp-module-name or 
> pp-module-partition shall currently be defined as an **object-like macro**._

The "where the pp-tokens (if any) shall not begin with a ( preprocessing token" 
prohibits function like macros already. The goal was to restrict only what was 
needed. So identifiers defined as function like macros are ok as long as it's 
not followed by `(`.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/9] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/8] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,80 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/D.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/E.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/F.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/G.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/H.cppm -triple x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 %t/I.cppm -triple x86_64-linux-gnu -verify
+
+//--- version.h
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+#define C c
+#define FUNC_LIKE(X) function_like_##X
+#define ATTR [[]]
+
+#endif
+
+//--- A.cppm
+#include "version.h"
+export module VERSION;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'VERSION', and the macro will not expand}}
+
+//--- B.cppm
+#include "version.h"
+export module A.B;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'A', and the macro will not expand}} \
+// expected-error {{the name of a module declaration 
cannot contains an object-like macro 'B', and the macro will not expand}}
+
+//--- C.cppm
+#include "version.h"
+export module A.FUNC_LIKE(foo):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand; did you mean 'a'?}} \
+// expected-error {{unexpected '(' after 
the name of a module declaration}} \
+// expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand}}
+
+//--- D.cppm
+#include "version.h"
+export module B.A.FUNC_LIKE(bar):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'B', and the macro will not 
expand; did you mean 'b'?}} \
+  // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand; did you mean 'a'?}} \
+  // expected-error {{unexpected '(' after 
the name of a module declaration}} \
+  // expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand; did you mean 'c'?}}
+
+//--- E.cppm
+#include "version.h"
+export module a.FUNC_LIKE:c; // OK, FUNC_LIKE would not be treated as a macro 
name.
+// expected-no-diagnostics
+
+//--- F.cppm
+#include "version.h"
+export module a.FUNC_LIKE:c ATTR; // OK, FUNC_LIKE would not be treated as a 
macro name.
+// expected-no-diagnostics
+
+//--- G.cppm
+#include "version.h"
+export module A.FUNC_LIKE(B c:C ATTR  // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand; did you mean 'a'?}} \

yronglin wrote:

The FixItHint looks like this.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/7] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1329,6 +1342,129 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
   return true;
 }
 
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+///   pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+///   pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+///   pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+///   pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+  // Figure out what kind of lexer we actually have.
+  recomputeCurLexerKind();
+  LexUnexpandedToken(Result);
+
+  auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+  };
+
+  // If we not expect an identifier but got an identifier, it's not a part of
+  // module name.
+  if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+  }
+
+  // The token sequence
+  //
+  // export[opt] module identifier (. identifier)*
+  //
+  // indicates a module import directive. We already saw the 'module'
+  // contextual keyword, so now we're looking for the identifiers.
+  if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+  auto BuildFixItHint =

yronglin wrote:

I tried to expand object-like macro and build a FixItHint, does it make sense? 
WDYT?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

> Either way, the parsing of semi conlon/attributes should stay in the parser.

Agree! It's still stay in the parser.



https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/6] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

cor3ntin wrote:

> Thanks! I'd like to move the implementation from Parser to Preprocessor. I 
> think It would make more sense to do these checks during lexing.

I'm not sure that would make a lot of sense (unless we were doing exactly how 
the specification does it, by injecting a token for a fully parsed module... 
but that does seem necessary).

Either way, the parsing of semi conlon/attributes should stay in the parser.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

Thanks! I'd like to move the implementation from Parser to Preprocessor. I 
think It would make more sense to do these checks during lexing.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

cor3ntin wrote:

> > I think the approach looks good.
> > Do we have existing tests for the use of `module` as an identifier outside 
> > of a module declaration?
> 
> IIUC, do you mean something looks like the following? I didn't find it in the 
> test case:
> 
> ```
> void foo() {
>   int module = 0;
> }
> ```

Yes, it might be useful to have that somewhere

I think in `ParseModuleDecl`, you want to reset the lexer kind before parsing 
attributes. 
(and add tests for that)

Here is a test:

```cpp
#define ATTRS [[]]
#define SEMICOLON
module unexpanded : unexpanded : ATTRS SEMICOLON
``` 

Sorry i did not notice that earlier.
The goal of the paper is that the name of the module is not a macro. everything 
else can be, including attributes attached to the module


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1

yronglin wrote:

Done!

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

> My reading is that
> 
> ```c++
> #define SOME_MACRO
> module foo SOME_MACRO;
> ```
> 
> SOME_MACRO is expanded

If this is the case,  the current processing in the 
`Preprocessor::LexAfterModuleDecl` function is incorrect. The implementation in 
this PR treats the tokens between `module` and `;` as normal text(use 
`LexUnexpandedToken`). So I'm a bit confused about the wording of 
[cpp.module]/p3 and the note.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

> I think the approach looks good.
> 
> Do we have existing tests for the use of `module` as an identifier outside of 
> a module declaration?

IIUC, do you mean something looks like the following? I didn't find it in the 
test case:
```
void foo() {
  int module = 0;
}
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,52 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/A.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/B.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/C.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/D.cppm -triple 
x86_64-linux-gnu -verify
+
+//--- version.h
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+#define C c
+#define FUNC_LIKE(X) function_like_##X
+
+#endif
+
+//--- A.cppm
+export module x;
+#include "version.h"
+export module VERSION;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'VERSION', and the macro will not expand}}
+
+//--- B.cppm
+export module x;
+#include "version.h"
+export module A.B;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'A', and the macro will not expand}} \
+// expected-error {{the name of a module declaration 
cannot contains an object-like macro 'B', and the macro will not expand}}
+
+//--- C.cppm
+export module x;
+#include "version.h"
+export module A.FUNC_LIKE(foo):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand}} \
+// expected-error {{the name of a module 
declaration cannot contains a function-like macro 'FUNC_LIKE', and the macro 
will not expand}} \
+// expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand}}
+
+//--- D.cppm
+export module x;
+#include "version.h"
+export module B.A.FUNC_LIKE(bar):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'B', and the macro will not 
expand}} \
+  // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand}} \
+  // expected-error {{the name of a module 
declaration cannot contains a function-like macro 'FUNC_LIKE', and the macro 
will not expand}} \
+  // expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand}}
+
+//--- E.cppm
+export module x;
+#include "version.h"
+export module a.FUNC_LIKE:c // OK, FUNC_LIKE would not be treated as a macro 
name.
+// expected-no-diagnostics

yronglin wrote:

Good catch!

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,52 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/A.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/B.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/C.cppm -triple 
x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/D.cppm -triple 
x86_64-linux-gnu -verify
+
+//--- version.h
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+#define C c
+#define FUNC_LIKE(X) function_like_##X
+
+#endif
+
+//--- A.cppm
+export module x;
+#include "version.h"
+export module VERSION;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'VERSION', and the macro will not expand}}
+
+//--- B.cppm
+export module x;
+#include "version.h"
+export module A.B;  // expected-error {{the name of a module declaration 
cannot contains an object-like macro 'A', and the macro will not expand}} \
+// expected-error {{the name of a module declaration 
cannot contains an object-like macro 'B', and the macro will not expand}}
+
+//--- C.cppm
+export module x;
+#include "version.h"
+export module A.FUNC_LIKE(foo):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand}} \
+// expected-error {{the name of a module 
declaration cannot contains a function-like macro 'FUNC_LIKE', and the macro 
will not expand}} \
+// expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand}}
+
+//--- D.cppm
+export module x;
+#include "version.h"
+export module B.A.FUNC_LIKE(bar):C;   // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'B', and the macro will not 
expand}} \
+  // expected-error {{the name of a module 
declaration cannot contains an object-like macro 'A', and the macro will not 
expand}} \
+  // expected-error {{the name of a module 
declaration cannot contains a function-like macro 'FUNC_LIKE', and the macro 
will not expand}} \
+  // expected-error {{the name of a module 
partition declaration cannot contains an object-like macro 'C', and the macro 
will not expand}}
+
+//--- E.cppm
+export module x;
+#include "version.h"
+export module a.FUNC_LIKE:c // OK, FUNC_LIKE would not be treated as a macro 
name.
+// expected-no-diagnostics

cor3ntin wrote:


Missing new line

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/cor3ntin commented:

I think the approach looks good.

Do we have existing tests for the use of `module` as an identifier outside of a 
module declaration?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

cor3ntin wrote:

> Dose this [note](https://eel.is/c++draft/cpp.module#note-1) have a conflict 
> with P3034R1?
> 
> ```
> [Note 1: Each identifier currently defined as a macro name is replaced by its 
> replacement list of preprocessing tokens. — end note]
> ```

My reading is that

```cpp
#define SOME_MACRO
module foo SOME_MACRO;
```

SOME_MACRO isd expanded


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

Dose this [note](https://eel.is/c++draft/cpp.module#note-1) have a conflict 
with P3034R1?
```
[Note 1: Each identifier currently defined as a macro name is replaced by its 
replacement list of preprocessing tokens. — end note]
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -2686,18 +2693,31 @@ bool Parser::ParseModuleName(
   }
 
   Diag(Tok, diag::err_module_expected_ident) << IsImport;
-  SkipUntil(tok::semi);
+  SkipUntil(tok::semi, StopBeforeMatch);
   return true;
 }
 
-// Record this part of the module path.
-Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
+Token Identifier = Tok;
 ConsumeToken();
 
-if (Tok.isNot(tok::period))
-  return false;
+// P3034R1: Module Declarations Shouldn’t be Macros.
+const auto *MI = PP.getMacroInfo(Identifier.getIdentifierInfo());
+if (!IsImport && MI) {
+  HasMacroInModuleName = true;
+  if (MI->isFunctionLike())
+SkipUntil(tok::r_paren, tok::period, tok::colon,
+  StopAtSemi | StopBeforeMatch);
+  Diag(Identifier, diag::err_module_decl_cannot_be_macros)
+  << Identifier.getLocation() << IsPartition << MI->isFunctionLike()
+  << Identifier.getIdentifierInfo();
+} else if (!HasMacroInModuleName) {
+  // Record this part of the module path.
+  Path.push_back(std::make_pair(Identifier.getIdentifierInfo(),
+Identifier.getLocation()));
+}

yronglin wrote:

Thanks! I've fixed this issue. Hmm, I tried to add a note diagnostic in local, 
but seems it's a bit noisy and redundant.
```
➜  test ../rel/bin/clang++ -c -std=c++20 ./lib.cppm
./lib.cppm:6:15: error: the name of a module declaration cannot contains an 
object-like macro 'A'
6 | export module A.FUNC_LIKE():CC;
  |   ^
./lib.cppm:6:15: note: object-like macro in the name of a module declaration 
will not expand
6 | export module A.FUNC_LIKE():CC;
  |   ^
./lib.cppm:6:17: error: the name of a module declaration cannot contains a 
function-like macro 'FUNC_LIKE'
6 | export module A.FUNC_LIKE():CC;
  | ^~~
./lib.cppm:6:17: note: function-like macro in the name of a module partition 
declaration will not expand
6 | export module A.FUNC_LIKE():CC;
  | ^~~
./lib.cppm:6:29: error: the name of a module partition declaration cannot 
contains an object-like macro 'CC'
6 | export module A.FUNC_LIKE():CC;
  | ^~
./lib.cppm:6:29: note: object-like macro in the name of a module declaration 
will not expand
6 | export module A.FUNC_LIKE():CC;
  | ^~
3 errors generated.
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/4] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/4] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -2686,18 +2693,31 @@ bool Parser::ParseModuleName(
   }
 
   Diag(Tok, diag::err_module_expected_ident) << IsImport;
-  SkipUntil(tok::semi);
+  SkipUntil(tok::semi, StopBeforeMatch);
   return true;
 }
 
-// Record this part of the module path.
-Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
+Token Identifier = Tok;
 ConsumeToken();
 
-if (Tok.isNot(tok::period))
-  return false;
+// P3034R1: Module Declarations Shouldn’t be Macros.
+const auto *MI = PP.getMacroInfo(Identifier.getIdentifierInfo());
+if (!IsImport && MI) {
+  HasMacroInModuleName = true;
+  if (MI->isFunctionLike())
+SkipUntil(tok::r_paren, tok::period, tok::colon,
+  StopAtSemi | StopBeforeMatch);
+  Diag(Identifier, diag::err_module_decl_cannot_be_macros)
+  << Identifier.getLocation() << IsPartition << MI->isFunctionLike()
+  << Identifier.getIdentifierInfo();
+} else if (!HasMacroInModuleName) {
+  // Record this part of the module path.
+  Path.push_back(std::make_pair(Identifier.getIdentifierInfo(),
+Identifier.getLocation()));
+}

cor3ntin wrote:

Having a function-like macro name in a module.
It would not be treated as a macro name because function-like macro are only 
replaced when a `(` follows, which can never be the case here.

in the tests below, `export module A.FUNC_LIKE` should be a valid module name
(and `export module A.FUNC_LIKE()` should be ill-formed with an `(` cannot 
appear in a module name,
or you can have a custom diag explaining that macros are not expanded in module 
names)


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 10aab63c9cb49d3ddfbe2cf8992de433efeef6f1 
0b472f255ca8f9279e58f25e2350cd0eb31baad7 -- 
clang/test/CXX/cpp/cpp.module/p2.cppm 
clang/include/clang/Basic/IdentifierTable.h 
clang/include/clang/Lex/Preprocessor.h clang/include/clang/Parse/Parser.h 
clang/lib/Basic/IdentifierTable.cpp clang/lib/Lex/PPLexerChange.cpp 
clang/lib/Lex/Preprocessor.cpp clang/lib/Parse/Parser.cpp 
clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.import/p1.cppm 
clang/test/SemaCXX/modules.cppm
``





View the diff from clang-format here.


``diff
diff --git a/clang/include/clang/Basic/IdentifierTable.h 
b/clang/include/clang/Basic/IdentifierTable.h
index 5bbb921955..645f7ae913 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -215,9 +215,9 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
 IsFutureCompatKeyword(false), IsPoisoned(false),
 IsCPPOperatorKeyword(false), NeedsHandleIdentifier(false),
 IsFromAST(false), ChangedAfterLoad(false), FEChangedAfterLoad(false),
-RevertedTokenID(false), OutOfDate(false), IsModulesImport(false), 
IsModulesDecl(false),
-IsMangledOpenMPVariantName(false), IsDeprecatedMacro(false),
-IsRestrictExpansion(false), IsFinal(false) {}
+RevertedTokenID(false), OutOfDate(false), IsModulesImport(false),
+IsModulesDecl(false), IsMangledOpenMPVariantName(false),
+IsDeprecatedMacro(false), IsRestrictExpansion(false), IsFinal(false) {}
 
 public:
   IdentifierInfo(const IdentifierInfo &) = delete;
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index b1571494f1..5982c9a72d 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -863,9 +863,8 @@ bool Preprocessor::HandleIdentifier(Token ) {
 CurLexerCallback = CLK_LexAfterModuleImport;
   }
 
-  if ((II.isModulesDecl() ||
-   Identifier.is(tok::kw_module)) &&
-  !InMacroArgs && !DisableMacroExpansion &&
+  if ((II.isModulesDecl() || Identifier.is(tok::kw_module)) && !InMacroArgs &&
+  !DisableMacroExpansion &&
   (getLangOpts().Modules || getLangOpts().DebuggerSupport) &&
   CurLexerCallback != CLK_CachingLexer) {
 CurLexerCallback = CLK_LexAfterModuleDecl;
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index f1e35028e1..2c45e037a1 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2705,14 +2705,15 @@ bool Parser::ParseModuleName(
 if (!IsImport && MI) {
   HasMacroInModuleName = true;
   if (MI->isFunctionLike())
-SkipUntil(tok::r_paren, tok::period, tok::colon, StopAtSemi | 
StopBeforeMatch);
+SkipUntil(tok::r_paren, tok::period, tok::colon,
+  StopAtSemi | StopBeforeMatch);
   Diag(Identifier, diag::err_module_decl_cannot_be_macros)
-<< Identifier.getLocation()
-<< IsPartition << MI->isFunctionLike()
-<< Identifier.getIdentifierInfo();
+  << Identifier.getLocation() << IsPartition << MI->isFunctionLike()
+  << Identifier.getIdentifierInfo();
 } else if (!HasMacroInModuleName) {
   // Record this part of the module path.
-  Path.push_back(std::make_pair(Identifier.getIdentifierInfo(), 
Identifier.getLocation()));
+  Path.push_back(std::make_pair(Identifier.getIdentifierInfo(),
+Identifier.getLocation()));
 }
 
 if (!TryConsumeToken(tok::period))

``




https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/3] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin updated 
https://github.com/llvm/llvm-project/pull/90574

>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH 1/2] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20De?=
 =?UTF-8?q?clarations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

> > This means that no macros of any kind can appear in the module name.
> 
> Exactly. This is why the grammar in the wording is defined in terms of 
> `pp-token` and not `identifiers`: no macro expansion takes place at all.

Thanks! Let's use `LexUnexpandedToken` during parsing module name.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

cor3ntin wrote:

> This means that no macros of any kind can appear in the module name.

Exactly. This is why the grammar in the wording is defined in terms of 
`pp-token` and not `identifiers`: no macro expansion takes place at all.


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

> module foo(); should be Ill-formed.

Ahh, so case like the following also should be ill-formed:
```
#define FUNC_LIKE(X) function_like_##X
export module FUNC_LIKE(bar);
```
> we can get the module name of a TU by cat and grep

This means that no macros of any kind can appear in the module name.

Thank you all. Things much clearer now.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> The paper does not clearly says whether disallow function-like macro is also 
> needed, but I think disallow function-like macro has the same goal as the 
> paper. WDYT? @cor3ntin @ChuanqiXu9
> 
> The wording in the paper said: _No identifier in the pp-module-name or 
> pp-module-partition shall currently be defined as an **object-like macro**._

The intention of the paper is, we can get the module name of a TU by `cat and 
grep`.  So I feel the current wording is correct: 
https://godbolt.org/z/45xnsh7Eh

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

cor3ntin wrote:

A function like macro needs parentheses to be substituted. As such,

#define foo()
module foo;

Should be allowed (and foo is not a macro expansion on line 2)

module foo(); should be Ill-formed.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

yronglin wrote:

The paper does not clearly state whether disallow function-like macro is also 
needed, but I think disallow function-like macro has the same goal as the 
paper. WDYT? @cor3ntin @ChuanqiXu9 

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1

yronglin wrote:

Agree!

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2

yronglin wrote:

Sure, I'd be happy to do this.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;

yronglin wrote:

Should we also print the macro spelling text in diagnostic message? And I found 
that the first letter in other diagnostic message is lower case, should we 
update `The name of a ...` to `the name of a ...`?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;

yronglin wrote:

I've a case like  the following:
```
// version.h
#ifndef VERSION_H
#define VERSION_H

#define VERSION libv5
#define A a
#define B b
#define AB A.B

#endif

// lib.cppm

module;
#include "version.h"
export module AB;
export module x:A;
export module VERSION;
export module A.B;
```
We get the following diagnostic message:
```
->  test ../rel/bin/clang++ -c -std=c++20 ./lib.cppm
./lib.cppm:3:15: error: module declaration cannot be a macro
3 | export module AB;
  |   ^
./version.h:7:12: note: expanded from macro 'AB'
7 | #define AB A.B
  |^
./version.h:5:11: note: expanded from macro 'A'
5 | #define A a
  |   ^
./lib.cppm:3:15: error: module declaration cannot be a macro
./version.h:7:14: note: expanded from macro 'AB'
7 | #define AB A.B
  |  ^
./version.h:6:11: note: expanded from macro 'B'
6 | #define B b
  |   ^
./lib.cppm:4:17: error: module declaration cannot be a macro
4 | export module x:A;
  | ^
./version.h:5:11: note: expanded from macro 'A'
5 | #define A a
  |   ^
./lib.cppm:5:15: error: module declaration cannot be a macro
5 | export module VERSION;
  |   ^
./version.h:4:17: note: expanded from macro 'VERSION'
4 | #define VERSION libv5
  | ^
./lib.cppm:6:15: error: module declaration cannot be a macro
6 | export module A.B;
  |   ^
./version.h:5:11: note: expanded from macro 'A'
5 | #define A a
  |   ^
./lib.cppm:6:17: error: module declaration cannot be a macro
6 | export module A.B;
  | ^
./version.h:6:11: note: expanded from macro 'B'
6 | #define B b
  |   ^
./lib.cppm:1:1: error: missing 'module' declaration at end of global module 
fragment introduced here
1 | module;
  | ^
7 errors generated.
```
I feel that `export module AB;`’s error report is not good enough. (Please 
ignore the error message wording, I am addressing review comments藍)


https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Chuanqi Xu via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1

ChuanqiXu9 wrote:

https://llvm.org/docs/TestingGuide.html#extra-files

Also it is somewhat clearly that split file is better than the sections guarded 
by #if-#endif. We can find the example by searching `split-file` under 
clang/test/Modules

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1

cor3ntin wrote:

Is that documented somewhere?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Chuanqi Xu via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1

ChuanqiXu9 wrote:

we prefer using split-file now.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 commented:

LGTM otherwise. I'd like to leave this to  @cor3ntin 

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2

cor3ntin wrote:

Can you add tests for module partitions?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;

cor3ntin wrote:

```suggestion
  "The name of a module|module partition declaration cannot contain a macro">;
```

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;

cor3ntin wrote:

I think we can continue here

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: None (yronglin)


Changes

This PR implement [P3034R1 Module Declarations Shouldn’t be 
Macros](https://wg21.link/P3034R1)


---
Full diff: https://github.com/llvm/llvm-project/pull/90574.diff


9 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+2) 
- (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2) 
- (modified) clang/lib/Parse/Parser.cpp (+7) 
- (added) clang/test/CXX/cpp/cpp.module/p1.cppm (+13) 
- (added) clang/test/CXX/cpp/cpp.module/version.h (+8) 
- (modified) clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
(+20-15) 
- (modified) clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.import/p1.cppm 
(+2-2) 
- (modified) clang/test/SemaCXX/modules.cppm (+4) 
- (modified) clang/www/cxx_status.html (+1-1) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2 -DEXPORT= -DMODULE_NAME=x
+// RUN:-DTEST=2
 //
 // Module interface for unknown and known module. (The latter is ill-formed 
due to
 // redefinition.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify 
%t/M.cpp \
-// RUN:-DTEST=3 -DEXPORT=export -DMODULE_NAME=z
+// RUN:-DTEST=3
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify 
%t/M.cpp \
-// 

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

[via cfe-commits]

https://github.com/yronglin created 
https://github.com/llvm/llvm-project/pull/90574

This PR implement [P3034R1 Module Declarations Shouldn’t be 
Macros](https://wg21.link/P3034R1)


>From 1dcb4c3ac1efaf3a6a4317751e23089a6c8ccac1 Mon Sep 17 00:00:00 2001
From: yronglin 
Date: Tue, 30 Apr 2024 17:18:26 +0800
Subject: [PATCH] =?UTF-8?q?[Clang]=20Implement=20P3034R1=20Module=20Declar?=
 =?UTF-8?q?ations=20Shouldn=E2=80=99t=20be=20Macros?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: yronglin 
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticParseKinds.td   |  2 ++
 clang/lib/Parse/Parser.cpp|  7 
 clang/test/CXX/cpp/cpp.module/p1.cppm | 13 +++
 clang/test/CXX/cpp/cpp.module/version.h   |  8 +
 .../basic/basic.link/module-declaration.cpp   | 35 +++
 .../dcl.module/dcl.module.import/p1.cppm  |  4 +--
 clang/test/SemaCXX/modules.cppm   |  4 +++
 clang/www/cxx_status.html |  2 +-
 9 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/CXX/cpp/cpp.module/p1.cppm
 create mode 100644 clang/test/CXX/cpp/cpp.module/version.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1abc00a25f1f42..40c6bd63e9948f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -153,6 +153,8 @@ C++2c Feature Support
 
 - Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary 
`_.
 
+- Implemented `P3034R1 Module Declarations Shouldn’t be Macros 
`_.
+
 Resolutions to C++ Defect Reports
 ^
 - Substitute template parameter pack, when it is not explicitly specified
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fdffb35ea0d955..0d4b526ec6d15a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1671,6 +1671,8 @@ def err_unexpected_module_decl : Error<
   "module declaration can only appear at the top level">;
 def err_module_expected_ident : Error<
   "expected a module name after '%select{module|import}0'">;
+def err_module_decl_cannot_be_macros : Error<
+  "module declaration cannot be a macro">;
 def err_attribute_not_module_attr : Error<
   "%0 attribute cannot be applied to a module">;
 def err_keyword_not_module_attr : Error<
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78e..ef66348a83125c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2690,6 +2690,13 @@ bool Parser::ParseModuleName(
   return true;
 }
 
+// P3034R1: Module Declarations Shouldn’t be Macros
+if (!IsImport && Tok.getLocation().isMacroID()) {
+  Diag(Tok, diag::err_module_decl_cannot_be_macros);
+  SkipUntil(tok::semi);
+  return true;
+}
+
 // Record this part of the module path.
 Path.push_back(std::make_pair(Tok.getIdentifierInfo(), Tok.getLocation()));
 ConsumeToken();
diff --git a/clang/test/CXX/cpp/cpp.module/p1.cppm 
b/clang/test/CXX/cpp/cpp.module/p1.cppm
new file mode 100644
index 00..b439366db3fba0
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/p1.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=1 -verify
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple 
x86_64-linux-gnu -DTEST=2 -verify
+
+module;
+export module x;
+#include "version.h"
+#if TEST == 1
+export module VERSION;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 1
+
+#if TEST == 2
+export module A.B;  // expected-error {{module declaration cannot be a 
macro}}
+#endif // TEST == 2
diff --git a/clang/test/CXX/cpp/cpp.module/version.h 
b/clang/test/CXX/cpp/cpp.module/version.h
new file mode 100644
index 00..4608934290950b
--- /dev/null
+++ b/clang/test/CXX/cpp/cpp.module/version.h
@@ -0,0 +1,8 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+#define VERSION libv5
+#define A a
+#define B b
+
+#endif
diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp 
b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
index d71358cc7a571f..aa4bb52a57face 100644
--- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp
@@ -9,26 +9,26 @@
 //
 // Module implementation for unknown and known module. (The former is 
ill-formed.)
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x.y=%t/x.y.pcm -verify -x c++ 
%t/M.cpp \
-// RUN:-DTEST=1 -DEXPORT= -DMODULE_NAME=z
+// RUN:-DTEST=1
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm 
-fmodule-file=x.y=%t/x.y.pcm -verify -x c++ %t/M.cpp \
-// RUN:-DTEST=2