[clang] 4aafd5f - [clang] Remove misleading assertion in FullSourceLoc

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-08-06T14:48:28+02:00
New Revision: 4aafd5f00c2a772337ec065d4542ef158453a343

URL: https://github.com/llvm/llvm-project/commit/4aafd5f00c2a772337ec065d4542ef158453a343
DIFF: https://github.com/llvm/llvm-project/commit/4aafd5f00c2a772337ec065d4542ef158453a343.diff

LOG: [clang] Remove misleading assertion in FullSourceLoc

D31709 added an assertion was added to `FullSourceLoc::hasManager()` that ensured a valid `SourceLocation` is always paired with a `SourceManager`, and missing `SourceManager` is always paired with an invalid `SourceLocation`.

This appears to be incorrect, since clients never cared about constructing `FullSourceLoc` to uphold that invariant, or always checking `isValid()` before calling `hasManager()`.

The assertion started failing when serializing diagnostics pointing into an explicit module. Explicit modules don't have valid `SourceLocation` for the `import` statement, since they are "imported" from the command-line argument `-fmodule-name=x.pcm`.

This patch removes the assertion, since `FullSourceLoc` was never intended to uphold any kind of invariants between the validity of `SourceLocation` and presence of `SourceManager`.

Reviewed By: arphaman

Differential Revision: https://reviews.llvm.org/D106862

Added: 
    clang/test/Modules/Inputs/explicit-build-diags/a.h
    clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
    clang/test/Modules/explicit-build-diags.cpp

Modified: 
    clang/include/clang/Basic/SourceLocation.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/SourceLocation.h b/clang/include/clang/Basic/SourceLocation.h
index 540de23b9f55e..ba2e9156a2b12 100644
--- a/clang/include/clang/Basic/SourceLocation.h
+++ b/clang/include/clang/Basic/SourceLocation.h
@@ -363,6 +363,10 @@ class FileEntry;
 /// A SourceLocation and its associated SourceManager.
 ///
 /// This is useful for argument passing to functions that expect both objects.
+///
+/// This class does not guarantee the presence of either the SourceManager or
+/// a valid SourceLocation. Clients should use `isValid()` and `hasManager()`
+/// before calling the member functions.
 class FullSourceLoc : public SourceLocation {
   const SourceManager *SrcMgr = nullptr;
 
@@ -373,13 +377,10 @@ class FullSourceLoc : public SourceLocation {
   explicit FullSourceLoc(SourceLocation Loc, const SourceManager &SM)
       : SourceLocation(Loc), SrcMgr(&SM) {}
 
-  bool hasManager() const {
-      bool hasSrcMgr =  SrcMgr != nullptr;
-      assert(hasSrcMgr == isValid() && "FullSourceLoc has location but no manager");
-      return hasSrcMgr;
-  }
+  /// Checks whether the SourceManager is present.
+  bool hasManager() const { return SrcMgr != nullptr; }
 
-  /// \pre This FullSourceLoc has an associated SourceManager.
+  /// \pre hasManager()
   const SourceManager &getManager() const {
     assert(SrcMgr && "SourceManager is NULL.");
     return *SrcMgr;

diff  --git a/clang/test/Modules/Inputs/explicit-build-diags/a.h b/clang/test/Modules/Inputs/explicit-build-diags/a.h
new file mode 100644
index 0000000000000..486941dde83b1
--- /dev/null
+++ b/clang/test/Modules/Inputs/explicit-build-diags/a.h
@@ -0,0 +1 @@
+void a() __attribute__((deprecated));

diff  --git a/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
new file mode 100644
index 0000000000000..bb00c840ce39d
--- /dev/null
+++ b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" }

diff  --git a/clang/test/Modules/explicit-build-diags.cpp b/clang/test/Modules/explicit-build-diags.cpp
new file mode 100644
index 0000000000000..4a37dc108a688
--- /dev/null
+++ b/clang/test/Modules/explicit-build-diags.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -fmodules -x c++ %S/Inputs/explicit-build-diags/module.modulemap -fmodule-name=a -emit-module -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -Wdeprecated-declarations -fdiagnostics-show-note-include-stack -serialize-diagnostic-file %t/tu.dia \
+// RUN:   -I %S/Inputs/explicit-build-diags -fmodule-file=%t/a.pcm -fsyntax-only %s
+
+#include "a.h"
+
+void foo() { a(); }