[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-11 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Mordante marked 3 inline comments as done. Closed by commit rG9c74fb402e1b: [Sema] Improve -Wrange-loop-analysis warnings. (authored by Mordante). Changed prior to commit:

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-11 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 8 inline comments as done. Mordante added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2812-2813 - // TODO: Determine a maximum size that a POD type can be before a diagnostic - // should be emitted. Also, only ignore POD types with trivial copy

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-08 Thread Richard Trieu via Phabricator via cfe-commits
rtrieu added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2812-2813 - // TODO: Determine a maximum size that a POD type can be before a diagnostic - // should be emitted. Also, only ignore POD types with trivial copy - // constructors. - if

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/SemaCXX/warn-range-loop-analysis.cpp:22 struct Bar { + // The type is too large to suppress the warning for trivially copyable types. + char s[128]; `too large to suppress` means it does not suppress the

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Commit eec0240f97180ea876193dcfa3cb03cb652d9fe3 "Adds -Wrange-loop-analysis to -Wall" was premature before this fix, as

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp:6 + struct Record { +volatile int a; +int b; `test_POD` can be dropped. It does not add test coverage. "Struct with a volatile member no

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked an inline comment as done. Mordante added a comment. You're welcome. It would be nice if we can get this one in the release. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72212/new/ https://reviews.llvm.org/D72212

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the patch. Will take some time to review. Without this, adding -Wrange-loop-analysis to -Wall will make lots of projects fail to build if they have -Werror. For example, `for (const absl::string_review : ...)` or `const std::string_view` is common.

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-05 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 3 inline comments as done. Mordante added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2806 - // constructors. - if (VariableType.isPODType(SemaRef.Context)) return; xbolva00 wrote: > Mordante wrote: > > xbolva00 wrote: > > >

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2806 - // constructors. - if (VariableType.isPODType(SemaRef.Context)) return; Mordante wrote: > xbolva00 wrote: > > assert it? > Sorry I don't understand what you mean. What do you

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked an inline comment as done. Mordante added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2806 - // constructors. - if (VariableType.isPODType(SemaRef.Context)) return; xbolva00 wrote: > assert it? Sorry I don't understand what

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2806 - // constructors. - if (VariableType.isPODType(SemaRef.Context)) return; assert it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61248 tests passed, 0 failed and 736 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision. Mordante added reviewers: aaron.ballman, xbolva00, MaskRay. Mordante added a project: clang. No longer generate a diagnostic when a small trivially copyable type is used without a reference. Before the test looked for a POD type and had no size restriction. Since