[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316268: [Sema] Fixes for enum handling for tautological comparison diagnostics (authored by lebedevri). Changed prior to commit: https://reviews.llvm.org/D39122?vs=119759=119760#toc Repository: rL

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Sema/SemaChecking.cpp:8619 + if (OtherRange.Width == 0) +return Value == 0 ? LimitType::Both : llvm::Optional(); +

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119759. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Address review notes. Repository: rL LLVM https://reviews.llvm.org/D39122 Files: lib/Sema/SemaChecking.cpp test/Sema/outof-range-enum-constant-compare.c

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8619 + if (OtherRange.Width == 0) +return Value == 0 ? LimitType::Both : llvm::Optional(); + aaron.ballman wrote: > Instead of default constructing the Optional, you should use

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8186 + // For enum types, for C code, use underlying data type. + if (const EnumType *ET = dyn_cast(T)) +T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8186 + // For enum types, for C code, use underlying data type. + if (const EnumType *ET = dyn_cast(T)) +T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119757. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Address review notes. Repository: rL LLVM https://reviews.llvm.org/D39122 Files: lib/Sema/SemaChecking.cpp test/Sema/outof-range-enum-constant-compare.c

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8185 +if (!C.getLangOpts().CPlusPlus) { + // For enum types, for C code, use underlying data type. + if (const EnumType *ET = dyn_cast(T)) For enum types in C code, use the

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119705. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Addressed review notes. For C++, enum handling clearly needs more work, because not all tautological comparisons are actually diagnosed, so there is no

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-20 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8610 + if(!S.getLangOpts().CPlusPlus && OtherT->isEnumeralType()) { +OtherT = OtherT->getAs()->getDecl()->getIntegerType(); Please drop the braces. Comment at:

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-20 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8612 +OtherT = OtherT->getAs()->getDecl()->getIntegerType(); + } + Why are you changing this here, as opposed to changing IntRange::forValueOfCanonicalType? Repository: rL LLVM

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. As Mattias Eriksson has reported in PR35009, in C, for enums, the underlying type should be used when checking for the tautological comparison, unlike C++, where the enumerator values define the value range. So if not in