[PATCH] D146342: [WIP][-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit

Mon Mar 20 18:27:34 PDT 2023

NoQ added a subscriber: usama54321.
NoQ added a comment.

Thank you for a detailed summary, it's very helpful!

> Can we move everything in AnalysisBasedWarnings.cpp to Sema? So far AnalysisBasedWarnings is used to bridge Sema and UnsafeBufferAnalysis so that the changes are minimal.

Currently Sema is completely oblivious to what specific analysis-based-warnings get issued per-function, and I see no need to do it differently for per-translation-unit warnings. (See also the inline comment.)

> Current tests are mostly fine except that some notes with message "in instantiation of ... " are missing. Although these notes are not emitted by our analysis, we better understand why things change.

This sounds familiar, I think @usama54321 ran into a similar problem from inside clang-tidy: https://discourse.llvm.org/t/clang-tidy-diagnostics-for-template-code/62909

================
Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:96-100
   void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
                      const Decl *D, QualType BlockType);

+  void CheckUnsafeBufferUsage(const TranslationUnitDecl *D);
+
----------------
Let's make the name generic. It doesn't really matter to the caller which specific warnings are emitted here (just like in the original method). We're likely to add more warnings of this kind in the future. What we're really doing in this patch, is create a new home for them to live in, so let's call it what it is!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146342