This is an archive of the discontinued LLVM Phabricator instance.

Better diagnostics for iterators missing ++, !=, and * in range-based for loops.
ClosedPublic

Authored by panzer on Aug 29 2012, 12:16 PM.

Details

Reviewers
rsmith
Summary

This patch adds an additional note when ++begin, !=begin, *__begin, or the loop variable declaration is invalid, stating the high-level source of the error (e.g. "cannot use type T as an iterator; no operator ++ is defined"). Ideally, this would be the error message rather than a note, but this should be suffcient to clarify the existing errors.

Diff Detail

Event Timeline

rsmith requested changes to this revision.Aug 30 2012, 7:03 AM
rsmith added inline comments.
include/clang/Basic/DiagnosticSemaKinds.td
1427–1429

Perhaps this would be clearer if presented as a context-style note:

in implicit call to 'operator!=' for iterator of type %0

That'd also remove the issue that 'no viable operator' isn't necessarily the problem (it could be an ambiguous operator, or an inaccessible destructor, etc).

test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
51

This note should presumably either say

loop variable 'a' has type 'char *'

or

loop variable 'a' is initialized by a value of type 'int'

Also, in these cases, it's not clear to me that this note is adding much value (since the primary diagnostic already mentions both relevant types). Are there other cases where the value is higher?

panzer updated this revision to Unknown Object (????).Sep 6 2012, 2:06 PM

Changed wording according to rsmith's suggestions.

panzer added a comment.Sep 6 2012, 2:25 PM

Changed the notes as per rsmith's suggestions.

include/clang/Basic/DiagnosticSemaKinds.td
1427–1429

Done. I feel it would be better still as the primary error message, if that becomes possible.

test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
51

I wasn't sure about this note either, and I can't remember the case where it seemed necessary.

rsmith accepted this revision.Sep 6 2012, 2:36 PM
dblaikie closed this revision.Oct 8 2012, 2:26 PM

Looks like this was committed in r163350