date:20210228

[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

2021-02-28 Thread Conrad Poelman via Phabricator via cfe-commits

poelmanc created this revision.
poelmanc added a reviewer: clang-tools-extra.
poelmanc added a project: clang-tools-extra.
Herald added a subscriber: jdoerfert.
poelmanc requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Running `check-clang-tools` on Windows produces 5 test failures:

  Failed Tests (5):
Clang Tools :: clang-apply-replacements/ClangRenameClassReplacements.cpp
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-move/move-used-helper-decls.cpp
Clang Tools :: clang-tidy/infrastructure/export-diagnostics.cpp

Four of these failures are simply due to fixed character position offsets 
differing on Windows versus Linux, since Windows line endings take up two 
characters instead of one:

- clang-apply-replacements/ClangRenameClassReplacements.cpp runs `clang-rename 
-offset=254`
- clang-apply-replacements/Inputs/basic/file[12].yaml specify e.g. `FileOffset: 
148` and `Offset: 298`
- clang-apply-replacements/Inputs/format/{no,yes}.yaml specify e.g. 
`FileOffset: 94` and `Offset: 94`
- clang-tidy/infrastructure/export-diagnostics.cpp specifies e.g. 
`CHECK-YAML-NEXT: FileOffset: 30`

(The move-used-helper-decls.cpp failure seems more complex; clang-move adds a 
blank line after `void HelperFun1() {}` when 
clang-move/Inputs/helper_decls_test.cpp has LF line endings, but does //not// 
add a blank line when the input files has CRLF line endings. That difference in 
behavior seems like it may be an actual bug, but I have yet to track it down.)

Do other folks developing on Windows see this? Performing a git checkout with 
Linux (LF) or "as-is" line endings would make the tests pass, but the 
underlying clang tools need to work on on input files with various line ending 
types, so it seems best to test the code against various line ending types. For 
example, last year I found a clang-tidy fix that //chomped// a CRLF line ending 
in half by implicitly assuming a line ending to be one byte, and the above 
clang-move issue could be similar. Ideally the Windows BuildKite job should 
test input files with CRLF line endings; since the above tests pass, I'd guess 
it's checking out test files only with LF line endings?

- I considered changing export-diagnostics.cpp to e.g. `CHECK-YAML-NEXT: 
FileOffset: 3[01]` to accept either, but that would relax the test on Linux - 
we really want exactly 30 for LF line endings, and exactly 31 for CRLF line 
endings.
- An ideal fix might somehow enable different offset numbers depending on the 
input file line endings, but that seems like a big change.
- Instead this patch adds a .gitattributes file to specify Linux LF line 
endings only for the above files that use `-offset`, `FileOffset` and `Offset` 
in their tests, CRLF line endings for the one crlf.cpp test, and auto for 
everything else.

I'm open to any other thoughts, ideas, or hints about testing clang tools on 
inputs with various line ending types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97625

Files:
  clang-tools-extra/test/.gitattributes


Index: clang-tools-extra/test/.gitattributes
===
--- /dev/null
+++ clang-tools-extra/test/.gitattributes
@@ -0,0 +1,19 @@
+# CRLF (Windows) line endings take two bytes instead of one, so any tests that
+# rely on or check fixed character -offset, Offset: or FileOffset: locations
+# will fail when run on input files checked out with different line endings.
+
+# Most test input files should use native line endings, to ensure that we run
+# tests against both line ending types.
+* text=auto
+
+# These test input files rely on one-byte Unix (LF) line-endings, as they use
+# fixed -offset, FileOffset:, or Offset: numbers in their tests.
+clang-apply-replacements/ClangRenameClassReplacements.cpp text eol=lf
+clang-apply-replacements/Inputs/basic/basic.h text eol=lf
+clang-apply-replacements/Inputs/format/no.cpp text eol=lf
+clang-apply-replacements/Inputs/format/yes.cpp text eol=lf
+clang-tidy/infrastructure/export-diagnostics.cpp text eol=lf
+
+# These test input files rely on two-byte Windows (CRLF) line endings.
+clang-apply-replacements/Inputs/crlf/crlf.cpp text eol=crlf
+clang-apply-replacements/Inputs/crlf/crlf.cpp.expected text eol=crlf


Index: clang-tools-extra/test/.gitattributes
===
--- /dev/null
+++ clang-tools-extra/test/.gitattributes
@@ -0,0 +1,19 @@
+# CRLF (Windows) line endings take two bytes instead of one, so any tests that
+# rely on or check fixed character -offset, Offset: or FileOffset: locations
+# will fail when run on input files checked out with different line endings.
+
+# Most test input files should use native line endings, to ensure that we run
+# tests against both line ending types.
+* text=auto
+
+# These test input files rely on one-byte Unix (LF)

[PATCH] D96896: [clang-format] Respect spaces in line comment section without an active column limit

2021-02-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96896/new/

https://reviews.llvm.org/D96896

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

2021-02-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius accepted this revision.
curdeius added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87587/new/

https://reviews.llvm.org/D87587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2021-02-28 Thread Conrad Poelman via Phabricator via cfe-commits

poelmanc updated this revision to Diff 326957.
poelmanc added a comment.

Removed a function in modernize/LoopConvertCheck.cpp that's no longer needed 
due to this patch. Fixed clang-format and clang-tidy issues identified by 
HarborMaster in prior submission.

Hopefully it's ready to go, but as always I'm happy to receive any feedback.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68682/new/

https://reviews.llvm.org/D68682

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Format/Format.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/CleanupTest.cpp

Index: clang/unittests/Format/CleanupTest.cpp
===
--- clang/unittests/Format/CleanupTest.cpp
+++ clang/unittests/Format/CleanupTest.cpp
@@ -11,6 +11,7 @@
 #include "../Tooling/ReplacementTest.h"
 #include "clang/Tooling/Core/Replacement.h"
 
+#include "llvm/Testing/Support/Annotations.h"
 #include "gtest/gtest.h"
 
 using clang::tooling::ReplacementTest;
@@ -320,6 +321,11 @@
 return tooling::Replacement(FileName, Offset, Length, Text);
   }
 
+  tooling::Replacement createReplacement(llvm::Annotations::Range Range,
+ StringRef Text) {
+return createReplacement(Range.Begin, Range.End - Range.Begin, Text);
+  }
+
   tooling::Replacement createInsertion(StringRef IncludeDirective) {
 return createReplacement(UINT_MAX, 0, IncludeDirective);
   }
@@ -373,10 +379,12 @@
  "namespace C {\n"
  "namespace D { int i; }\n"
  "inline namespace E { namespace { int y; } }\n"
+ "\n"
  "int x= 0;"
  "}";
-  std::string Expected = "\n\nnamespace C {\n"
- "namespace D { int i; }\n\n"
+  std::string Expected = "\nnamespace C {\n"
+ "namespace D { int i; }\n"
+ "\n"
  "int x= 0;"
  "}";
   tooling::Replacements Replaces =
@@ -386,6 +394,104 @@
   EXPECT_EQ(Expected, formatAndApply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, RemoveLineWhenAllNonWhitespaceRemoved) {
+  llvm::Annotations Code("namespace A {$r1[[ // Useless comment]]\n"
+ "  $r2[[int]] $r3[[x]]\t $r4[[=]] $r5[[0;]]\t\n"
+ "  int y\t = 0;$r6[[\t]]\n"
+ "} // namespace A\n");
+  std::string Expected = "namespace A {\n"
+ "  int y\t = 0;\n"
+ "} // namespace A\n";
+  tooling::Replacements Replaces =
+  toReplacements({createReplacement(Code.range("r1"), ""),
+  createReplacement(Code.range("r2"), ""),
+  createReplacement(Code.range("r3"), ""),
+  createReplacement(Code.range("r4"), ""),
+  createReplacement(Code.range("r5"), ""),
+  createReplacement(Code.range("r6"), "")});
+
+  EXPECT_EQ(Expected, apply(Code.code(), Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, RemoveLinesWhenAllNonWhitespaceRemoved) {
+  llvm::Annotations Code(R"cpp(struct A {
+  A()
+  $r3[[:]] $r1[[f()]]$r2[[,]]
+$r4[[g()]]
+  {}
+  int f = 0;
+  int g = 0;
+};)cpp");
+  std::string Expected = R"cpp(struct A {
+  A()
+  {}
+  int f = 0;
+  int g = 0;
+};)cpp";
+  tooling::Replacements Replaces =
+  toReplacements({createReplacement(Code.range("r1"), ""),
+  createReplacement(Code.range("r2"), ""),
+  createReplacement(Code.range("r3"), ""),
+  createReplacement(Code.range("r4"), "")});
+
+  EXPECT_EQ(Expected, apply(Code.code(), Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, KeepLinesWithInsertsOrReplacesEvenIfBlank) {
+  // Not using raw string literals so newlines and spaces are clear and explicit
+  llvm::Annotations Code("struct A {\n"
+ "  A() {}\n"
+ "$r3[[]]  $r1[[int]] $r2[[f;]]\n" // "  int f;\n"
+ "  \n"
+ "$r4[[  ]]\n"
+ "};");
+  std::string Expected = "struct A {\n"
+ "  A() {}

[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

2021-02-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D87587#2592605 , @kuzkry wrote:

>> Do you need someone to push it?
>
> Yes, I need someone to do that. I don't have write permissions. I'm new here.

Please state the name and mail for the commit. I will push it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87587/new/

https://reviews.llvm.org/D87587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Another point, is there any use case of this outside of clang-query. If not 
would it not be wise to move this infrastructure to 
clang-tools-extra/clang-query?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds an option, `PreferResetCall`, currently defaulted to `false`, to the check.
When `true` the check will refactor by calling the `reset` member function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97630

Files:
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
@@ -1,5 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t
-
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=NULLPTR
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=RESET -config='{ \
+// RUN: CheckOptions: [{key: readability-uniqueptr-delete-release.PreferResetCall, value: true}]}'
 namespace std {
 template 
 struct default_delete {};
@@ -13,6 +14,7 @@
   template 
   unique_ptr(unique_ptr&&);
   T* release();
+  void reset(decltype(nullptr) P = nullptr);
 };
 }  // namespace std
 
@@ -21,22 +23,30 @@
 void Positives() {
   std::unique_ptr P;
   delete P.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P.reset();
 
   auto P2 = P;
   delete P2.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P2 = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P2 = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P2.reset();
 
   std::unique_ptr Array[20];
   delete Array[4].release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  Array[4] = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  Array[4] = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  Array[4].reset();
 
   delete ReturnsAUnique().release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  ReturnsAUnique().reset();
 }
 
 struct NotDefaultDeleter {};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
@@ -15,3 +15,21 @@
 
   std::unique_ptr P;
   P = nullptr;
+  
+Options
+---
+
+.. option:: PreferResetCall
+
+  If ``true``, refactor by calling the reset member function instead of
+  assigning to ``nullptr``. Default value is ``false``.
+
+  .. code-block:: c++
+
+   std::unique_ptr P;
+   delete P.release();
+
+   // becomes
+
+   std::unique_ptr P;
+   P.reset();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
 
   Added an option to choose the set of allowed functions.
 
+- Improved :doc:`readability-uniqueptr-delete-release
+  ` check.
+
+  Added an option to choose whether to refactor by calling the ``reset`` member
+  function or assignment to ``nullptr``.
+
 Improvements

[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

2021-02-28 Thread Felix Berger via Phabricator via cfe-commits

flx added a comment.

In D97577#2592093 , @lebedev.ri wrote:

> It is best not to change existing tests, but add new ones.

Could elaborate on this, Roman?

In this case the tests use special auto convertible types with the intention to 
test that the check doesn't trigger because the loop variable needs to be 
constructed. But the reason they don't trigger is that the loop variable 
erroneously is already a reference type.

If we keep the tests as is we should rename the test functions and use 
different types or document that the real reason they don't trigger is that 
they already are const reference types, but we already have a test for that as 
well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97577/new/

https://reviews.llvm.org/D97577

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 326972.
njames93 added a comment.

Clean up some fix-it logic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97630/new/

https://reviews.llvm.org/D97630

Files:
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
@@ -1,5 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t
-
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=NULLPTR
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=RESET -config='{ \
+// RUN: CheckOptions: [{key: readability-uniqueptr-delete-release.PreferResetCall, value: true}]}'
 namespace std {
 template 
 struct default_delete {};
@@ -13,6 +14,7 @@
   template 
   unique_ptr(unique_ptr&&);
   T* release();
+  void reset(decltype(nullptr) P = nullptr);
 };
 }  // namespace std
 
@@ -21,22 +23,30 @@
 void Positives() {
   std::unique_ptr P;
   delete P.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P.reset();
 
   auto P2 = P;
   delete P2.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P2 = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P2 = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P2.reset();
 
   std::unique_ptr Array[20];
   delete Array[4].release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  Array[4] = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  Array[4] = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  Array[4].reset();
 
   delete ReturnsAUnique().release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  ReturnsAUnique().reset();
 }
 
 struct NotDefaultDeleter {};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
@@ -15,3 +15,21 @@
 
   std::unique_ptr P;
   P = nullptr;
+  
+Options
+---
+
+.. option:: PreferResetCall
+
+  If ``true``, refactor by calling the reset member function instead of
+  assigning to ``nullptr``. Default value is ``false``.
+
+  .. code-block:: c++
+
+   std::unique_ptr P;
+   delete P.release();
+
+   // becomes
+
+   std::unique_ptr P;
+   P.reset();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
 
   Added an option to choose the set of allowed functions.
 
+- Improved :doc:`readability-uniqueptr-delete-release
+  ` check.
+
+  Added an option to choose whether to refactor by calling the ``reset`` member
+  function or assignment to ``nullptr``.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
===
--- clang-tools-extra/clang-tidy/readabil

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-02-28 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst:24
+
+  If ``true``, refactor by calling the reset member function instead of
+  assigning to ``nullptr``. Default value is ``false``.

Please use single back-ticks for option values. Same below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97630/new/

https://reviews.llvm.org/D97630

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97632: [clang-tidy] Simplify diagnostics for UniqueptrResetRelease check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Tweak the diagnostics to create small replacements rather than grabbing source 
text from the lexer.
Also simplified the diagnostic message.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97632

Files:
  clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp
@@ -33,27 +33,27 @@
   std::unique_ptr *y = &b;
 
   a.reset(b.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) over ptr.reset(ptr2.release()) [misc-uniqueptr-reset-release]
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment over release and reset [misc-uniqueptr-reset-release]
   // CHECK-FIXES: a = std::move(b);
   a.reset(c.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: a = std::move(c);
   a.reset(Create().release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release()) [misc-uniqueptr-reset-release]
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: a = Create();
   x->reset(y->release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: *x = std::move(*y);
   Look().reset(Look().release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: Look() = std::move(Look());
   Get()->reset(Get()->release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: *Get() = std::move(*Get());
 
   std::unique_ptr func_a, func_b;
   func_a.reset(func_b.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: func_a = std::move(func_b);
 }
 
Index: clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
@@ -19,18 +19,21 @@
 void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   cxxMemberCallExpr(
-  on(expr().bind("left")), callee(memberExpr().bind("reset_member")),
-  callee(
-  cxxMethodDecl(hasName("reset"),
-ofClass(cxxRecordDecl(hasName("::std::unique_ptr"),
-  decl().bind("left_class"),
-  has(ignoringParenImpCasts(cxxMemberCallExpr(
-  on(expr().bind("right")),
-  callee(memberExpr().bind("release_member")),
-  callee(cxxMethodDecl(
-  hasName("release"),
-  ofClass(cxxRecordDecl(hasName("::std::unique_ptr"),
-decl().bind("right_class")
+  callee(memberExpr(
+ member(cxxMethodDecl(
+ hasName("reset"),
+ ofClass(cxxRecordDecl(hasName("::std::unique_ptr"),
+   decl().bind("left_class"))
+ .bind("reset_member")),
+  hasArgument(
+  0, ignoringParenImpCasts(cxxMemberCallExpr(
+ on(expr().bind("right")),
+ callee(memberExpr(member(cxxMethodDecl(
+   hasName("release"),
+   ofClass(cxxRecordDecl(
+   hasName("::std::unique_ptr"),
+   decl().bind("right_class"))
+.bind("release_member"))
   .bind("reset_call"),
   this);
 }
@@ -95,37 +98,31 @@
   const auto *ReleaseMember =
   Result.Nodes.getNodeAs("release_member");
   const auto *Right = Result.Nodes.getNodeAs("right");
-  const auto *L

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

@Eugene.Zelenko I think there should be a herald rule for marking projects as 
clang-tools-extra automatically


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97630/new/

https://reviews.llvm.org/D97630

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 326977.
njames93 marked an inline comment as done.
njames93 added a comment.

Use single back ticks for option values.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97630/new/

https://reviews.llvm.org/D97630

Files:
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
  clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
@@ -1,5 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t
-
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=NULLPTR
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=RESET -config='{ \
+// RUN: CheckOptions: [{key: readability-uniqueptr-delete-release.PreferResetCall, value: true}]}'
 namespace std {
 template 
 struct default_delete {};
@@ -13,6 +14,7 @@
   template 
   unique_ptr(unique_ptr&&);
   T* release();
+  void reset(decltype(nullptr) P = nullptr);
 };
 }  // namespace std
 
@@ -21,22 +23,30 @@
 void Positives() {
   std::unique_ptr P;
   delete P.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P.reset();
 
   auto P2 = P;
   delete P2.release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
-  // CHECK-FIXES: {{^}}  P2 = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+  // CHECK-FIXES-NULLPTR: {{^}}  P2 = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  P2.reset();
 
   std::unique_ptr Array[20];
   delete Array[4].release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  Array[4] = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  Array[4] = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  Array[4].reset();
 
   delete ReturnsAUnique().release();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
-  // CHECK-FIXES: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+  // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+  // CHECK-FIXES-NULLPTR: {{^}}  ReturnsAUnique() = nullptr;
+  // CHECK-FIXES-RESET: {{^}}  ReturnsAUnique().reset();
 }
 
 struct NotDefaultDeleter {};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
@@ -15,3 +15,21 @@
 
   std::unique_ptr P;
   P = nullptr;
+  
+Options
+---
+
+.. option:: PreferResetCall
+
+  If `true`, refactor by calling the reset member function instead of
+  assigning to ``nullptr``. Default value is `false`.
+
+  .. code-block:: c++
+
+   std::unique_ptr P;
+   delete P.release();
+
+   // becomes
+
+   std::unique_ptr P;
+   P.reset();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
 
   Added an option to choose the set of allowed functions.
 
+- Improved :doc:`readability-uniqueptr-delete-release
+  ` check.
+
+  Added an option to choose whether to refactor by calling the ``reset`` member
+  function or assignment to ``nullptr``.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire updated this revision to Diff 326985.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/fallback/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,95 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 4u);
+
+  EXPECT_EQ(result.LocationAccessors.begin()->first, FooCall->getBeginLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.LocationAccessors.begin()->second.get()),
+"getBeginLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 1)->first,
+FooCall->getExprLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 1)->second.get()),
+"getExprLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 2)->first,
+FooCall->getRParenLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 2)->second.get()),
+"getRParenLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 3)->first,
+FooCall->getEndLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 3)->second.get()),
+"getEndLoc()");
+
+  EXPECT_EQ(result.RangeAccessors.size(), 1u);
+  EXPECT_EQ(result.RangeAccessors.begin()->first, FooCall->getSourceRange());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.RangeAccessors.begin()->second.get()),
+"getSourceRange()");
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clang

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire updated this revision to Diff 326986.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/fallback/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,95 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 4u);
+
+  EXPECT_EQ(result.LocationAccessors.begin()->first, FooCall->getBeginLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.LocationAccessors.begin()->second.get()),
+"getBeginLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 1)->first,
+FooCall->getExprLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 1)->second.get()),
+"getExprLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 2)->first,
+FooCall->getRParenLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 2)->second.get()),
+"getRParenLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 3)->first,
+FooCall->getEndLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 3)->second.get()),
+"getEndLoc()");
+
+  EXPECT_EQ(result.RangeAccessors.size(), 1u);
+  EXPECT_EQ(result.RangeAccessors.begin()->first, FooCall->getSourceRange());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.RangeAccessors.begin()->second.get()),
+"getSourceRange()");
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clang

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire updated this revision to Diff 326987.
steveire marked 7 inline comments as done.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/fallback/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,95 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 4u);
+
+  EXPECT_EQ(result.LocationAccessors.begin()->first, FooCall->getBeginLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.LocationAccessors.begin()->second.get()),
+"getBeginLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 1)->first,
+FooCall->getExprLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 1)->second.get()),
+"getExprLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 2)->first,
+FooCall->getRParenLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 2)->second.get()),
+"getRParenLoc()");
+  EXPECT_EQ(std::next(result.LocationAccessors.begin(), 3)->first,
+FooCall->getEndLoc());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+std::next(result.LocationAccessors.begin(), 3)->second.get()),
+"getEndLoc()");
+
+  EXPECT_EQ(result.RangeAccessors.size(), 1u);
+  EXPECT_EQ(result.RangeAccessors.begin()->first, FooCall->getSourceRange());
+  EXPECT_EQ(LocationCallFormatterCpp::format(
+result.RangeAccessors.begin()->second.get()),
+"getSourceRange()");
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire marked 3 inline comments as done.
steveire added inline comments.



Comment at: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp:22-25
+auto publicAccessor = [](auto... InnerMatcher) {
+  return cxxMethodDecl(isPublic(), parameterCountIs(0), isConst(),
+   InnerMatcher...);
+};

njames93 wrote:
> Why is this a variable, a templated function should do the same thing.
> I imagine its something like this.
That seems far more noisy. I've left it as a lambda and moved it to the point 
of use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire marked 2 inline comments as done.
steveire added inline comments.



Comment at: clang/include/clang/Tooling/NodeIntrospection.h:54
+public:
+  static std::string format(LocationCall *Call) {
+std::vector vec;

njames93 wrote:
> Should this (and potential a few others) be moved to an implementation file.
The implementation file is generated by the python script. Rather than hiding 
this in the python script, I think it's better to leave it here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97639: [clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: alexfh, aaron.ballman, gribozavr2.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Make use of the `equalsBoundNode` matcher to ensure Init, Conditon and 
Increment variables all refer to the same variable during matching.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97639

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -61,21 +61,16 @@
 static const char LoopNameReverseIterator[] = "forLoopReverseIterator";
 static const char LoopNamePseudoArray[] = "forLoopPseudoArray";
 static const char ConditionBoundName[] = "conditionBound";
-static const char ConditionVarName[] = "conditionVar";
-static const char IncrementVarName[] = "incrementVar";
 static const char InitVarName[] = "initVar";
 static const char BeginCallName[] = "beginCall";
 static const char EndCallName[] = "endCall";
-static const char ConditionEndVarName[] = "conditionEndVar";
 static const char EndVarName[] = "endVar";
 static const char DerefByValueResultName[] = "derefByValueResult";
 static const char DerefByRefResultName[] = "derefByRefResult";
-// shared matchers
-static const TypeMatcher anyType() { return anything(); }
 
 static const StatementMatcher integerComparisonMatcher() {
   return expr(ignoringParenImpCasts(
-  declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName);
+  declRefExpr(to(varDecl(equalsBoundNode(InitVarName));
 }
 
 static const DeclarationMatcher initToZeroMatcher() {
@@ -85,25 +80,19 @@
 }
 
 static const StatementMatcher incrementVarMatcher() {
-  return declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName)));
+  return declRefExpr(to(varDecl(equalsBoundNode(InitVarName;
 }
 
 /// The matcher for loops over arrays.
-///
-/// In this general example, assuming 'j' and 'k' are of integral type:
 /// \code
-///   for (int i = 0; j < 3 + 2; ++k) { ... }
+///   for (int i = 0; i < 3 + 2; ++i) { ... }
 /// \endcode
 /// The following string identifiers are bound to these parts of the AST:
-///   ConditionVarName: 'j' (as a VarDecl)
 ///   ConditionBoundName: '3 + 2' (as an Expr)
 ///   InitVarName: 'i' (as a VarDecl)
-///   IncrementVarName: 'k' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///
 /// Client code will need to make sure that:
-///   - The three index variables identified by the matcher are the same
-/// VarDecl.
 ///   - The index variable is only used as an array index.
 ///   - All arrays indexed by the loop are the same.
 StatementMatcher makeArrayLoopMatcher() {
@@ -131,27 +120,22 @@
 /// catch loops of the following textual forms (regardless of whether the
 /// iterator type is actually a pointer type or a class type):
 ///
-/// Assuming f, g, and h are of type containerType::iterator,
 /// \code
 ///   for (containerType::iterator it = container.begin(),
-///e = createIterator(); f != g; ++h) { ... }
+///e = createIterator(); it != e; ++it) { ... }
 ///   for (containerType::iterator it = container.begin();
-///f != anotherContainer.end(); ++h) { ... }
+///it != anotherContainer.end(); ++it) { ... }
 /// \endcode
 /// The following string identifiers are bound to the parts of the AST:
 ///   InitVarName: 'it' (as a VarDecl)
-///   ConditionVarName: 'f' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///   In the first example only:
 /// EndVarName: 'e' (as a VarDecl)
-/// ConditionEndVarName: 'g' (as a VarDecl)
 ///   In the second example only:
 /// EndCallName: 'container.end()' (as a CXXMemberCallExpr)
 ///
 /// Client code will need to make sure that:
-///   - The iterator variables 'it', 'f', and 'h' are the same.
 ///   - The two containers on which 'begin' and 'end' are called are the same.
-///   - If the end iterator variable 'g' is defined, it is the same as 'f'.
 StatementMatcher makeIteratorLoopMatcher(bool IsReverse) {
 
   auto BeginNameMatcher = IsReverse ? hasAnyName("rbegin", "crbegin")
@@ -180,13 +164,13 @@
 
   StatementMatcher IteratorBoundMatcher =
   expr(anyOf(ignoringParenImpCasts(
- declRefExpr(to(varDecl().bind(ConditionEndVarName,
+ declRefExpr(to(varDecl(equalsBoundNode(EndVarName),
  ignoringParenImpCasts(expr(EndCallMatcher).bind(EndCallName)),
  materializeTemporaryExpr(ignoringParenImpCasts(
  expr(EndCallMatcher).bind(EndCallName);
 
-  StatementMatcher IteratorComparisonMatcher = expr(
-  ignoringParenImpCasts(declRefExpr(to(varDecl().bind(ConditionVarNam

[PATCH] D97351: [clangd] Use flags from open files when opening headers they include

2021-02-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

thanks, this looks great.




Comment at: clang-tools-extra/clangd/TUScheduler.cpp:187
+///
+/// This could also naturally live in the index, but there are advantages to
+/// using open files instead:

I'm a bit confused, I don't get the meaning of the "This could also naturally 
live in the index", what lives in the index? my best guess is the include 
structure captured in background index.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:202
+///   proxy file is invalidated *and* a new candidate proxy file is built.
+///   Switching proxies likely invalidates the preamble, so it's expensive.
+/// - We don't capture the actual compile command, but just the filename we

maybe it is just me, it took me a while to understand the meaning of 
"invalidates the preamble" -- switching proxies usually indicates the change of 
CMD of the header, thus the preamble of the header is invalidated, is that 
right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97351/new/

https://reviews.llvm.org/D97351

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93164: [AST] Add generator for source location introspection

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire updated this revision to Diff 326997.
steveire marked an inline comment as done.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93164/new/

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/fallback/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,96 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  std::map ExpectedLocations;
+  llvm::transform(result.LocationAccessors,
+  std::inserter(ExpectedLocations, ExpectedLocations.end()),
+  [](const auto &Accessor) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_EQ(ExpectedLocations,
+(std::map{
+{"getBeginLoc()", FooCall->getBeginLoc()},
+{"getEndLoc()", FooCall->getEndLoc()},
+{"getExprLoc()", FooCall->getExprLoc()},
+{"getRParenLoc()", FooCall->getRParenLoc()}}));
+
+  std::map ExpectedRanges;
+  llvm::transform(result.RangeAccessors,
+  std::inserter(ExpectedRanges, ExpectedRanges.end()),
+  [](const auto &Accessor) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_EQ(ExpectedRanges,
+(std::map{
+{"getSourceRange()", FooCall->getSourceRange()}}));
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clangBasic
+  clangSerialization
+  clangFrontend
+  )
+target_compile_definitions(IntrospectionTests PRIV

[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

2021-02-28 Thread Krystian Kuzniarek via Phabricator via cfe-commits

kuzkry added a comment.

In D87587#2592722 , 
@HazardyKnusperkeks wrote:

> In D87587#2592605 , @kuzkry wrote:
>
>>> Do you need someone to push it?
>>
>> Yes, I need someone to do that. I don't have write permissions. I'm new here.
>
> Please state the name and mail for the commit. I will push it for you.

Name: Krystian Kuzniarek
Email: krystian.kuznia...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87587/new/

https://reviews.llvm.org/D87587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97639: [clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire accepted this revision.
steveire added a comment.
This revision is now accepted and ready to land.

Looks like a good simplification!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97639/new/

https://reviews.llvm.org/D97639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 40cee38 - Add tests which include brace initialization

2021-02-28 Thread Stephen Kelly via cfe-commits


Author: Stephen Kelly
Date: 2021-03-01T00:34:58Z
New Revision: 40cee381c1779256e039d66ea5d01ad7d344efb7

URL: 
https://github.com/llvm/llvm-project/commit/40cee381c1779256e039d66ea5d01ad7d344efb7
DIFF: 
https://github.com/llvm/llvm-project/commit/40cee381c1779256e039d66ea5d01ad7d344efb7.diff

LOG: Add tests which include brace initialization

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
index d637806ba20a..3428d453c63d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -64,6 +64,13 @@ void test() {
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after 
creation; did you mean to name the object?
 // CHECK-FIXES: FooBar give_me_a_name;
 
+  Foo{42};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: Foo give_me_a_name{42};
+  FooBar{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: FooBar give_me_a_name;
+
   templ();
   templ();
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97142: [clang-tidy] Simplify unused RAII check

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire updated this revision to Diff 327004.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97142/new/

https://reviews.llvm.org/D97142

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -46,6 +46,42 @@
   T();
 }
 
+struct CtorDefaultArg {
+  CtorDefaultArg(int i = 0);
+  ~CtorDefaultArg();
+};
+
+template 
+struct TCtorDefaultArg {
+  TCtorDefaultArg(T i = 0);
+  ~TCtorDefaultArg();
+};
+
+struct CtorTwoDefaultArg {
+  CtorTwoDefaultArg(int i = 0, bool b = false);
+  ~CtorTwoDefaultArg();
+};
+
+template 
+void templatetest() {
+  TCtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+  TCtorDefaultArg{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
+  TCtorDefaultArg(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name(T{});
+  TCtorDefaultArg{T{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name{T{}};
+
+  int i = 0;
+  (void)i;
+}
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
@@ -71,6 +107,22 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
   // CHECK-FIXES: FooBar give_me_a_name;
 
+  CtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: CtorDefaultArg give_me_a_name;
+
+  CtorTwoDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: CtorTwoDefaultArg give_me_a_name;
+
+  TCtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
+  TCtorDefaultArg{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
   templ();
   templ();
 
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
@@ -28,6 +28,9 @@
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,25 +25,37 @@
 
 void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
   // Look for temporaries that are constructed in-place and immediately
-  // destroyed. Look for temporaries created by a functional cast but not for
-  // those returned from a call.
-  auto BindTemp = cxxBindTemporaryExpr(
-  unless(has(ignoringParenImpCasts(callExpr(,
-  unless(has(ignoringParenImpCasts(objcMessageExpr()
-  .bind("temp");
+  // destroyed.
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   exprWithCleanups(
-   unless(isInTemplateInstantiation()),
-   hasParent(compoundStmt().bind("compound")),
-   hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-   anyOf(has(ignoringParenImpCasts(BindTemp)),
- has(ignoringParenImpCasts(cxxFunctionalCastExpr(
- has(ignoringParenImpCasts(BindTemp)))
-   .bind("expr")),
+  mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
+  .with

[PATCH] D97121: [clang-tidy] Add a single fix mode to clang-tidy

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:516
+  /// applied at a time.
+  bool isSingleFixMode() const { return Context->isSingleFixMode(); }
 };

I find the naming of this as "single fix" confusing.

Something along the lines of "self contained diagnostics" or so I think would 
be better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97121/new/

https://reviews.llvm.org/D97121

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97142: [clang-tidy] Simplify unused RAII check

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

LG, with 1 nit.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp:81-82
+
+  int i = 0;
+  (void)i;
+}

Is this necessary?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97142/new/

https://reviews.llvm.org/D97142

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

2021-02-28 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

FYI, we're seeing test failures caused by this patch in iOS/arm64 builds when 
arc is used (simulator is fine though): 
https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11

We'll try to investigate a bit more on our side, but I wanted to give an 
early(ish) heads-up in case others see this or whatnot.

Not sure if this landed before or after the 12.0 branch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92808/new/

https://reviews.llvm.org/D92808

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97156: [ASTMatchers] Make Param functors variadic

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Is it not nicer to name the class `PolymorphicMatcher` and do away with 
`WithParamN`, It may require partial specialization for the empty parameters 
case.
But I think it would be worth it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97156/new/

https://reviews.llvm.org/D97156

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

2021-02-28 Thread Tim Wojtulewicz via Phabricator via cfe-commits

timwoj added a comment.

In D94500#2592448 , 
@HazardyKnusperkeks wrote:

> Do you need someone to push this?

Yes I do. I don't have committer access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: alexfh, aaron.ballman, JonasToth, rsmith, 
MyDeveloperDay.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: shafik.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently these checks will overwrite the default capture token (=|&) if any 
default captures refer to decls that need fixing.
There was an a previous patch (D59540 ) that 
tried to address this shortfall but it went about it trying to detect if it was 
inside a lambda then comparing the textual representation.
Learning from the previous its obvious that we need someway to tag a 
DeclRefExpr.
I've settled on `RefersToDefaultCapture` for now, but there could be a case to 
make it `IsImplicit`, for when a DeclRefExpr isn't actually written.
If anyone can think of good examples when this could happen I'd definitely 
support that instead.
Right now I've only addressed lambdas as I don't know anything about ObjectiveC 
Blocks, If this makes sense there I'd be happy to extend it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97653

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -599,6 +599,7 @@
   Record.push_back(E->hasTemplateKWAndArgsInfo());
   Record.push_back(E->hadMultipleCandidates());
   Record.push_back(E->refersToEnclosingVariableOrCapture());
+  Record.push_back(E->refersToDefaultCapture());
   Record.push_back(E->isNonOdrUse());
 
   if (E->hasTemplateKWAndArgsInfo()) {
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2282,6 +2282,8 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ExplicitTemplateArgs
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //HadMultipleCandidates
   Abv->Add(BitCodeAbbrevOp(0)); // RefersToEnclosingVariableOrCapture
+  Abv->Add(
+  BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // RefersToDefaultCapture
   Abv->Add(BitCodeAbbrevOp(0)); // NonOdrUseReason
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclRef
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Location
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -597,6 +597,7 @@
   E->DeclRefExprBits.HasTemplateKWAndArgsInfo = Record.readInt();
   E->DeclRefExprBits.HadMultipleCandidates = Record.readInt();
   E->DeclRefExprBits.RefersToEnclosingVariableOrCapture = Record.readInt();
+  E->DeclRefExprBits.RefersToDefaultCapture = Record.readInt();
   E->DeclRefExprBits.NonOdrUseReason = Record.readInt();
   unsigned NumTemplateArgs = 0;
   if (E->hasTemplateKWAndArgsInfo())
@@ -2787,12 +2788,14 @@
 
 case EXPR_DECL_REF:
   S = DeclRefExpr::CreateEmpty(
-Context,
-/*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
-/*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
-/*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
-/*NumTemplateArgs=*/Record[ASTStmtReader::NumExprFields + 2] ?
-  Record[ASTStmtReader::NumExprFields + 6] : 0);
+  Context,
+  /*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
+  /*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
+  /*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
+  /*NumTemplateArgs=*/
+  Record[ASTStmtReader::NumExprFields + 2]
+  ? Record[ASTStmtReader::NumExprFields + 7]
+  : 0);
   break;
 
 case EXPR_INTEGER_LITERAL:
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1615,6 +1615,8 @@
 return ExprError();
 
   Expr *InitExpr = Init.get();
+  if (Cap.isVariableCapture() && ImplicitCaptureLoc.isValid())
+cast(InitExpr)->setRefersToDefaultCapture(true);
   InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
   Name, Cap.getCaptureType(), Loc);
   InitializationKind InitKind =
Index: clang/lib/AST/Expr.cpp
=

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Where would be a good place for testing refersToDefaultCapture support?




Comment at: clang/lib/Sema/SemaLambda.cpp:1619
+  if (Cap.isVariableCapture() && ImplicitCaptureLoc.isValid())
+cast(InitExpr)->setRefersToDefaultCapture(true);
   InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(

Correct me if I'm wrong, but there shouldn't be a case where the assert in cast 
fails.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2285-2286
   Abv->Add(BitCodeAbbrevOp(0)); // RefersToEnclosingVariableOrCapture
+  Abv->Add(
+  BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // RefersToDefaultCapture
   Abv->Add(BitCodeAbbrevOp(0)); // NonOdrUseReason

I have no idea about BitcodeWriter, is this correct or should I just be using 
the literal abbreviation?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97653/new/

https://reviews.llvm.org/D97653

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96114: [ASTMatchers] Fix parent-child traversal between functions and parms

2021-02-28 Thread Stephen Kelly via Phabricator via cfe-commits

steveire abandoned this revision.
steveire added a comment.

Clearing this out of my outgoing list.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96114/new/

https://reviews.llvm.org/D96114

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-02-28 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 327019.
njames93 added a comment.

Add a test case using init capture.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97653/new/

https://reviews.llvm.org/D97653

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -599,6 +599,7 @@
   Record.push_back(E->hasTemplateKWAndArgsInfo());
   Record.push_back(E->hadMultipleCandidates());
   Record.push_back(E->refersToEnclosingVariableOrCapture());
+  Record.push_back(E->refersToDefaultCapture());
   Record.push_back(E->isNonOdrUse());
 
   if (E->hasTemplateKWAndArgsInfo()) {
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2282,6 +2282,8 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ExplicitTemplateArgs
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //HadMultipleCandidates
   Abv->Add(BitCodeAbbrevOp(0)); // RefersToEnclosingVariableOrCapture
+  Abv->Add(
+  BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // RefersToDefaultCapture
   Abv->Add(BitCodeAbbrevOp(0)); // NonOdrUseReason
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclRef
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Location
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -597,6 +597,7 @@
   E->DeclRefExprBits.HasTemplateKWAndArgsInfo = Record.readInt();
   E->DeclRefExprBits.HadMultipleCandidates = Record.readInt();
   E->DeclRefExprBits.RefersToEnclosingVariableOrCapture = Record.readInt();
+  E->DeclRefExprBits.RefersToDefaultCapture = Record.readInt();
   E->DeclRefExprBits.NonOdrUseReason = Record.readInt();
   unsigned NumTemplateArgs = 0;
   if (E->hasTemplateKWAndArgsInfo())
@@ -2787,12 +2788,14 @@
 
 case EXPR_DECL_REF:
   S = DeclRefExpr::CreateEmpty(
-Context,
-/*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
-/*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
-/*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
-/*NumTemplateArgs=*/Record[ASTStmtReader::NumExprFields + 2] ?
-  Record[ASTStmtReader::NumExprFields + 6] : 0);
+  Context,
+  /*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
+  /*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
+  /*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
+  /*NumTemplateArgs=*/
+  Record[ASTStmtReader::NumExprFields + 2]
+  ? Record[ASTStmtReader::NumExprFields + 7]
+  : 0);
   break;
 
 case EXPR_INTEGER_LITERAL:
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1615,6 +1615,8 @@
 return ExprError();
 
   Expr *InitExpr = Init.get();
+  if (Cap.isVariableCapture() && ImplicitCaptureLoc.isValid())
+cast(InitExpr)->setRefersToDefaultCapture(true);
   InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
   Name, Cap.getCaptureType(), Loc);
   InitializationKind InitKind =
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -389,6 +389,7 @@
   DeclRefExprBits.HadMultipleCandidates = false;
   DeclRefExprBits.RefersToEnclosingVariableOrCapture =
   RefersToEnclosingVariableOrCapture;
+  DeclRefExprBits.RefersToDefaultCapture = false;
   DeclRefExprBits.NonOdrUseReason = NOUR;
   DeclRefExprBits.Loc = L;
   setDependence(computeDependence(this, Ctx));
@@ -415,6 +416,7 @@
 = (TemplateArgs || TemplateKWLoc.isValid()) ? 1 : 0;
   DeclRefExprBits.RefersToEnclosingVariableOrCapture =
   RefersToEnclosingVariableOrCapture;
+  DeclRefExprBits.RefersToDefaultCapture = false;
   DeclRefExprBits.NonOdrUseReason = NOUR;
   if (TemplateArgs) {
 auto Deps = TemplateArgumentDependence::None;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImport

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

2021-02-28 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak added a comment.

In D92808#2593204 , @thakis wrote:

> FYI, we're seeing test failures caused by this patch in iOS/arm64 builds when 
> arc is used (simulator is fine though): 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11
>
> We'll try to investigate a bit more on our side, but I wanted to give an 
> early(ish) heads-up in case others see this or whatnot.
>
> Not sure if this landed before or after the 12.0 branch.

Thank you. Let me know when you have more information.

This isn't in release/12.x. The older version of the patch, which used 
attributes instead of operand bundles, was in release/12.x, but got reverted 
later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92808/new/

https://reviews.llvm.org/D92808

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93031: Enable fexec-charset option

2021-02-28 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added a comment.

Hi, Abhina.  Sorry for the delay getting back to you.  I added some more 
comments.




Comment at: clang/include/clang/Lex/LiteralSupport.h:191
+Preprocessor &PP, tok::TokenKind kind,
+ConversionState TranslationState = TranslateToExecCharset);
 

Is the default argument for `TranslationState` actually used anywhere?  I'm 
skeptical that a default argument provides a benefit here.

Actually, this diff doesn't include any changes to construct a 
`CharLiteralParser` with an explicit argument.  It seems this argument isn't 
actually needed.

The only places I see objects of `CharLiteralParser` type constructed are in 
`EvaluateValue()` in `clang/lib/Lex/PPExpressions.cpp` and 
`Sema::ActOnCharacterConstant()` in `clang/lib/Sema/SemaExpr.cpp`.



Comment at: clang/include/clang/Lex/LiteralSupport.h:238-239
+ResultPtr(ResultBuf.data()), hadError(false), Pascal(false) {
+LT = new LiteralTranslator();
+LT->setTranslationTables(Features, Target, *Diags);
+init(StringToks, TranslationState);

I don't think a `LiteralTranslator` object is actually needed in this case.  
The only use of this constructor that I see is in 
`ModuleMapParser::consumeToken()` in `clang/lib/Lex/ModuleMap.cpp` and, in that 
case, I don't think any translation is necessary.

This suggests that `TranslationState` is not needed for this constructor 
either; `NoTranslation` can be passed to `init()`.



Comment at: clang/include/clang/Lex/LiteralSupport.h:260-262
+  unsigned getOffsetOfStringByte(
+  const Token &TheTok, unsigned ByteNo,
+  ConversionState TranslationState = TranslateToExecCharset) const;

I don't think `getOffsetOfStringByte()` should require a `ConversionState` 
parameter.  If I understand it correctly, this function should be operating on 
the string in the internal encoding, never in a converted encoding.



Comment at: clang/include/clang/Lex/LiteralTranslator.h:19-23
+enum ConversionState {
+  NoTranslation,
+  TranslateToSystemCharset,
+  TranslateToExecCharset
+};

Some naming suggestions...  The enumeration is not used to record a state, but 
rather to indicate an action to take.  Also, use of both "conversion" and 
"translation" could be confusing, so I suggest sticking with one.  Perhaps:
  enum class LiteralConversion {
None,
ToSystemCharset,
ToExecCharset
  };



Comment at: clang/include/clang/Lex/LiteralTranslator.h:30-31
+
+class LiteralTranslator {
+public:
+  llvm::StringRef InternalCharset;

I don't know the LLVM style guides well, but I suspect a class with all public 
members should be defined using `struct` and not include access specifiers.



Comment at: clang/include/clang/Lex/LiteralTranslator.h:35
+  llvm::StringRef ExecCharset;
+  llvm::StringMap ExecCharsetTables;
+

Given the converter setters and accessors below, `ExecCharsetTables` should be 
a private member.



Comment at: clang/include/clang/Lex/LiteralTranslator.h:37
+
+  llvm::CharSetConverter *getConversionTable(const char *Codepage);
+  CharsetTableStatusCode findOrCreateExecCharsetTable(const char *To);

`getConversionTable()` is logically `const`.  Perhaps `ExecCharsetTables` 
should be `mutable`.

From a terminology stand point, this function is misnamed.  It doesn't return a 
table, it returns a converter for an encoding.  I suggest:
  llvm::CharSetConverter *getCharSetConverter(const char *Encoding) const;



Comment at: clang/include/clang/Lex/LiteralTranslator.h:38
+  llvm::CharSetConverter *getConversionTable(const char *Codepage);
+  CharsetTableStatusCode findOrCreateExecCharsetTable(const char *To);
+  llvm::CharSetConverter *

`findOrCreateExecCharsetTable()` seems oddly named since it doesn't return 
whatever it finds or creates.  It seems like this function would be more useful 
if it returned a `llvm::CharSetConverter` pointer with `nullptr` indicating 
lookup/creation failed.

This function seems like it should be an implementation detail of the class, 
not a public interface.



Comment at: clang/include/clang/Lex/LiteralTranslator.h:41-43
+  void setTranslationTables(const clang::LangOptions &Opts,
+const clang::TargetInfo &TInfo,
+clang::DiagnosticsEngine &Diags);

`setTranslationTables()` is awkward.  It is effectively operating as a 
constructor for the class, but isn't called at object construction and it does 
work that goes beyond initialization.



Comment at: clang/include/clang/Lex/LiteralTranslator.h:44
+clang::DiagnosticsEngine &Diags);
+};
+

I suggest trying a design more like this:
  class LiteralTransl

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

2021-02-28 Thread Bing Yu via Phabricator via cfe-commits

yubing added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:99
+  Loop *RowLoop = LI.AllocateLoop();
+  Loop *ColLoop = LI.AllocateLoop();
+  RowLoop->addChildLoop(ColLoop);

pengfei wrote:
> Not sure how about the arithmetic intrinsics. But at least for load and store 
> intrinsics we can use LLVM intrinsic `llvm.masked.load/store` to reduce the 
> inner loop.
I think We can compose a follow-up patch for this optimization


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93594/new/

https://reviews.llvm.org/D93594

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

2021-02-28 Thread Min-Yih Hsu via Phabricator via cfe-commits

myhsu updated this revision to Diff 327029.
myhsu marked 4 inline comments as done.
myhsu added a comment.

[NFC] Update FileCheck prefix


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88394/new/

https://reviews.llvm.org/D88394

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/M68k.cpp
  clang/lib/Driver/ToolChains/Arch/M68k.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/m68k-features.cpp
  clang/test/Driver/m68k-sub-archs.cpp

Index: clang/test/Driver/m68k-sub-archs.cpp
===
--- /dev/null
+++ clang/test/Driver/m68k-sub-archs.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68000 %s 2>&1 | FileCheck --check-prefix=CHECK-M00 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68000 %s 2>&1 | FileCheck --check-prefix=CHECK-M00 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68000 %s 2>&1 | FileCheck --check-prefix=CHECK-M00 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68000 %s 2>&1 | FileCheck --check-prefix=CHECK-M00 %s
+// CHECK-M00: "-target-cpu" "M68000"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68010 %s 2>&1 | FileCheck --check-prefix=CHECK-M10 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68010 %s 2>&1 | FileCheck --check-prefix=CHECK-M10 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68010 %s 2>&1 | FileCheck --check-prefix=CHECK-M10 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68010 %s 2>&1 | FileCheck --check-prefix=CHECK-M10 %s
+// CHECK-M10: "-target-cpu" "M68010"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68020 %s 2>&1 | FileCheck --check-prefix=CHECK-M20 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68020 %s 2>&1 | FileCheck --check-prefix=CHECK-M20 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68020 %s 2>&1 | FileCheck --check-prefix=CHECK-M20 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68020 %s 2>&1 | FileCheck --check-prefix=CHECK-M20 %s
+// CHECK-M20: "-target-cpu" "M68020"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68030 %s 2>&1 | FileCheck --check-prefix=CHECK-M30 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68030 %s 2>&1 | FileCheck --check-prefix=CHECK-M30 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68030 %s 2>&1 | FileCheck --check-prefix=CHECK-M30 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68030 %s 2>&1 | FileCheck --check-prefix=CHECK-M30 %s
+// CHECK-M30: "-target-cpu" "M68030"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68040 %s 2>&1 | FileCheck --check-prefix=CHECK-M40 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68040 %s 2>&1 | FileCheck --check-prefix=CHECK-M40 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68040 %s 2>&1 | FileCheck --check-prefix=CHECK-M40 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68040 %s 2>&1 | FileCheck --check-prefix=CHECK-M40 %s
+// CHECK-M40: "-target-cpu" "M68040"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68060 %s 2>&1 | FileCheck --check-prefix=CHECK-M60 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68060 %s 2>&1 | FileCheck --check-prefix=CHECK-M60 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68060 %s 2>&1 | FileCheck --check-prefix=CHECK-M60 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68060 %s 2>&1 | FileCheck --check-prefix=CHECK-M60 %s
+// CHECK-M60: "-target-cpu" "M68060"
Index: clang/test/Driver/m68k-features.cpp
===
--- /dev/null
+++ clang/test/Driver/m68k-features.cpp
@@ -0,0 +1,45 @@
+// Check macro definitions
+// RUN: %clang -target m68k-unknown-linux -m68000 -dM -E %s | FileCheck --check-prefix=CHECK-MX %s
+// CHECK-MX: #define __mc68000 1
+// CHECK-MX: #define __mc68000__ 1
+// CHECK-MX: #define mc68000 1
+
+// RUN: %clang -target m68k-unknown-linux -m68010 -dM -E %s | FileCheck --check-prefix=CHECK-MX10 %s
+// CHECK-MX10: #define __mc68000 1
+// CHECK-MX10: #define __mc68000__ 1
+// CHECK-MX10: #define __mc68010 1
+// CHECK-MX10: #define __mc68010__ 1
+// CHECK-MX10: #define mc68000 1
+// CHECK-MX10: #define mc68010 1
+
+// RUN: %clang -target m68k-unknown-linux -m68020 -dM -E %s | FileCheck --check-prefix=CHECK-MX20 %s
+// CHECK-MX20: #define __mc68000 1
+// CHECK-MX20: #define __mc68000__ 1
+// CHECK-MX20: #define __mc68020 1
+// CHECK-MX20: #define __mc68020__ 1
+// CHECK-MX20: #define mc68000 1
+// CHECK-MX20: #define mc68020 1
+
+// RUN: %clang -target m68k-unknown-linux -m68030 -dM -E %s | FileCheck --check-prefix=CHECK-MX30 %s
+// CHECK-MX30: #define __mc68000 1
+// CHECK-MX30: #define __mc68000__ 1
+// CHECK-MX30: #define __mc68030 1
+// CHECK-MX30: #define __mc68030__ 1
+// CHECK-MX30: #define mc68000 1
+// CHECK-MX

[PATCH] D97659: BPF: fix enum value 0 issue for __builtin_preserve_enum_value()

2021-02-28 Thread Yonghong Song via Phabricator via cfe-commits

yonghong-song created this revision.
yonghong-song added reviewers: ast, anakryiko.
yonghong-song requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Lorenz Bauer reported that the following code will have
compilation error for bpf target:

  enum e { TWO };
  bpf_core_enum_value_exists(enum e, TWO);

The clang emitted the following error message:

  __builtin_preserve_enum_value argument 1 invalid

In SemaChecking, an expression like "*(enum NAME)1" will have
cast kind CK_IntegralToPointer, but "*(enum NAME)0" will have
cast kind CK_NullToPointer. Current implementation only permits
CK_IntegralToPointer, missing enum value 0 case.

This patch permits CK_NullToPointer cast kind and 
the above test case can pass now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97659

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c


Index: clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
===
--- clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
+++ clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
@@ -4,10 +4,11 @@
 #define _(x, y) (__builtin_preserve_enum_value((x), (y)))
 
 enum AA {
+  VAL0 = 0,
   VAL1 = 2,
   VAL2 = 0x8000UL,
 };
-typedef enum { VAL10 = -2, VAL11 = 0x8000, }  __BB;
+typedef enum { VAL00, VAL10 = -2, VAL11 = 0x8000, }  __BB;
 
 unsigned unit1() {
   return _(*(enum AA *)VAL1, 0) + _(*(__BB *)VAL10, 1);
@@ -17,10 +18,16 @@
   return _(*(enum AA *)VAL2, 0) + _(*(__BB *)VAL11, 1);
 }
 
+unsigned unit3() {
+  return _(*(enum AA *)VAL0, 0) + _(*(__BB *)VAL00, 1);
+}
+
 // CHECK: @0 = private unnamed_addr constant [7 x i8] c"VAL1:2\00", align 1
 // CHECK: @1 = private unnamed_addr constant [9 x i8] c"VAL10:-2\00", align 1
 // CHECK: @2 = private unnamed_addr constant [17 x i8] c"VAL2:-2147483648\00", 
align 1
 // CHECK: @3 = private unnamed_addr constant [17 x i8] c"VAL11:4294934528\00", 
align 1
+// CHECK: @4 = private unnamed_addr constant [7 x i8] c"VAL0:0\00", align 1
+// CHECK: @5 = private unnamed_addr constant [8 x i8] c"VAL00:0\00", align 1
 
 // CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 0, i8* getelementptr 
inbounds ([7 x i8], [7 x i8]* @0, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[ENUM_AA:[0-9]+]]
 // CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 1, i8* getelementptr 
inbounds ([9 x i8], [9 x i8]* @1, i32 0, i32 0), i64 1), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[TYPEDEF_ENUM:[0-9]+]]
@@ -28,5 +35,8 @@
 // CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 2, i8* getelementptr 
inbounds ([17 x i8], [17 x i8]* @2, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[ENUM_AA]]
 // CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 3, i8* getelementptr 
inbounds ([17 x i8], [17 x i8]* @3, i32 0, i32 0), i64 1), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[TYPEDEF_ENUM]]
 
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 4, i8* getelementptr 
inbounds ([7 x i8], [7 x i8]* @4, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[ENUM_AA]]
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 5, i8* getelementptr 
inbounds ([8 x i8], [8 x i8]* @5, i32 0, i32 0), i64 1), !dbg !{{[0-9]+}}, 
!llvm.preserve.access.index ![[TYPEDEF_ENUM]]
+
 // CHECK: ![[ENUM_AA]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: 
"AA"
 // CHECK: ![[TYPEDEF_ENUM]] = !DIDerivedType(tag: DW_TAG_typedef, name: "__BB"
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2626,7 +2626,10 @@
 return false;
 
   const auto *CE = dyn_cast(UO->getSubExpr());
-  if (!CE || CE->getCastKind() != CK_IntegralToPointer)
+  if (!CE)
+return false;
+  if (CE->getCastKind() != CK_IntegralToPointer &&
+  CE->getCastKind() != CK_NullToPointer)
 return false;
 
   // The integer must be from an EnumConstantDecl.


Index: clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
===
--- clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
+++ clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
@@ -4,10 +4,11 @@
 #define _(x, y) (__builtin_preserve_enum_value((x), (y)))
 
 enum AA {
+  VAL0 = 0,
   VAL1 = 2,
   VAL2 = 0x8000UL,
 };
-typedef enum { VAL10 = -2, VAL11 = 0x8000, }  __BB;
+typedef enum { VAL00, VAL10 = -2, VAL11 = 0x8000, }  __BB;
 
 unsigned unit1() {
   return _(*(enum AA *)VAL1, 0) + _(*(__BB *)VAL10, 1);
@@ -17,10 +18,16 @@
   return _(*(enum AA *)VAL2, 0) + _(*(__BB *)VAL11, 1);
 }
 
+unsigned unit3() {
+  return _(*(enum AA *)VAL0, 0) + _(*(__BB *)VAL00, 1);
+}
+
 // CHECK: @0 = private unnamed_addr constant [7 x i8] c"VAL1:2\00", align 1
 // CHECK: @1 = private unnamed

39 matches

Mail list logo