================
@@ -1483,26 +1484,40 @@ void Preprocessor::emitFinalMacroWarning(const Token 
&Identifier,
 }
 
 bool Preprocessor::isSafeBufferOptOut(const SourceManager &SourceMgr,
-                                           const SourceLocation &Loc) const {
-  // Try to find a region in `SafeBufferOptOutMap` where `Loc` is in:
-  auto FirstRegionEndingAfterLoc = llvm::partition_point(
-      SafeBufferOptOutMap,
-      [&SourceMgr,
-       &Loc](const std::pair<SourceLocation, SourceLocation> &Region) {
-        return SourceMgr.isBeforeInTranslationUnit(Region.second, Loc);
-      });
+                                      const SourceLocation &Loc) const {
+  // The lambda that tests if a `Loc` is in an opt-out region given one opt-out
+  // region map:
+  auto TestInMap = [&SourceMgr](const SafeBufferOptOutRegionsTy &Map,
+                                const SourceLocation &Loc) -> bool {
+    // Try to find a region in `SafeBufferOptOutMap` where `Loc` is in:
+    auto FirstRegionEndingAfterLoc = llvm::partition_point(
+        Map, [&SourceMgr,
+              &Loc](const std::pair<SourceLocation, SourceLocation> &Region) {
+          return SourceMgr.isBeforeInTranslationUnit(Region.second, Loc);
+        });
+
+    if (FirstRegionEndingAfterLoc != Map.end()) {
+      // To test if the start location of the found region precedes `Loc`:
+      return SourceMgr.isBeforeInTranslationUnit(
+          FirstRegionEndingAfterLoc->first, Loc);
+    }
+    // If we do not find a region whose end location passes `Loc`, we want to
+    // check if the current region is still open:
+    if (!Map.empty() && Map.back().first == Map.back().second)
+      return SourceMgr.isBeforeInTranslationUnit(Map.back().first, Loc);
+    return false;
+  };
 
----------------
haoNoQ wrote:

Ok @ziqingluo-90 has just walked me through this offline and I think I 
understand what's happening now.

What was unobvious to me: shouldn't we `TestInMap` over the entire stack of 
includes? And the answer is:
- Regular textual `#include`s will just show up in the current map normally; 
there's no need to go up the stack.
- PCHes are going to be included with the `-include-pch` flag, not directly. 
There's not really much of a stack going on at this point and there's no way to 
"put pragmas around the compiler flag".
- In case of modules, we don't care what this function returns deep inside a 
module, because we probably shouldn't be analyzing that code in the first 
place. Instead, the warning should be naturally enabled in the clang invocation 
that builds the module itself, and that's where the warning would be emitted, 
taking only the current module into account.
  - As a matter of fact, as of today we're analyzing that nested code. But we 
shouldn't be, that's not how modules are intended to work. We consider it a bug 
and have a separate plan to fix this. Right now this means that the warning 
will be emitted multiple times for the same line of code (both by module 
compilation and by .cpp file compilation, and possibly by everything in 
between). And the worst that could happen if this function misbehaves, is that 
some of these duplicates will disappear. Which is annoying but harmless.

Let's add a comment explaining this!

https://github.com/llvm/llvm-project/pull/92031
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to