george.karpenkov 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);
----------------
NoQ wrote:
> 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.
I would still say that just analyzing included c++ files is a lesser evil.


================
Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:145-147
+      if (Name.endswith_lower(".c") || Name.endswith_lower(".cpp") ||
+          Name.endswith_lower(".cc") || Name.endswith_lower(".cxx") ||
+          Name.endswith_lower(".m") || Name.endswith_lower(".mm")) {
----------------
NoQ wrote:
> majnemer wrote:
> > C++ source code is also found in files which end in .C, this code will 
> > match against strange file endings like .cXx and .mM
> > 
> > I think the above logic should be changed to match 
> > https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/FrontendOptions.cpp#L27
> Aha, yeah, thanks, that's the place i was looking for.
Why not just use the included function then? It's static.


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