steakhal added a comment.

Vince, please update the summary - it looks really weird. Along with that the 
content of it might not be much useful, as we have a test case to demonstrate 
the crash; you can probably remove those dumps etc.
Otherwise LGTM.



================
Comment at: clang/test/Analysis/cast-value-state-dump.cpp:26
   if (dyn_cast_or_null<Square>(S)) {
-    // expected-note@-1 {{Assuming 'S' is not a 'Square'}}
+    // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Square *'}}
     // expected-note@-2 {{Taking false branch}}
----------------
martong wrote:
> steakhal wrote:
> > IDK, I see the motivation, but we don't need the full name of these in most 
> > cases.
> > I find it more disturbing than helpful. I would instead stick to the 
> > shorter form.
> I see your point, but I this way, the checker provides more precise type 
> info, which is good in my opinion.
Okay. It's not that important. This is a domain-specific check only for llvm 
codebases, so I'm not too picky about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127105

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

Reply via email to