<div dir="ltr">Richard and I happened to discuss this problem two years ago, I think. What I half remember from the conversation is that the problem of delayed typo checking is somewhat equivalent to memory ownership management. Some methods produce Stmt nodes that may contain delayed typos, and others do not, and the caller just has to "know" if they are responsible for checking delayed typos or not. I guess it's different in that you can check the same Stmt tree twice for typos, but you can't free memory twice. The consequences of forgetting to check delayed typos are that the compiler may accept invalid code containing typos, only to crash later in CodeGen or analysis.<div><br></div><div>Richard explained that Clang used to do manual memory management for Stmt nodes. You can see evidence of this in clang/Sema/Ownership.h header. Experience showed that managing ownership was onerous, so a decision was made to make all nodes implicitly owned by the ASTContext. Given that clang removed one form of ownership management, it seems like it would be awkward to try to use the type system to track delayed typo correction.</div><div><br></div><div>Based on that background, I think we need a design that has safe failure modes. For example, if we could emit errors at end of TU instead of asserting, that would probably be good enough. Then placing the calls to CorrectDelayedTyposInExpr well is a matter of diagnostic quality, not correctness.</div><div><br></div><div>Anyway, we got this far in the discussion, but then had to run off and go do other things as usual. :) If someone has time to improve this, I agree, it would be great, this assertion pops up in the clang fuzzer reports again and again.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 15, 2019 at 11:19 AM Ilya Biryukov via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi cfe-dev,<div><br></div><div>In clangd, we often hit the following assertion in the Sema's destructor:<br></div><div>    assert(DelayedTypos.empty() && "Uncorrected typos!");<br></div><div><br></div><div>It's purpose seems to be to indicate that there are places that should call CorrectDelayedTypos* and forget to do so.</div><div><br></div><div>However, when the assertion fires it's hard to track down the exact place where the CorrectDelayedTypos* call should be inserted. Moreover, it never fires in normal clang executions because the driver passes -disable-free by default, which stops Sema's destructor from running.</div><div><br></div><div>That leaves clangd and libclang in an unfortunate situation of being the only clients that experience crashes (apart from lit that run clang -cc1 mode and don't pass -disable-free).</div><div><br></div><div>We would like to avoid assertion failures for those, which leads me to the following questions:</div><div>- Is there a way to quickly track down the place that miss the CorrectDelayedTypos* call?</div><div>- If no, would it be ok to turn this assertion into some kind of debug-only warning and </div><div>  document that some typos are never actually corrected due to limitations?</div><div>- Even broader, are there any ideas for an alternative design that would be more resilient to</div><div>  changes in the codebase? </div><div>  E.g. no delayed typo corrections or easy-to-audit places that should run CorrectDelayedTypos*, etc.</div><div><br></div><div>-- <br></div><div><div dir="ltr" class="gmail-m_-2849858533500887113gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>