[PATCH] D106862: [clang][modules] Avoid creating partial FullSourceLoc for explicit modules imports

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 363033.
jansvoboda11 edited the summary of this revision.
jansvoboda11 added a comment.

Remove assertion altogether.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106862/new/

https://reviews.llvm.org/D106862

Files:
  clang/include/clang/Basic/SourceLocation.h
  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


Index: clang/test/Modules/explicit-build-diags.cpp
===================================================================
--- /dev/null
+++ 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(); }
Index: clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" }
Index: clang/test/Modules/Inputs/explicit-build-diags/a.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-diags/a.h
@@ -0,0 +1 @@
+void a() __attribute__((deprecated));
Index: clang/include/clang/Basic/SourceLocation.h
===================================================================
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -363,6 +363,10 @@
 /// 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 @@
   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;



Index: clang/test/Modules/explicit-build-diags.cpp
===================================================================
--- /dev/null
+++ 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(); }
Index: clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" }
Index: clang/test/Modules/Inputs/explicit-build-diags/a.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-diags/a.h
@@ -0,0 +1 @@
+void a() __attribute__((deprecated));
Index: clang/include/clang/Basic/SourceLocation.h
===================================================================
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -363,6 +363,10 @@
 /// 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 @@
   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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits