NoQ added inline comments.
================ Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:144 + // includes the full path. + if (SM.getFilename(IL).contains("UnifiedSource")) { + StringRef Name = SM.getFilename(SL); ---------------- george.karpenkov wrote: > Is this `if` really necessary? This logic has too much overfitting, and it > seems that if someone decides to include `.cc` files, we should analyze them > in any case, right? We also would prefer to not stop working if webkit > decides on using a different naming for those. This is indeed an act of overfitting. But also there are very few reasons to include a non-header file, and all of them are pretty exotic. I'm not sure we want to analyze these files in all cases. So i want to play safe until we gather more data. ================ Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:148 + Name.endswith_lower(".cc") || Name.endswith_lower(".m") || + Name.endswith_lower(".mm")) { + return true; ---------------- baloghadamsoftware wrote: > Although not very common, but .cxx is also a possibly extension for C++ > source files. Yup, thanks! ================ Comment at: test/Analysis/unified-sources/source1.cpp:8 + if (x) {} + return 1 / x; // expected-warning{{}} +} ---------------- george.karpenkov wrote: > Wow, expected-* directives work across multiple files?? This is really cool! `-verify` works over preprocessed files, so yeah, these directives respect `#if`s (which we regularly abuse) and `#include`s (which we rarely abuse). This test also acts as a normal test, but the `UnifiedSource-1.cpp` test is the one that'll fail if we break the newly added functionality. https://reviews.llvm.org/D45839 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits