[clang] [clang][Modules] make ActOnPrivateModuleFragmentDecl return non-null on success. (PR #223757)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author:  Mahmoud Ahmed (anondeveg)

<details>
<summary>Changes</summary>

his code
```cpp
module;
module :private;
export module Foo;
```
a Private module fragment here is illegal because it can only be declared in a primary module interface unit, instead of raising an error clang crashed because of this assertion
```cpp
  assert((!getLangOpts().CPlusPlusModules ||
          SeenGMF == (bool)this->TheGlobalModuleFragment) &&
         "mismatched global module state");
``` 
the mismatch happened because `ParseModuleDecl`  assigned ImporState wether or not it was semantically correct:
```cpp
 ImportState = ImportState == Sema::ModuleImportState::ImportAllowed
                      ? Sema::ModuleImportState::PrivateFragmentImportAllowed
                      : Sema::ModuleImportState::PrivateFragmentImportFinished;
    return Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
  }
```
Also `ActOnPrivateModuleFragmentDecl` was returning `nullptr` in both failure and success cases. 

So i made it return a non-null object and only assign the ImportState on sema failure. 

fixes: #<!-- -->219950 

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


2 Files Affected:

- (modified) clang/lib/Parse/Parser.cpp (+4-2) 
- (modified) clang/lib/Sema/SemaModule.cpp (+1-1) 


``````````diff
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index bad81ea92cd2d..319a985655144 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2360,12 +2360,14 @@ Parser::ParseModuleDecl(Sema::ModuleImportState &ImportState) {
     SourceLocation PrivateLoc = ConsumeToken();
     DiagnoseAndSkipCXX11Attributes();
     ExpectAndConsumeSemi(diag::err_private_module_fragment_expected_semi);
+    auto Result = Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
+    if(Result){
     ImportState = ImportState == Sema::ModuleImportState::ImportAllowed
                       ? Sema::ModuleImportState::PrivateFragmentImportAllowed
                       : Sema::ModuleImportState::PrivateFragmentImportFinished;
-    return Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
+    }
+    return nullptr;
   }
-
   SmallVector<IdentifierLoc, 2> Path;
   if (ParseModuleName(ModuleLoc, Path, /*IsImport*/ false))
     return nullptr;
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index d7a182fe5654c..b769f133359e8 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -566,7 +566,7 @@ Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
   TU->setLocalOwningModule(PrivateModuleFragment);
 
   // FIXME: Consider creating an explicit representation of this declaration.
-  return nullptr;
+  return ConvertDeclToDeclGroup(TU);
 }
 
 DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,

``````````

</details>


https://github.com/llvm/llvm-project/pull/223757