This is an archive of the discontinued LLVM Phabricator instance.

Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template
Needs ReviewPublic

Authored by faisalv on Nov 19 2020, 1:42 PM.

Details

Reviewers
rsmith
Summary

https://bugs.llvm.org/show_bug.cgi?id=42049

Currently clang, in the following code, when tentatively parsing the template-argument-list recognizes the error, and skips till the semi-colon. But when annotating the broken decltype specifier token (in an effort to backtrack gracefully and continue parsing), it appears we manually sync up the cache position with the last token that we think is part of the (broken) decltype construct. Not doing so results in a book-keeping assertion failure.

void f() {
  g<decltype>();
}

This situation did not seem to arise prior to https://github.com/llvm/llvm-project/commit/b23c5e8c3df850177449268c5ca7dbf986157525 - which implements assumption of names as function templates even if no function with that name is found.

Diff Detail

Event Timeline

faisalv created this revision.Nov 19 2020, 1:42 PM
faisalv requested review of this revision.Nov 19 2020, 1:42 PM
rsmith added inline comments.Nov 20 2020, 2:40 PM
clang/lib/Parse/ParseDeclCXX.cpp
1055

It seems to me that either EndLoc was miscomputed and should already point to the last token that we skipped over, in which case we should fix ParseDecltypeSpecifier to return the correct source location, or EndLoc is correct and we should revert back to the corresponding token and only annotate that portion of the token stream as the decltype specifier. (Or maybe a third option: EndLoc is meaningless when the type specifier is erroneous, in which case this code is still wrong because it's using that value in the case where backtracking is not enabled.)

Rolling back the token stream to the token corresponding to EndLoc seems likely to give us the best error recovery here, assuming that EndLoc is currently set to the token that we think is the last one that's intended to be part of the decltype specifier.

faisalv edited the summary of this revision. (Show Details)Dec 6 2020, 9:40 PM
faisalv updated this revision to Diff 309809.Dec 6 2020, 9:48 PM
faisalv edited the summary of this revision. (Show Details)

Per Richard's suggestion, instead of including the cached tokens into the decltype annotation, i revert the cache to match the end of where we think the (broken) decltype annotated token should end.

Thanks!