Skip to content

Commit

Permalink
[Modules] Fix ModuleDeclState transition when module is used as a reg…
Browse files Browse the repository at this point in the history
…ular identifier (#71134)

`ModuleDeclState` is incorrectly changed to `NamedModuleImplementation`
for `struct module {}; void foo(module a);`. This is mostly benign but
leads to a spurious warning after #69555.

A real world example is:
```
// pybind11.h
class module_ { ... };
using module = module_;

// tensorflow
void DefineMetricsModule(pybind11::module main_module);
// `module main_module);` incorrectly changes `ModuleDeclState` to `NamedModuleImplementation`

#include <algorithm> // spurious warning
```
  • Loading branch information
MaskRay committed Nov 3, 2023
1 parent 7443af1 commit 071f3b5
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 18 deletions.
39 changes: 21 additions & 18 deletions clang/lib/Lex/Preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -957,26 +957,29 @@ void Preprocessor::Lex(Token &Result) {
ModuleDeclState.handlePeriod();
break;
case tok::identifier:
if (Result.getIdentifierInfo()->isModulesImport()) {
TrackGMFState.handleImport(StdCXXImportSeqState.afterTopLevelSeq());
StdCXXImportSeqState.handleImport();
if (StdCXXImportSeqState.afterImportSeq()) {
ModuleImportLoc = Result.getLocation();
NamedModuleImportPath.clear();
IsAtImport = false;
ModuleImportExpectsIdentifier = true;
CurLexerKind = CLK_LexAfterModuleImport;
}
break;
} else if (Result.getIdentifierInfo() == getIdentifierInfo("module")) {
TrackGMFState.handleModule(StdCXXImportSeqState.afterTopLevelSeq());
ModuleDeclState.handleModule();
break;
} else {
ModuleDeclState.handleIdentifier(Result.getIdentifierInfo());
if (ModuleDeclState.isModuleCandidate())
// Check "import" and "module" when there is no open bracket. The two
// identifiers are not meaningful with open brackets.
if (StdCXXImportSeqState.atTopLevel()) {
if (Result.getIdentifierInfo()->isModulesImport()) {
TrackGMFState.handleImport(StdCXXImportSeqState.afterTopLevelSeq());
StdCXXImportSeqState.handleImport();
if (StdCXXImportSeqState.afterImportSeq()) {
ModuleImportLoc = Result.getLocation();
NamedModuleImportPath.clear();
IsAtImport = false;
ModuleImportExpectsIdentifier = true;
CurLexerKind = CLK_LexAfterModuleImport;
}
break;
} else if (Result.getIdentifierInfo() == getIdentifierInfo("module")) {
TrackGMFState.handleModule(StdCXXImportSeqState.afterTopLevelSeq());
ModuleDeclState.handleModule();
break;
}
}
ModuleDeclState.handleIdentifier(Result.getIdentifierInfo());
if (ModuleDeclState.isModuleCandidate())
break;
[[fallthrough]];
default:
TrackGMFState.handleMisc();
Expand Down
8 changes: 8 additions & 0 deletions clang/test/Preprocessor/include-in-module-purview.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o %t/tmp 2>&1 | FileCheck %t/a.cppm
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o - 2>&1 \
// RUN: -Wno-include-angled-in-module-purview | FileCheck %t/a.cppm --check-prefix=CHECK-NO-WARN
// RUN: %clang_cc1 -std=c++20 %t/b.cpp -E -P -I%t -o - 2>&1 | FileCheck %t/a.cppm --check-prefix=CHECK-NO-WARN

//--- a.h
// left empty
Expand Down Expand Up @@ -58,3 +59,10 @@ module :private;
// CHECK: 10 warnings generated.

// CHECK-NO-WARN-NOT: warning

//--- b.cpp
/// Don't recognize `module m);` as a module purview or report a spurious
/// warning for <stddef.h>.
struct module {};
void foo(module m);
#include <stddef.h>

0 comments on commit 071f3b5

Please sign in to comment.