[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

[Balázs Benics via Phabricator via cfe-commits]

steakhal planned changes to this revision.
steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:20-21
 #include "clang/Basic/Version.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Lex/Preprocessor.h"
----------------
NoQ wrote:
> Will these two includes be ultimately removed? Like, even in the CTU case?
I think `PlistDiagnostics` can depend on `CTU`.
`ASTUnit` is a different thing though. I think I just accidentally left that include.

I will remove the `#include "clang/Frontend/ASTUnit.h"` line.

---
What do you mean by the 'CTU case'?


================
Comment at: clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:148
      <key>name</key><string>SET_PTR_VAR_TO_NULL</string>
-     <key>expansion</key><string>ptr = 0</string>
+     <key>expansion</key><string>ptr =0</string>
     </dict>
----------------
NoQ wrote:
> steakhal wrote:
> > xazax.hun wrote:
> > > martong wrote:
> > > > martong wrote:
> > > > > I wonder how much added value do we have with these huge and clumsy plist files... We already have the concise unittests, which are quite self explanatory. I'd just simply delete these plist files.
> > > > Perhaps in the test cpp file we should just execute a FileCheck for the expansions. We are totally not interested to check the whole contents of the plist, we are interested only to see if there were expansions.
> > > We do need some plist tests to ensure that the correct plist format is emitted. How much of those do we need might be up for debate.
> > It's certainly a pain to keep all the locations in sync with the code.
> I'm absolutely in favor of not testing the //entire// plist files whenever possible. Just keep some minimal context available so that to not accidentally match the wrong test. Yes we obviously need some tests for the entire plist files but we already have a lot of that.
Ok, I will remove these files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93224