Skip to content

Commit 04e9520

Browse files
committed
[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
1 parent 81c528d commit 04e9520

File tree

4 files changed

+17
-6
lines changed

4 files changed

+17
-6
lines changed

clang/include/clang/Basic/SourceLocation.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ class FileEntry;
363363
/// A SourceLocation and its associated SourceManager.
364364
///
365365
/// This is useful for argument passing to functions that expect both objects.
366+
///
367+
/// This class does not guarantee the presence of either the SourceManager or
368+
/// a valid SourceLocation. Clients should use `isValid()` and `hasManager()`
369+
/// before calling the member functions.
366370
class FullSourceLoc : public SourceLocation {
367371
const SourceManager *SrcMgr = nullptr;
368372

@@ -373,13 +377,10 @@ class FullSourceLoc : public SourceLocation {
373377
explicit FullSourceLoc(SourceLocation Loc, const SourceManager &SM)
374378
: SourceLocation(Loc), SrcMgr(&SM) {}
375379

376-
bool hasManager() const {
377-
bool hasSrcMgr = SrcMgr != nullptr;
378-
assert(hasSrcMgr == isValid() && "FullSourceLoc has location but no manager");
379-
return hasSrcMgr;
380-
}
380+
/// Checks whether the SourceManager is present.
381+
bool hasManager() const { return SrcMgr != nullptr; }
381382

382-
/// \pre This FullSourceLoc has an associated SourceManager.
383+
/// \pre hasManager()
383384
const SourceManager &getManager() const {
384385
assert(SrcMgr && "SourceManager is NULL.");
385386
return *SrcMgr;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void a() __attribute__((deprecated));
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module a { header "a.h" }
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: rm -rf %t && mkdir %t
2+
// RUN: %clang_cc1 -fmodules -x c++ %S/Inputs/explicit-build-diags/module.modulemap -fmodule-name=a -emit-module -o %t/a.pcm
3+
// RUN: %clang_cc1 -fmodules -Wdeprecated-declarations -fdiagnostics-show-note-include-stack -serialize-diagnostic-file %t/tu.dia \
4+
// RUN: -I %S/Inputs/explicit-build-diags -fmodule-file=%t/a.pcm -fsyntax-only %s
5+
6+
#include "a.h"
7+
8+
void foo() { a(); }

0 commit comments

Comments
 (0)