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

Reply via email to