aaron.ballman added inline comments.

================
Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307
   .Case(FULLNAME, HELPTEXT)
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER
----------------
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > This thing still worries me but this definitely isn't a link-time 
> > > > dependency.
> > > D53277#1285757, rGb8cfcc71469d40a98f4cc79fcdc46cd67bea45f7
> > Ok, what's the proper solution here? This is clearly a layering violation 
> > now; this generic diagnostic consumer shouldn't know anything about the 
> > Static Analyzer specifically. I guess we could separate it into an 
> > independent polymorphic object ("DescriptionGetter" or something like that) 
> > that the Static Analyzer would instantiate manually. Or ideally we could 
> > ship this information with the bug report itself.
> > 
> > I'll add a FIXME and try to reproduce potential modules problems locally.
> I am puzzled myself :/ Maybe we could ask @aaron.ballman, since he landed 
> most of these changes back in the day?
The original code was added because it's needed for the required SARIF output. 
At the time, it didn't seem like it was a layering violation because the SARIF 
output was limited to just the clang static analyzer diagnostics -- however, I 
agree that work needs to be done here now. For instance, one of the issues with 
the SARIF output is that it only captures output from the static analyzer and 
doesn't report diagnostics from Clang itself, IIRC. We'll need to solve this 
for clang-tidy diagnostics as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67422

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to