This revision was automatically updated to reflect the committed changes.
Closed by commit rG5902bb9584d6: [clang-tidy] Implement cppcoreguidelines F.19
(authored by ccotter, committed by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D14692
PiotrZSL updated this revision to Diff 520043.
PiotrZSL added a comment.
Add delayed template parsing in tests (to fix windows tests)
Reorder release notes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
PiotrZSL added a comment.
`forwarding reference parameter '' is never forwarded inside the function body
[cppcoreguidelines-missing-std-forward]`
consider ignoring unnamed parameters, they unused, so nothing to forward.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:136
+- New :doc:`cppcoreguidelines-missing-std-forward
+ ` check.
Should be after `cppcoreguidelines-misleading-capture-default-by-value`.
Repository:
rG LLVM Github
ccotter updated this revision to Diff 520032.
ccotter added a comment.
- fix merge
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
ccotter added a comment.
Note, @PiotrZSL I don't have commit access, so if you're happy with the
updates, could you please land this for me?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
__
ccotter updated this revision to Diff 520027.
ccotter added a comment.
rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang
ccotter added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:70
+ Finder->addMatcher(
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
PiotrZSL wrote:
> maybe think l
ccotter updated this revision to Diff 520026.
ccotter marked 6 inline comments as done.
ccotter added a comment.
- feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang
PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Overall looks fine. My main concern are lambdas (and maybe functions/classes in
functions, but that should only hit performance).
Please close all comments before pushing this.
=
ccotter added a comment.
bump?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ccotter updated this revision to Diff 512455.
ccotter marked an inline comment as done.
ccotter added a comment.
Fix tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
ccotter marked 2 inline comments as done.
ccotter added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
PiotrZSL wrote:
> ccotter wrote:
>
PiotrZSL added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
ccotter wrote:
> ccotter wrote:
> > PiotrZSL wrote:
> > > what about when s
ccotter updated this revision to Diff 512318.
ccotter added a comment.
- format
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
cla
ccotter added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
ccotter wrote:
> PiotrZSL wrote:
> > what about when someone uses std::move
ccotter updated this revision to Diff 511847.
ccotter added a comment.
- Rename
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
cla
ccotter updated this revision to Diff 509539.
ccotter added a comment.
- Use common function
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLi
ccotter updated this revision to Diff 508869.
ccotter marked 2 inline comments as done.
ccotter added a comment.
- fix docs, fix test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-e
ccotter added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+ if (isa(Node) || isa(Node))
PiotrZSL wrote:
> move this matcher to some ut
PiotrZSL added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+ if (isa(Node) || isa(Node))
move this matcher to some utils/...
It may be
ccotter updated this revision to Diff 508450.
ccotter added a comment.
- add tests, simplify expr, handle unevaluated exprs
- formatting
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tool
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:127
+
+ Warns when a forwarding reference parameter is not forwarded within the
function body.
+
Please follow 80 characters limit for text.
Repository:
rG LLVM Github
ccotter added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84
+
+ if (!Param)
+return;
+
PiotrZSL wrote:
> I thing this can never happen
Another review suggested I check the mat
ccotter updated this revision to Diff 508441.
ccotter marked 4 inline comments as done.
ccotter retitled this revision from "[clang-tidy] Implement cppcoreguidelines
F.19 " to "[clang-tidy] Implement cppcoreguidelines F.19".
ccotter added a comment.
- add tests, simplify expr, handle unevaluated
PiotrZSL added a comment.
maybe some other name for check, like missing-std-forward.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:62
+ namedDecl(hasUnderlyingDecl(hasName("::std::forward")),
+
26 matches
Mail list logo
