Thanks for taking a look at this! I have two comments: 1. If we’re doing the check at the end of both branches of the if, we might as well just move it to after. 2. It doesn’t make sense to have a failure that only occurs in C mode in test/SemaCXX. Maybe just append it to the end of test/Sema/typo-correction.c.
> On Jul 27, 2016, at 5:36 PM, David Tarditi via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > r272587 (http://reviews.llvm.org/D20490 <http://reviews.llvm.org/D20490>) > fixed an issue where typo correction could cause a crash when compiling C > programs. The problem was that a typo expression could be inadvertently > processed twice. r272587 fixed this for BinOp expressions. Conditional > expressions can hit the same problem too. This change adds the two line fix > for them, as well as a small test case illustrating the crash. This is my > first time proposing a patch for clang. I don’t have commit privileges, so > if someone could review/commit this for me, I’d appreciate it. If > Phrabicator is the preferred way or there’s another preferred process for > submitting patches, let me know. Yes, Phabricator is the preferred method of getting a patch reviewed! Please submit any future patches there. > I see from the prior review that the consensus was that “typo correction is > in a messy state, we should fix this”. I agree. There are other > problematic places in the code where double-processing might or might not > occur for C code. An example is the processing of subscript expressions in > Parser::ParsePostfixExpressionSuffix. Without clear invariants, it is hard > to know what to do. > > Testing: clang fails on the test case without this change, passes with it. > The clang test suite results were otherwise the same before and after this > change. > > Thanks, > David > <c-typo-crash.patch>_______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
