[PATCH] D95754: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.

2021-01-30 Thread Justin Lebar via Phabricator via cfe-commits
jlebar created this revision.
jlebar requested review of this revision.
Herald added a project: clang.

Previously, -fshow-overloads=best always showed 4 candidates.  The
problem is, when this isn't enough, you're kind of up a creek; the only
option available is to recompile with different flags.  This can be
quite expensive!

With this change, we try to strike a compromise.  The *first* error with
more than 4 candidates will show up to 32 candidates.  All further
errors continue to show only 4 candidates.

The hope is that this way, users will have *some chance* of making
forward progress, without facing unbounded amounts of error spam.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95754

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
  clang/test/SemaCXX/overloaded-builtin-operators.cpp

Index: clang/test/SemaCXX/overloaded-builtin-operators.cpp
===
--- clang/test/SemaCXX/overloaded-builtin-operators.cpp
+++ clang/test/SemaCXX/overloaded-builtin-operators.cpp
@@ -195,8 +195,7 @@
 
 void test_dr425(A a) {
   (void)(1.0f * a); // expected-error{{ambiguous}} \
-// expected-note 4{{candidate}} \
-// expected-note {{remaining 8 candidates omitted; pass -fshow-overloads=all to show them}}
+// expected-note 12{{candidate}}
 }
 
 // pr5432
Index: clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
===
--- clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
+++ clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
@@ -10,9 +10,20 @@
   S(signed int*);
 };
 void f(const S& s);
-void g() {
-  f(0);
-}
+
+// First call to f emits all candidates.  Second call emits just the first 4.
+void g() { f(0); }
+// CHECK: {{conversion from 'int' to 'const S' is ambiguous}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+
+void h() { f(0); }
 // CHECK: {{conversion from 'int' to 'const S' is ambiguous}}
 // CHECK-NEXT: {{candidate constructor}}
 // CHECK-NEXT: {{candidate constructor}}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10354,18 +10354,15 @@
  const PartialDiagnostic ) const {
   S.Diag(CaretLoc, PDiag)
 << Ambiguous.getFromType() << Ambiguous.getToType();
-  // FIXME: The note limiting machinery is borrowed from
-  // OverloadCandidateSet::NoteCandidates; there's an opportunity for
-  // refactoring here.
-  const OverloadsShown ShowOverloads = S.Diags.getShowOverloads();
   unsigned CandsShown = 0;
   AmbiguousConversionSequence::const_iterator I, E;
   for (I = Ambiguous.begin(), E = Ambiguous.end(); I != E; ++I) {
-if (CandsShown >= 4 && ShowOverloads == Ovl_Best)
+if (CandsShown >= S.Diags.getNumOverloadCandidatesToShow())
   break;
 ++CandsShown;
 S.NoteOverloadCandidate(I->first, I->second);
   }
+  S.Diags.noteNumOverloadCandidatesShown(CandsShown);
   if (I != E)
 S.Diag(SourceLocation(), diag::note_ovl_too_many_candidates) << int(E - I);
 }
@@ -11643,7 +11640,7 @@
  (Cand.Function->template hasAttr() &&
   Cand.Function->template hasAttr());
 });
-DeferHint = WrongSidedCands.size();
+DeferHint = !WrongSidedCands.empty();
   }
   return DeferHint;
 }
@@ -11676,10 +11673,8 @@
   for (; I != E; ++I) {
 OverloadCandidate *Cand = *I;
 
-// Set an arbitrary limit on the number of candidate functions we'll spam
-// the user with.  FIXME: This limit should depend on details of the
-// candidate list.
-if (CandsShown >= 4 && ShowOverloads == Ovl_Best) {
+if (CandsShown >= S.Diags.getNumOverloadCandidatesToShow() &&
+ShowOverloads == Ovl_Best) {
   break;
 }
 ++CandsShown;
@@ -11708,6 +11703,10 @@
 }
   }
 
+  // Inform S.Diags that we've shown an overload set with N elements.  This may
+  // inform the future value of S.Diags.getNumOverloadCandidatesToShow().
+  S.Diags.noteNumOverloadCandidatesShown(CandsShown);
+
   if (I != E)
 S.Diag(OpLoc, diag::note_ovl_too_many_candidates,
shouldDeferDiags(S, Args, OpLoc))
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2291,9 +2291,7 @@
   int SuppressedOverloads = 0;
   for (UnresolvedSetImpl::iterator It = Overloads.begin(),

[PATCH] D93023: Replace deprecated %T in 2 tests.

2021-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93023

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


[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D95714#2532516 , @poelmanc wrote:

> @njames93 Thank you so much for the quick feedback, I made your suggested 
> changes and added a test that it properly converts `result = (a1 > (ptr == 0 
> ? a1 : a2));` to `result = (a1 > (ptr == nullptr ? a1 : a2));` now.
> This looks a little bit different from your AST but I'm not sure what code 
> your AST was generated from. If you have a test case I'd be happy to take a 
> look. Thanks!

I used the same test code as yours, but rather than creating my own 
`std::strong_ordering` I included the compare header. For reference I was using 
a trunk clang build with a trunk version of libstdc++ - 
https://godbolt.org/z/PrjhbK

This does highlight an issue where the mimicked std library stubs used for 
tests don't correspond exactly to what the stdlib actually looks like and can 
result in subtly different ASTs that have added/removed implicit nodes.

Going a little off point here but a few months back I pushed a fix for another 
check that passed its tests. 
However the bug report was re-opened as the bug was still observable in the 
real word. 
Turned out the implementation of std::string used for the test had a trivial 
destructor resulting in the AST not needed to emit `CXXBindTemporaryExpr`s all 
over the place, which threw off the matching logic.

Unfortunately this kind of disparity is hard to detect in tests so it may be 
wise to test this locally using the compare header from a real standard library 
implementation (preferably all 3 main ones if you have the machines) and see if 
this behaviour is correct.


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

https://reviews.llvm.org/D95714

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


[PATCH] D95702: [AIX] Improve option processing for mabi=vec-extabi and mabi=vec=defaul

2021-01-30 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95702

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


[PATCH] D95753: [CoverageMapping] Don't absolutize source paths

2021-01-30 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

@vsk @davidxl I'd be interested in your opinion. I've made this change 
unconditionally to match the compiler behavior in cases like debug info or 
`__FILE__` expansion, but I'm also happy to conditionalize it on a flag (for 
example `-f[no]-coverage-mapping-abspath`) if you'd like to avoid changing the 
current behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95753

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


[PATCH] D95753: [CoverageMapping] Don't absolutize source paths

2021-01-30 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: vsk, davidxl.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We currently always absolutize paths in coverage mapping. This is
problematic for reproducibility and it poses a problem for distributed
compilation where source location might differ across machines. It also
doesn't match path handling elsewhere in the compiler, for example in
debug info or __FILE__ expansion: we use the path as given to the
compiler, that is if Clang is invoked with a relative source path then
we use the relative path, and if invoked with an absolute path, then we
use the absolute path. This patch changes the coverage mapping behavior
to match that. We still cannonicalize the path by removing dots to avoid
duplicates in the coverage report.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95753

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/abspath.cpp
  clang/test/Profile/profile-prefix-map.c


Index: clang/test/Profile/profile-prefix-map.c
===
--- clang/test/Profile/profile-prefix-map.c
+++ clang/test/Profile/profile-prefix-map.c
@@ -5,10 +5,14 @@
 // RUN: echo "void f1() {}" > %t/root/nested/profile-prefix-map.c
 // RUN: cd %t/root
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
nested/profile-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
%t/root/nested/profile-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
 //
 // ABSOLUTE: @__llvm_coverage_mapping = 
{{.*"\\01.*root.*nested.*profile-prefix-map\.c}}
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
nested/profile-prefix-map.c -fprofile-prefix-map=%/t/root=. -o - | FileCheck 
--check-prefix=PROFILE-PREFIX-MAP %s --implicit-check-not=root
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
../root/nested/profile-prefix-map.c -o - | FileCheck --check-prefix=RELATIVE %s
 //
+// RELATIVE: @__llvm_coverage_mapping = 
{{.*"\\01[^/]*}}..{{/|\\+}}root{{/|\\+}}nested{{.*profile-prefix-map\.c}}
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
%t/root/nested/profile-prefix-map.c -fprofile-prefix-map=%/t/root=. -o - | 
FileCheck --check-prefix=PROFILE-PREFIX-MAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c 
../root/nested/profile-prefix-map.c -fprofile-prefix-map=../root=. -o - | 
FileCheck --check-prefix=PROFILE-PREFIX-MAP %s
 // PROFILE-PREFIX-MAP: @__llvm_coverage_mapping = 
{{.*"\\01[^/]*}}.{{/|\\+}}nested{{.*profile-prefix-map\.c}}
Index: clang/test/CoverageMapping/abspath.cpp
===
--- clang/test/CoverageMapping/abspath.cpp
+++ clang/test/CoverageMapping/abspath.cpp
@@ -7,9 +7,14 @@
 // RUN: mkdir -p %t/test && cd %t/test
 // RUN: echo "void f1() {}" > f1.c
 // RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -mllvm 
-enable-name-compression=false -emit-llvm -main-file-name abspath.cpp 
../test/f1.c -o - | FileCheck -check-prefix=RELPATH %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -mllvm 
-enable-name-compression=false -emit-llvm -main-file-name abspath.cpp 
%t/test/f1.c -o - | FileCheck -check-prefix=ABSPATH %s
 
 // RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
-// RELPATH: {{[/\\].*(/|)test(/|)f1}}.c
+// RELPATH: {{..(/|)test(/|)f1}}.c
 // RELPATH: "
 
+// ABSPATH: @__llvm_coverage_mapping = {{.*}}"\01
+// ABSPATH: {{[/\\].*(/|)test(/|)f1}}.c
+// ABSPATH: "
+
 void f1() {}
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1593,7 +1593,6 @@
 
 std::string CoverageMappingModuleGen::normalizeFilename(StringRef Filename) {
   llvm::SmallString<256> Path(Filename);
-  llvm::sys::fs::make_absolute(Path);
   llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
   for (const auto  : ProfilePrefixMap) {
 if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))


Index: clang/test/Profile/profile-prefix-map.c

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 320333.
njames93 added a comment.

Small fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93800

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/FormatProvider.cpp
  clang-tools-extra/clangd/FormatProvider.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
 #include "index/Index.h"
@@ -60,6 +61,8 @@
   std::vector ExtraArgs;
 
   TidyProvider ClangTidyProvider = {};
+
+  mutable FormatProvider ClangFormatProvider = formatFallbackProvider;
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -9,6 +9,7 @@
 #include "TestTU.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "FormatProvider.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
@@ -61,6 +62,9 @@
   Inputs.Opts = ParseOptions();
   if (ClangTidyProvider)
 Inputs.ClangTidyProvider = ClangTidyProvider;
+  ClangFormatProvider =
+  getClangFormatProvider(FS, format::DefaultFallbackStyle);
+  Inputs.ClangFormatProvider = ClangFormatProvider;
   Inputs.Index = ExternalIndex;
   return Inputs;
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "Matchers.h"
 #include "Protocol.h"
 #include "Quality.h"
@@ -154,6 +155,7 @@
   MockFS FS;
   Annotations Test(Text);
   ParseInputs ParseInput{tooling::CompileCommand(), , Test.code().str()};
+  ParseInput.ClangFormatProvider = formatFallbackProvider;
   return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
   Opts);
 }
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -9,6 +9,7 @@
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Features.inc"
+#include "FormatProvider.h"
 #include "PathMapping.h"
 #include "Protocol.h"
 #include "TidyProvider.h"
@@ -860,6 +861,9 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  FormatProvider ClangFormatOptProvider =
+  getClangFormatProvider(TFS, FallbackStyle);
+  Opts.ClangFormatProvider = ClangFormatOptProvider;
   Opts.AsyncPreambleBuilds = AsyncPreamble;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak ) {
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -76,7 +76,7 @@
   // from buildInvocation
   ParseInputs Inputs;
   std::unique_ptr Invocation;
-  format::FormatStyle Style;
+  std::shared_ptr Style;
   // from buildAST
   std::shared_ptr Preamble;
   llvm::Optional AST;
@@ -135,6 +135,7 @@
 return false;
   }
 }
+Inputs.ClangFormatProvider = Opts.ClangFormatProvider;
 log("Parsing command...");
 Invocation =
 buildCompilerInvocation(Inputs, CaptureInvocationDiags, );
@@ -148,7 +149,7 @@
 
 // FIXME: Check that resource-dir/built-in-headers exist?
 
-Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+Style = Inputs.ClangFormatProvider(File, Inputs.Contents);
 
 return true;
   }
@@ -216,7 +217,7 @@
   unsigned Definitions = locateSymbolAt(*AST, Pos, ).size();
   vlog("definition: {0}", Definitions);
 
- 

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

2021-01-30 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment.

@njames93 Thank you so much for the quick feedback, I made your suggested 
changes and added a test that it properly converts `result = (a1 > (ptr == 0 ? 
a1 : a2));` to `result = (a1 > (ptr == nullptr ? a1 : a2));` now.

In these examples so far, checking the grandparent works. Here's the AST:

  $ clang -std=c++20 -Xclang -ast-dump modernize-use-nullptr-cxx20.cpp
  ...
  |-BinaryOperator 0x22d30bec4e0  'bool' lvalue '=' // 
result = (a1 < a2); // result = (a1 > (ptr == 0 ? a1 : a2));
  | |-DeclRefExpr 0x22d30be8960  'bool' lvalue Var 0x22d30be88e0 
'result' 'bool'
  | `-ParenExpr 0x22d30bec4c0  'bool'
  |   `-CXXRewrittenBinaryOperator 0x22d30bec4a8  'bool'
  | `-CXXOperatorCallExpr 0x22d30bec470  'bool' '<' adl
  |   |-ImplicitCastExpr 0x22d30bec458  'bool (*)(const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' 

  |   | `-DeclRefExpr 0x22d30bec3d8  'bool (const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' lvalue 
Function 0x22d30ba9f28 'operator<' 'bool (const std::strong_ordering, 
std::strong_ordering::zero_type) noexcept'
  |   |-CXXOperatorCallExpr 0x22d30bec360  
'std::strong_ordering':'std::strong_ordering' '<=>'
  |   | |-ImplicitCastExpr 0x22d30bec348  'std::strong_ordering 
(*)(const A &) const noexcept' 
  |   | | `-DeclRefExpr 0x22d30be8a08  'std::strong_ordering 
(const A &) const noexcept' lvalue CXXMethod 0x22d30bedcd8 'operator<=>' 
'std::strong_ordering (const A &) const noexcept'
  |   | |-ImplicitCastExpr 0x22d30be89f0  'const A' lvalue 

  |   | | `-DeclRefExpr 0x22d30be8998  'A' lvalue Var 
0x22d30be8290 'a1' 'A'
  |   | `-ImplicitCastExpr 0x22d30be89d8  'const A' lvalue 

  |   |   `-DeclRefExpr 0x22d30be89b8  'A' lvalue Var 
0x22d30be87f0 'a2' 'A'
  |   `-ImplicitCastExpr 0x22d30bec3c0  
'std::strong_ordering::zero_type':'nullptr_t'  // Auto-generated 
cast we should ignore
  | `-IntegerLiteral 0x22d30bec398  'int' 0
  ...
  `-BinaryOperator 0x22d30becb20  'bool' lvalue '='
|-DeclRefExpr 0x22d30bec830  'bool' lvalue Var 0x22d30be88e0 
'result' 'bool'
`-ParenExpr 0x22d30becb00  'bool'
  `-CXXRewrittenBinaryOperator 0x22d30becae8  'bool'
`-CXXOperatorCallExpr 0x22d30becab0  'bool' '>' adl
  |-ImplicitCastExpr 0x22d30beca98  'bool (*)(const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' 

  | `-DeclRefExpr 0x22d30beca78  'bool (const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' lvalue 
Function 0x22d30baa1a0 'operator>' 'bool (const std::strong_ordering, 
std::strong_ordering::zero_type) noexcept'
  |-CXXOperatorCallExpr 0x22d30beca00  
'std::strong_ordering':'std::strong_ordering' '<=>'
  | |-ImplicitCastExpr 0x22d30bec9e8  'std::strong_ordering 
(*)(const A &) const noexcept' 
  | | `-DeclRefExpr 0x22d30bec9c8  'std::strong_ordering 
(const A &) const noexcept' lvalue CXXMethod 0x22d30bedcd8 'operator<=>' 
'std::strong_ordering (const A &) const noexcept'
  | |-ImplicitCastExpr 0x22d30bec9b0  'const A' lvalue 

  | | `-DeclRefExpr 0x22d30bec850  'A' lvalue Var 
0x22d30be8290 'a1' 'A'
  | `-ImplicitCastExpr 0x22d30bec998  'const A' 
lvalue 
  |   `-ParenExpr 0x22d30bec978  'A' lvalue
  | `-ConditionalOperator 0x22d30bec948  'A' 
lvalue
  |   |-BinaryOperator 0x22d30bec8e8  'bool' 
'=='
  |   | |-ImplicitCastExpr 0x22d30bec8b8  'int *' 

  |   | | `-DeclRefExpr 0x22d30bec870  'int *' lvalue 
Var 0x22d30bec758 'ptr' 'int *'
  |   | `-ImplicitCastExpr 0x22d30bec8d0  'int *' 
 // ptr == 0 cast that we want to convert to nullptr
  |   |   `-IntegerLiteral 0x22d30bec890  'int' 0
  |   |-DeclRefExpr 0x22d30bec908  'A' lvalue Var 
0x22d30be8290 'a1' 'A'
  |   `-DeclRefExpr 0x22d30bec928  'A' lvalue Var 
0x22d30be87f0 'a2' 'A'
  `-ImplicitCastExpr 0x22d30beca60  
'std::strong_ordering::zero_type':'nullptr_t'  // Auto-generated 
cast we should ignore
`-IntegerLiteral 0x22d30beca38  'int' 0

This looks a little bit different from your AST but I'm not sure what code your 
AST was generated from. If you have a test case I'd be happy to take a look. 
Thanks!


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

https://reviews.llvm.org/D95714

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


[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

2021-01-30 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc updated this revision to Diff 320330.
poelmanc added a comment.

Thanks to the great feedback, changed 
`unless(hasAncestor(cxxRewrittenBinaryOperator()))` to 
`unless(hasParent(expr(hasParent(cxxRewrittenBinaryOperator()` and added a 
test to verify the improvement (and removed an extraneous comment.)


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

https://reviews.llvm.org/D95714

Files:
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-nullptr %t
+
+namespace std {
+
+struct strong_ordering {
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+  using zero_type = decltype(nullptr);
+
+  friend constexpr bool operator<(const strong_ordering value, zero_type) 
noexcept {
+return value.comparison_value < 0;
+  }
+
+  friend constexpr bool operator>(const strong_ordering value, zero_type) 
noexcept {
+return value.comparison_value > 0;
+  }
+
+  friend constexpr bool operator>=(const strong_ordering value, zero_type) 
noexcept {
+return value.comparison_value >= 0;
+  }
+
+  signed char comparison_value;
+};
+
+inline constexpr strong_ordering strong_ordering::less{-1};
+inline constexpr strong_ordering strong_ordering::equal{0};
+inline constexpr strong_ordering strong_ordering::greater{1};
+
+} // namespace std
+
+class A {
+public:
+  auto operator<=>(const A ) const = default;
+};
+
+void test_cxx_rewritten_binary_ops() {
+  A a1, a2;
+  bool result;
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 < a2);
+  // CHECK-FIXES: result = (a1 < a2);
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 >= a2);
+  // CHECK-FIXES: result = (a1 >= a2);
+  int *ptr = 0;
+  // CHECK-FIXES: int *ptr = nullptr;
+  result = (a1 > (ptr == 0 ? a1 : a2));
+  // CHECK-FIXES: result = (a1 > (ptr == nullptr ? a1 : a2));
+}
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -45,6 +45,8 @@
   TK_AsIs,
   castExpr(anyOf(ImplicitCastToNull,
  explicitCastExpr(hasDescendant(ImplicitCastToNull))),
+   // Skip implicit casts to 0 generated by operator<=> rewrites.
+   
unless(hasParent(expr(hasParent(cxxRewrittenBinaryOperator(),
unless(hasAncestor(explicitCastExpr(
   .bind(CastSequence));
 }


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-nullptr %t
+
+namespace std {
+
+struct strong_ordering {
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+  using zero_type = decltype(nullptr);
+
+  friend constexpr bool operator<(const strong_ordering value, zero_type) noexcept {
+return value.comparison_value < 0;
+  }
+
+  friend constexpr bool operator>(const strong_ordering value, zero_type) noexcept {
+return value.comparison_value > 0;
+  }
+
+  friend constexpr bool operator>=(const strong_ordering value, zero_type) noexcept {
+return value.comparison_value >= 0;
+  }
+
+  signed char comparison_value;
+};
+
+inline constexpr strong_ordering strong_ordering::less{-1};
+inline constexpr strong_ordering strong_ordering::equal{0};
+inline constexpr strong_ordering strong_ordering::greater{1};
+
+} // namespace std
+
+class A {
+public:
+  auto operator<=>(const A ) const = default;
+};
+
+void test_cxx_rewritten_binary_ops() {
+  A a1, a2;
+  bool result;
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 < a2);
+  // CHECK-FIXES: result = (a1 < a2);
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 >= a2);
+  // CHECK-FIXES: result = (a1 >= a2);
+  int *ptr = 0;
+  // CHECK-FIXES: int *ptr = nullptr;
+  result = (a1 > (ptr == 0 ? a1 : a2));
+  // CHECK-FIXES: result = (a1 > (ptr == nullptr ? a1 : a2));
+}
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -45,6 +45,8 @@
   TK_AsIs,
   

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D95168#2532458 , @curdeius wrote:

> In D95168#2532410 , @steveire wrote:
>
>> In D95168#2532258 , @MyDeveloperDay 
>> wrote:
>>
>>> I wonder if we should consider suggesting a different type of tool for clang
>>>
>>> `clang-reformat`
>>>
>>> A place where changes such as this and east/west fixer could be actively 
>>> encouraged.
>>
>> I don't think this should be done. These kinds of things should be in 
>> `clang-format`. One of the advantages of this and east/west const being in 
>> clang-format is that editors, CI systems and other tools have clang-format 
>> support. They would be unlikely to get support for a new tool. There are 
>> plenty of clang tools which exist but which don't get enough attention to 
>> get support in editors CI tools etc.
>
> Not saying that I'm in favour of creating another tool, but...
> I believe that such a tool, if it were pretty much a drop-in replacement of 
> clang-format, it could profit from the current tooling support.

If it's a drop in replacement (does everything clang-format does and more), 
what's the benefit for that cost?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

In D95168#2532410 , @steveire wrote:

> In D95168#2532258 , @MyDeveloperDay 
> wrote:
>
>> I wonder if we should consider suggesting a different type of tool for clang
>>
>> `clang-reformat`
>>
>> A place where changes such as this and east/west fixer could be actively 
>> encouraged.
>
> I don't think this should be done. These kinds of things should be in 
> `clang-format`. One of the advantages of this and east/west const being in 
> clang-format is that editors, CI systems and other tools have clang-format 
> support. They would be unlikely to get support for a new tool. There are 
> plenty of clang tools which exist but which don't get enough attention to get 
> support in editors CI tools etc.

Not saying that I'm in favour of creating another tool, but...
I believe that such a tool, if it were pretty much a drop-in replacement of 
clang-format, it could profit from the current tooling support.
Why? Because most of them let you define the clang-format binary path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[PATCH] D95665: [Builtins][FIX] Allow targets to opt-out of `long double` builtins

2021-01-30 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert abandoned this revision.
jdoerfert added a comment.

I guess this doesn't work then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95665

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


[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

The idea has been floated to create a new different tool for changes like this 
(see eg D95168 ).

I don't think this should be done. These kinds of things should be in 
clang-format. One of the advantages of this and east/west const being in 
clang-format is that editors, CI systems and other tools have clang-format 
support. They would be unlikely to get support for a new tool. There are plenty 
of clang tools which exist but which don't get enough attention to get support 
in editors CI tools etc.

What can be done to move this change along?


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

https://reviews.llvm.org/D69764

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


[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D95168#2532258 , @MyDeveloperDay 
wrote:

> I wonder if we should consider suggesting a different type of tool for clang
>
> `clang-reformat`
>
> A place where changes such as this and east/west fixer could be actively 
> encouraged.

I don't think this should be done. These kinds of things should be in 
`clang-format`. One of the advantages of this and east/west const being in 
clang-format is that editors, CI systems and other tools have clang-format 
support. They would be unlikely to get support for a new tool. There are plenty 
of clang tools which exist but which don't get enough attention to get support 
in editors CI tools etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 320319.
njames93 added a comment.

Hopefully fix the build failing. ASAN said it was a use-after-scope issue which 
seemed suspicious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93800

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/FormatProvider.cpp
  clang-tools-extra/clangd/FormatProvider.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
 #include "index/Index.h"
@@ -60,6 +61,8 @@
   std::vector ExtraArgs;
 
   TidyProvider ClangTidyProvider = {};
+
+  mutable FormatProvider ClangFormatProvider = formatFallbackProvider;
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -9,6 +9,7 @@
 #include "TestTU.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "FormatProvider.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
@@ -61,6 +62,9 @@
   Inputs.Opts = ParseOptions();
   if (ClangTidyProvider)
 Inputs.ClangTidyProvider = ClangTidyProvider;
+  ClangFormatProvider =
+  getClangFormatProvider(FS, format::DefaultFallbackStyle);
+  Inputs.ClangFormatProvider = ClangFormatProvider;
   Inputs.Index = ExternalIndex;
   return Inputs;
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "Matchers.h"
 #include "Protocol.h"
 #include "Quality.h"
@@ -154,6 +155,7 @@
   MockFS FS;
   Annotations Test(Text);
   ParseInputs ParseInput{tooling::CompileCommand(), , Test.code().str()};
+  ParseInput.ClangFormatProvider = formatFallbackProvider;
   return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
   Opts);
 }
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -9,6 +9,7 @@
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Features.inc"
+#include "FormatProvider.h"
 #include "PathMapping.h"
 #include "Protocol.h"
 #include "TidyProvider.h"
@@ -860,6 +861,9 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  FormatProvider ClangFormatOptProvider =
+  getClangFormatProvider(TFS, FallbackStyle);
+  Opts.ClangFormatProvider = ClangFormatOptProvider;
   Opts.AsyncPreambleBuilds = AsyncPreamble;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak ) {
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -76,7 +76,7 @@
   // from buildInvocation
   ParseInputs Inputs;
   std::unique_ptr Invocation;
-  format::FormatStyle Style;
+  std::shared_ptr Style;
   // from buildAST
   std::shared_ptr Preamble;
   llvm::Optional AST;
@@ -135,6 +135,7 @@
 return false;
   }
 }
+Inputs.ClangFormatProvider = Opts.ClangFormatProvider;
 log("Parsing command...");
 Invocation =
 buildCompilerInvocation(Inputs, CaptureInvocationDiags, );
@@ -148,7 +149,7 @@
 
 // FIXME: Check that resource-dir/built-in-headers exist?
 
-Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+Style = Inputs.ClangFormatProvider(File, Inputs.Contents);
 
 return true;
   }
@@ -216,7 +217,7 @@
   unsigned Definitions = 

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-01-30 Thread Daan De Meyer via Phabricator via cfe-commits
DaanDeMeyer created this revision.
DaanDeMeyer added a reviewer: clang.
DaanDeMeyer added a project: clang.
DaanDeMeyer requested review of this revision.
Herald added a subscriber: cfe-commits.

When compiling UEFI applications, the main function is named
efi_main() instead of main(). Let's exclude efi_main() from
-Wmissing-prototypes as well to avoid warnings when working
on UEFI applications.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D95745: Support unwinding from inline assembly

2021-01-30 Thread Paul via Phabricator via cfe-commits
cynecx added a comment.

Tests are missing right now. But I'd like to get some feedback first whether 
this approach goes in the right direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95745

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


[PATCH] D95745: Support unwinding from inline assembly

2021-01-30 Thread Paul via Phabricator via cfe-commits
cynecx created this revision.
cynecx added reviewers: LLVM, clang.
cynecx added projects: LLVM, clang.
Herald added a reviewer: deadalnix.
Herald added subscribers: dexonsmith, hiraditya.
cynecx requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, aheejin.

I haven't received any feedback about the approach I've taken yet 
(https://lists.llvm.org/pipermail/llvm-dev/2021-January/148175.html).
So I thought I'd try my luck here.

A splitted patchset can be found here: 
https://github.com/cynecx/llvm-project/commits/invokeasm2

I've taken the following steps to add support to unwinding from inline assembly:

1. Add a new `canThrow` "attribute" (like `sideeffect`) to the asm syntax:

  invoke void asm sideeffect unwind "call thrower", 
"~{dirflag},~{fpsr},~{flags}"()
  to label %exit unwind label %uexit

2.) Add Bitcode writing/reading support + LLVM-IR parsing.

3.) Emit EHLabels around inline assembly lowering (SelectionDAGBuilder + 
GlobalISel) when canThrow is enabled.

4.) Tweak InstCombineCalls/InlineFunction pass to not mark inline assembly 
"calls" as nounwind.

5.) Add clang support by introducing a new clobber: "unwind", which lower to 
the `canThrow` being enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95745

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  llvm/bindings/go/llvm/ir.go
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/InlineAsm.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantsContext.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp

Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -56,6 +56,7 @@
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
+#include "llvm/IR/InlineAsm.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -543,9 +544,16 @@
 // instructions require no special handling.
 CallInst *CI = dyn_cast(I);
 
-if (!CI || CI->doesNotThrow() || CI->isInlineAsm())
+if (!CI || CI->doesNotThrow())
   continue;
 
+if (CI->isInlineAsm()) {
+  InlineAsm *IA = cast(CI->getCalledOperand());
+  if (!IA->canThrow()) {
+continue;
+  }
+}
+
 // We do not need to (and in fact, cannot) convert possibly throwing calls
 // to @llvm.experimental_deoptimize (resp. @llvm.experimental.guard) into
 // invokes.  The caller's "segment" of the deoptimization continuation
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -56,6 +56,7 @@
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueHandle.h"
+#include "llvm/IR/InlineAsm.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
@@ -2121,9 +2122,13 @@
   }
 
   if (isa(Callee) && !Call.doesNotThrow()) {
-// Inline asm calls cannot throw - mark them 'nounwind'.
-Call.setDoesNotThrow();
-Changed = true;
+InlineAsm *IA = cast(Callee);
+if (!IA->canThrow()) {
+  // Normal inline asm calls cannot throw - mark them
+  // 'nounwind'.
+  Call.setDoesNotThrow();
+  Changed = true;
+}
   }
 
   // Try to optimize the call if possible, we require DataLayout for most of
Index: llvm/lib/IR/InlineAsm.cpp
===
--- llvm/lib/IR/InlineAsm.cpp
+++ llvm/lib/IR/InlineAsm.cpp
@@ -29,11 +29,11 @@
 
 InlineAsm::InlineAsm(FunctionType *FTy, const std::string ,
  const std::string , bool hasSideEffects,
- bool isAlignStack, AsmDialect asmDialect)
+ bool isAlignStack, AsmDialect asmDialect, bool canThrow)
 : Value(PointerType::getUnqual(FTy), Value::InlineAsmVal),
   AsmString(asmString), Constraints(constraints), FTy(FTy),
   HasSideEffects(hasSideEffects), IsAlignStack(isAlignStack),
-  Dialect(asmDialect) {
+  Dialect(asmDialect), CanThrow(canThrow) {
   // Do various checks on the constraint string and type.
   assert(Verify(getFunctionType(), constraints) &&
  "Function type not legal for constraints!");
@@ -41,9 +41,10 @@
 
 

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

2021-01-30 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:684-685
+  if (!GeneratedStringsEqual)
+llvm::report_fatal_error("Mismatch between arguments generated during "
+ "round-trip");
+

dexonsmith wrote:
> jansvoboda11 wrote:
> > dexonsmith wrote:
> > > Another option vs. report_fatal_error would be to create (fatal) clang 
> > > error diagnostics. That applies here, and to my previous comments. WDYT? 
> > > (@Bigcheese, any thoughts?)
> > > 
> > > Also, I think `-round-trip-args-debug` might not be super discoverable. 
> > > Maybe we can add a comment in the code where the errors are reported 
> > > suggesting adding that.
> > If we decide to go with custom diagnostics instead of just using 
> > `llvm::errs` and `llvm::report_fatal_error`, we'd need to instantiate a 
> > custom `DiagnosticsEngine` here in `RoundTrip`, because we cannot rely on 
> > clients passing us reasonable `Diags`.
> > 
> > One such example is `CompilerInvocationTest`, where we don't care what 
> > diagnostics get emitted (by using `TextDiagnosticBuffer` consumer). The 
> > errors we're emitting here would be still useful to see when testing though.
> I'm not entirely following why this would need a custom Diags. If the client 
> wants to ignore diagnostics that should be up to the client, especially in 
> clang-as-a-library situations. In fact, using `errs` or `dbgs` at all is 
> pretty iffy when clang is used as a library.
> 
> For CompilerInvocationTest, I imagine we could change the consumer / test 
> fixture / etc. somehow to capture this case.
My understanding is that we'd be converting `llvm::report_fatal_error` into 
error diagnostics and `PrintArgs` into notes and remarks.

The hiccup here is that `Diags` most probably hasn't been configured yet (we're 
still just parsing the `DiagnosticsOptions`). And by default, 
`DiagnosticsEngine` ignores all remarks.

So my thinking was that to ensure our remarks are not thrown away, we could 
modify the existing `Diags` to our liking (too invasive) or set up a custom 
`DiagnosticsEngine` instance here in `RoundTrip`. In the end, the best solution 
is probably to do this of setup in `cc1_main`, so that we're emitting 
diagnostics only when used within compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

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


[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

2021-01-30 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 updated this revision to Diff 320315.
jansvoboda11 added a comment.

Create proper diagnostics, add remark tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/command-line-round-trip.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Option/ArgList.h
  llvm/lib/Option/ArgList.cpp

Index: llvm/lib/Option/ArgList.cpp
===
--- llvm/lib/Option/ArgList.cpp
+++ llvm/lib/Option/ArgList.cpp
@@ -90,11 +90,22 @@
 }
 
 std::vector ArgList::getAllArgValues(OptSpecifier Id) const {
+  recordQueriedOpts(Id);
   SmallVector Values;
   AddAllArgValues(Values, Id);
   return std::vector(Values.begin(), Values.end());
 }
 
+void ArgList::AddAllArgsExcept(ArgStringList ,
+   const DenseSet ) const {
+  for (const Arg *Arg : *this) {
+if (!ExcludeIds.contains(Arg->getOption().getID())) {
+  Arg->claim();
+  Arg->render(*this, Output);
+}
+  }
+}
+
 void ArgList::AddAllArgsExcept(ArgStringList ,
ArrayRef Ids,
ArrayRef ExcludeIds) const {
Index: llvm/include/llvm/Option/ArgList.h
===
--- llvm/include/llvm/Option/ArgList.h
+++ llvm/include/llvm/Option/ArgList.h
@@ -137,6 +137,16 @@
   /// The first and last index of each different OptSpecifier ID.
   DenseMap OptRanges;
 
+  /// The OptSpecifiers that were queried from this argument list.
+  mutable DenseSet QueriedOpts;
+
+  /// Record the queried OptSpecifiers.
+  template 
+  void recordQueriedOpts(OptSpecifiers... Ids) const {
+SmallVector OptsSpecifiers({toOptSpecifier(Ids).getID()...});
+QueriedOpts.insert(OptsSpecifiers.begin(), OptsSpecifiers.end());
+  }
+
   /// Get the range of indexes in which options with the specified IDs might
   /// reside, or (0, 0) if there are no such options.
   OptRange getRange(std::initializer_list Ids) const;
@@ -203,6 +213,7 @@
   template
   iterator_range>
   filtered(OptSpecifiers ...Ids) const {
+recordQueriedOpts(Ids...);
 OptRange Range = getRange({toOptSpecifier(Ids)...});
 auto B = Args.begin() + Range.first;
 auto E = Args.begin() + Range.second;
@@ -214,6 +225,7 @@
   template
   iterator_range>
   filtered_reverse(OptSpecifiers ...Ids) const {
+recordQueriedOpts(Ids...);
 OptRange Range = getRange({toOptSpecifier(Ids)...});
 auto B = Args.rend() - Range.second;
 auto E = Args.rend() - Range.first;
@@ -308,6 +320,10 @@
   A->render(*this, Output);
   }
 
+  /// AddAllArgsExcept - Render all arguments not matching any of the excluded
+  /// ids.
+  void AddAllArgsExcept(ArgStringList ,
+const DenseSet ) const;
   /// AddAllArgsExcept - Render all arguments matching any of the given ids
   /// and not matching any of the excluded ids.
   void AddAllArgsExcept(ArgStringList , ArrayRef Ids,
@@ -342,6 +358,12 @@
   ///
   void ClaimAllArgs() const;
 
+  /// Return the OptSpecifiers queried from this argument list.
+  const DenseSet () const { return QueriedOpts; }
+
+  /// Clear the set of queried OptSpecifiers.
+  void clearQueriedOpts() const { QueriedOpts.clear(); }
+
   /// @}
   /// @name Arg Synthesis
   /// @{
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -203,6 +203,12 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
+
+  // Setup round-trip remarks for the DiagnosticsEngine used in CreateFromArgs.
+  if (find(Argv, StringRef("-Rround-trip")) != Argv.end())
+Diags.setSeverity(diag::remark_cc1_round_trip_generated,
+  diag::Severity::Remark, {});
+
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
Index: clang/test/Frontend/command-line-round-trip.c
===
--- /dev/null
+++ clang/test/Frontend/command-line-round-trip.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -Rround-trip 2>&1 | FileCheck %s -check-prefix=CHECK-WITHOUT-ROUND-TRIP -allow-empty
+// RUN: %clang_cc1 %s -round-trip-args 2>&1 | FileCheck %s -check-prefix=CHECK-ROUND-TRIP-WITHOUT-REMARKS -allow-empty
+// RUN: %clang_cc1 %s -round-trip-args -Rround-trip 2>&1 | FileCheck %s 

[PATCH] D95583: Frontend: Add -f{, no-}implicit-modules-uses-lock and -Rmodule-lock

2021-01-30 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/test/Modules/implicit-modules-use-lock.m:21
+
+// CHECK-NO-LOCKS-NOT: remark:
+// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' 
[-Rmodule-lock]

dexonsmith wrote:
> jansvoboda11 wrote:
> > Where is the empty remark coming from? Is it useful to us in any way?
> This is a `CHECK-NOT: ` line (with a custom `-check-prefix`). It 
> confirms that `` does not occur at least until the next positive check 
> matches. The `FileCheck` invocation that uses `CHECK-NO-LOCKS` has no other 
> check lines, so this really means: "`remark:` does not match on any line". 
> Note I also needed to add `-allow-empty` since with no diagnostics at all, 
> `FileCheck`'s stdin will be empty.
> 
> FYI, if you want to read more:
> - `-check-prefix` is documented at 
> https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-filecheck-check-prefix-option
> - `CHECK-NOT` is documented at 
> https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-not-directive
> 
> Often in clang tests it's easier to use `-cc1 -verify` for diagnostics 
> instead of manual `FileCheck`s (`expected-no-diagnostics` comment being the 
> equivalent of `-allow-empty`). In the modules testcases, there's often a 
> complicated setup that we want to run a lot of `RUN:` lines against where 
> each one expects different diagnostics. Maybe we should add prefix support to 
> `-verify` (or maybe it's there and no one told me...).
Ah, that makes sense, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95583

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


[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

Hmm, interesting idea. Do you have anything more precise in mind? Would it be 
an "augmented" clang-format? I.e. it will have all its options and some 
additional ones? Or rather more independent tool? Or clang-format's 
experimental field?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[PATCH] D95726: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons - hasGrandparent version

2021-01-30 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment.

Thanks for the quick feedback everyone. I agree too, withdrawing this in favor 
of https://reviews.llvm.org/D95714.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95726

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


[PATCH] D95726: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons - hasGrandparent version

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire requested changes to this revision.
steveire added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3451
+internal::TypeList>
+hasGrandparent;
+

I agree with @njames93 that we shouldn't introduce this matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95726

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


[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 320309.
njames93 added a comment.

Tests pass on my machine, rebasing on main and trigger a fresh rebuild.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93800

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/FormatProvider.cpp
  clang-tools-extra/clangd/FormatProvider.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
 #include "index/Index.h"
@@ -60,6 +61,8 @@
   std::vector ExtraArgs;
 
   TidyProvider ClangTidyProvider = {};
+
+  mutable FormatProvider ClangFormatProvider = 
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -9,6 +9,7 @@
 #include "TestTU.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "FormatProvider.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
@@ -61,6 +62,9 @@
   Inputs.Opts = ParseOptions();
   if (ClangTidyProvider)
 Inputs.ClangTidyProvider = ClangTidyProvider;
+  ClangFormatProvider =
+  getClangFormatProvider(FS, format::DefaultFallbackStyle);
+  Inputs.ClangFormatProvider = ClangFormatProvider;
   Inputs.Index = ExternalIndex;
   return Inputs;
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "Matchers.h"
 #include "Protocol.h"
 #include "Quality.h"
@@ -154,6 +155,7 @@
   MockFS FS;
   Annotations Test(Text);
   ParseInputs ParseInput{tooling::CompileCommand(), , Test.code().str()};
+  ParseInput.ClangFormatProvider = 
   return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
   Opts);
 }
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -9,6 +9,7 @@
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Features.inc"
+#include "FormatProvider.h"
 #include "PathMapping.h"
 #include "Protocol.h"
 #include "TidyProvider.h"
@@ -860,6 +861,9 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  FormatProvider ClangFormatOptProvider =
+  getClangFormatProvider(TFS, FallbackStyle);
+  Opts.ClangFormatProvider = ClangFormatOptProvider;
   Opts.AsyncPreambleBuilds = AsyncPreamble;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak ) {
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -76,7 +76,7 @@
   // from buildInvocation
   ParseInputs Inputs;
   std::unique_ptr Invocation;
-  format::FormatStyle Style;
+  std::shared_ptr Style;
   // from buildAST
   std::shared_ptr Preamble;
   llvm::Optional AST;
@@ -135,6 +135,7 @@
 return false;
   }
 }
+Inputs.ClangFormatProvider = Opts.ClangFormatProvider;
 log("Parsing command...");
 Invocation =
 buildCompilerInvocation(Inputs, CaptureInvocationDiags, );
@@ -148,7 +149,7 @@
 
 // FIXME: Check that resource-dir/built-in-headers exist?
 
-Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+Style = Inputs.ClangFormatProvider(File, Inputs.Contents);
 
 return true;
   }
@@ -216,7 +217,7 @@
   unsigned Definitions = locateSymbolAt(*AST, Pos, ).size();
   vlog("definition: {0}", 

[PATCH] D95726: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons - hasGrandparent version

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

Not a fan of this approach, as pointed out in D95714 
 the `hasGrandparent` functionality can be 
achieved by chaining `hasParent` calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95726

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


[PATCH] D95714: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Thanks for looking at this, its been on my todo list for long time.
Regarding the hasGrandparent idea, that could be recreated with 
`hasParent(expr(hasParent(cxxRewrittenBinaryOperator(`
Though looking at the (trimmed) AST I'm not sure that's enough. Appears to have 
4 levels of parents before reaching the cxxRewrittenBinaryOperator

  -CXXRewrittenBinaryOperator  'bool'
-CXXOperatorCallExpr  'bool' '<' adl
 |-ImplicitCastExpr  'bool (*)(std::strong_ordering, 
__cmp_cat::__unspec) noexcept' 
 |-CXXOperatorCallExpr  
'std::strong_ordering':'std::strong_ordering' '<=>'
  -ImplicitCastExpr  
'__cmp_cat::__unspec':'std::__cmp_cat::__unspec' 
-CXXConstructExpr  
'__cmp_cat::__unspec':'std::__cmp_cat::__unspec' 'void 
(std::__cmp_cat::__unspec *) noexcept'
  -ImplicitCastExpr  'std::__cmp_cat::__unspec *' 
 // Cast we care about
-IntegerLiteral  'int' 0




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp:3-4
+
+// Not all systems have #include  to define std::std_ordering
+// required by operator<=>, so recreate minimal version for tests below.
+namespace std {

We don't compile against a std library for tests so this comment is redundant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95714

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


[PATCH] D95448: [flang][driver] Add support for `-J/-module-dir`

2021-01-30 Thread Arnamoy B via Phabricator via cfe-commits
arnamoy10 updated this revision to Diff 320306.
arnamoy10 added a comment.

Addressing reviewers' comments, adding `-module-dir` as the default option 
instead of `-J`


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

https://reviews.llvm.org/D95448

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/include-module.f90

Index: flang/test/Flang-Driver/include-module.f90
===
--- flang/test/Flang-Driver/include-module.f90
+++ flang/test/Flang-Driver/include-module.f90
@@ -8,12 +8,28 @@
 !--
 ! RUN: not %flang-new -fsyntax-only -I %S/Inputs -I %S/Inputs/module-dir %s  2>&1 | FileCheck %s --check-prefix=INCLUDED
 ! RUN: not %flang-new -fsyntax-only -I %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -J %S/Inputs -I %S/Inputs/module-dir %s 2>&1 | FileCheck %s --check-prefix=INCLUDED
+! RUN: not %flang-new -fsyntax-only -J %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -module-dir %S/Inputs -I %S/Inputs/module-dir %s  2>&1 | FileCheck %s --check-prefix=INCLUDED
+! RUN: not %flang-new -fsyntax-only -module-dir %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -J %S/Inputs/module-dir -J %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -J %S/Inputs/module-dir -module-dir %S/Inputs/ %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -module-dir %S/Inputs/module-dir -J%S/Inputs/ %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fsyntax-only -module-dir %S/Inputs -I %S/Inputs/module-dir %s 2>&1 | FileCheck %s --check-prefix=INCLUDED
 
 !-
 ! FRONTEND FLANG DRIVER (flang-new -fc1)
 !-
 ! RUN: not %flang-new -fc1 -fsyntax-only -I %S/Inputs -I %S/Inputs/module-dir %s  2>&1 | FileCheck %s --check-prefix=INCLUDED
 ! RUN: not %flang-new -fc1 -fsyntax-only -I %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -J %S/Inputs -I %S/Inputs/module-dir %s 2>&1 | FileCheck %s --check-prefix=INCLUDED
+! RUN: not %flang-new -fc1 -fsyntax-only -J %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -module-dir %S/Inputs -I %S/Inputs/module-dir %s  2>&1 | FileCheck %s --check-prefix=INCLUDED
+! RUN: not %flang-new -fc1 -fsyntax-only -module-dir %S/Inputs %s  2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -J %S/Inputs/module-dir -J %S/Inputs/ %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -J %S/Inputs/module-dir -module-dir %S/Inputs/ %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -module-dir %S/Inputs/module-dir -J%S/Inputs/ %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE
+! RUN: not %flang-new -fc1 -fsyntax-only -module-dir %S/Inputs -I %S/Inputs/module-dir %s 2>&1 | FileCheck %s --check-prefix=INCLUDED
 
 !-
 ! EXPECTED OUTPUT FOR MISSING MODULE FILE
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -26,6 +26,7 @@
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -help  Display available options
 ! HELP-NEXT: -IAdd directory to the end of the list of include search paths
+! HELP-NEXT: -module-dirPut MODULE files in 'directory'
 ! HELP-NEXT: -o   Write output to 
 ! HELP-NEXT: -U  Undefine macro 
 ! HELP-NEXT: --version  Print version information
@@ -41,6 +42,7 @@
 ! HELP-FC1-NEXT: -E Only run the preprocessor
 ! HELP-FC1-NEXT: -help  Display available options
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of include search paths
+! HELP-FC1-NEXT: -module-dirPut MODULE files in 'directory'
 ! HELP-FC1-NEXT: -o   Write output to 
 ! HELP-FC1-NEXT: -U  Undefine macro 
 ! HELP-FC1-NEXT: --version  Print version information
Index: flang/test/Flang-Driver/driver-help-hidden.f90

[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:995
 TEST_F(FormatTest, ForEachLoops) {
   verifyFormat("void f() {\n"
+   "  foreach (Item *item, itemlist) {\n"

GoBigorGoHome wrote:
> MyDeveloperDay wrote:
> > I'd like you to assert that short loops are off
> You mean make sure that `AllowShortLoopsOnASingleLine` is false?
> The default style of `verifyFormat` is LLVM style where 
> `AllowShortLoopsOnASingleLine` is already false.
you should assert here to show that the previous tests were wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94955

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


[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

I wonder if we should consider suggesting a different type of tool for clang

`clang-reformat`

A place where changes such as this and east/west fixer could be actively 
encouraged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[clang] b10d445 - [ASTMatchers] Fix definition of decompositionDecl

2021-01-30 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-01-30T16:29:40Z
New Revision: b10d445307a0f3c7e5522836b4331090aacaf349

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

LOG: [ASTMatchers] Fix definition of decompositionDecl

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/ASTMatchersInternal.cpp

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 10532b3da209..cbce6d7d0439 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -344,7 +344,7 @@ extern const internal::VariadicAllOfMatcher decl;
 ///   int number = 42;
 ///   auto [foo, bar] = std::make_pair{42, 42};
 /// \endcode
-extern const internal::VariadicAllOfMatcher
+extern const internal::VariadicDynCastAllOfMatcher
 decompositionDecl;
 
 /// Matches a declaration of a linkage specification.

diff  --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp 
b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 6e36842d0660..6ebc72d450fe 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -732,7 +732,7 @@ const internal::VariadicDynCastAllOfMatcher typeAliasDecl;
 const internal::VariadicDynCastAllOfMatcher
 typeAliasTemplateDecl;
 const internal::VariadicAllOfMatcher decl;
-const internal::VariadicAllOfMatcher decompositionDecl;
+const internal::VariadicDynCastAllOfMatcher 
decompositionDecl;
 const internal::VariadicDynCastAllOfMatcher
 linkageSpecDecl;
 const internal::VariadicDynCastAllOfMatcher namedDecl;



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


[PATCH] D95740: [ASTMatchers] Ignore parts of BindingDecls which are not spelled in source

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision.
steveire added reviewers: aaron.ballman, alexfh.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95740

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2591,6 +2591,31 @@
 Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
 std::make_unique>("allExprs", 1)));
   }
+
+  Code = R"cpp(
+void foo()
+{
+int arr[3];
+auto &[f, s, t] = arr;
+
+f = 42;
+}
+  )cpp";
+  {
+auto M = bindingDecl(hasName("f"), has(expr()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = integerLiteral(hasAncestor(bindingDecl(hasName("f";
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = declRefExpr(hasAncestor(bindingDecl(hasName("f";
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
 }
 
 TEST(Traversal, traverseNoImplicit) {
Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -1148,6 +1148,15 @@
 {
   hasDefaultArg(42);
 }
+
+void decomposition()
+{
+int arr[3];
+auto &[f, s, t] = arr;
+
+f = 42;
+}
+
 )cpp",
{"-std=c++20"});
 
@@ -1443,6 +1452,46 @@
 CallExpr
 |-DeclRefExpr 'hasDefaultArg'
 `-IntegerLiteral
+)cpp");
+  }
+
+  {
+auto FN = ast_matchers::match(
+functionDecl(hasName("decomposition"),
+ hasDescendant(decompositionDecl().bind("decomp"))),
+AST2->getASTContext());
+EXPECT_EQ(FN.size(), 1u);
+
+EXPECT_EQ(
+dumpASTString(TK_AsIs, FN[0].getNodeAs("decomp")),
+R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'arr'
+|-BindingDecl 'f'
+| `-ArraySubscriptExpr
+|   |-ImplicitCastExpr
+|   | `-DeclRefExpr ''
+|   `-IntegerLiteral
+|-BindingDecl 's'
+| `-ArraySubscriptExpr
+|   |-ImplicitCastExpr
+|   | `-DeclRefExpr ''
+|   `-IntegerLiteral
+`-BindingDecl 't'
+  `-ArraySubscriptExpr
+|-ImplicitCastExpr
+| `-DeclRefExpr ''
+`-IntegerLiteral
+)cpp");
+
+EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
+FN[0].getNodeAs("decomp")),
+  R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'arr'
+|-BindingDecl 'f'
+|-BindingDecl 's'
+`-BindingDecl 't'
 )cpp");
   }
 }
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1216,6 +1216,8 @@
   ScopedChildren = true;
 if (FD->isTemplateInstantiation())
   ScopedTraversal = true;
+  } else if (isa(DeclNode)) {
+ScopedChildren = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -438,6 +438,8 @@
   }
 
   void VisitBindingDecl(const BindingDecl *D) {
+if (Traversal == TK_IgnoreUnlessSpelledInSource)
+  return;
 if (const auto *E = D->getBinding())
   Visit(E);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D95739: [ASTMatchers] Add matchers for decomposition decls

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision.
steveire added reviewers: aaron.ballman, alexfh.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95739

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2129,6 +2129,28 @@
   EXPECT_TRUE(matchesObjC(ObjCString, objcFinallyStmt()));
 }
 
+TEST(ASTMatchersTest, DecompositionDecl) {
+  StringRef Code = R"cpp(
+void foo()
+{
+int arr[3];
+auto &[f, s, t] = arr;
+
+f = 42;
+}
+  )cpp";
+  EXPECT_TRUE(matchesConditionally(
+  Code, decompositionDecl(hasBinding(0, bindingDecl(hasName("f", true,
+  {"-std=c++17"}));
+
+  EXPECT_TRUE(matchesConditionally(
+  Code,
+  bindingDecl(decl().bind("self"), hasName("f"),
+  forDecomposition(decompositionDecl(
+  hasAnyBinding(bindingDecl(equalsBoundNode("self")),
+  true, {"-std=c++17"}));
+}
+
 TEST(ASTMatchersTestObjC, ObjCAutoreleasePoolStmt) {
   StringRef ObjCString = "void f() {"
  "@autoreleasepool {"
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(binaryOperation);
+  REGISTER_MATCHER(bindingDecl);
   REGISTER_MATCHER(blockDecl);
   REGISTER_MATCHER(blockExpr);
   REGISTER_MATCHER(blockPointerType);
@@ -229,6 +230,7 @@
   REGISTER_MATCHER(exprWithCleanups);
   REGISTER_MATCHER(fieldDecl);
   REGISTER_MATCHER(floatLiteral);
+  REGISTER_MATCHER(forDecomposition);
   REGISTER_MATCHER(forEach);
   REGISTER_MATCHER(forEachArgumentWithParam);
   REGISTER_MATCHER(forEachArgumentWithParamType);
@@ -269,6 +271,8 @@
   REGISTER_MATCHER(hasAttr);
   REGISTER_MATCHER(hasAutomaticStorageDuration);
   REGISTER_MATCHER(hasBase);
+  REGISTER_MATCHER(hasAnyBinding);
+  REGISTER_MATCHER(hasBinding);
   REGISTER_MATCHER(hasBitWidth);
   REGISTER_MATCHER(hasBody);
   REGISTER_MATCHER(hasCanonicalType);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -347,6 +347,16 @@
 extern const internal::VariadicDynCastAllOfMatcher
 decompositionDecl;
 
+/// Matches binding declarations
+/// Example matches \c foo and \c bar
+/// (matcher = bindingDecl()
+///
+/// \code
+///   auto [foo, bar] = std::make_pair{42, 42};
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+bindingDecl;
+
 /// Matches a declaration of a linkage specification.
 ///
 /// Given
@@ -7379,6 +7389,30 @@
 Expr::NPC_ValueDependentIsNull);
 }
 
+/// Matches the DecompositionDecl the binding belongs to.
+AST_MATCHER_P(BindingDecl, forDecomposition, internal::Matcher,
+  InnerMatcher) {
+  if (!Node.getDecomposedDecl())
+return false;
+  return InnerMatcher.matches(*Node.getDecomposedDecl(), Finder, Builder);
+}
+
+/// Matches the Nth binding of a DecompositionDecl.
+AST_MATCHER_P2(DecompositionDecl, hasBinding, unsigned, N,
+   internal::Matcher, InnerMatcher) {
+  if (Node.bindings().size() <= N)
+return false;
+  return InnerMatcher.matches(*Node.bindings()[N], Finder, Builder);
+}
+
+/// Matches any binding of a DecompositionDecl.
+AST_MATCHER_P(DecompositionDecl, hasAnyBinding, internal::Matcher,
+  InnerMatcher) {
+  return llvm::any_of(Node.bindings(), [&](const auto *Binding) {
+return InnerMatcher.matches(*Binding, Finder, Builder);
+  });
+}
+
 /// Matches declaration of the function the statement belongs to
 ///
 /// Given:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D93800: [clangd][WIP] Add caching behaviour for clang-format config

2021-01-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 320301.
njames93 added a comment.

Refactor alot of the implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93800

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/FormatProvider.cpp
  clang-tools-extra/clangd/FormatProvider.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
 #include "index/Index.h"
@@ -60,6 +61,8 @@
   std::vector ExtraArgs;
 
   TidyProvider ClangTidyProvider = {};
+
+  mutable FormatProvider ClangFormatProvider = 
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -9,6 +9,7 @@
 #include "TestTU.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "FormatProvider.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
@@ -61,6 +62,9 @@
   Inputs.Opts = ParseOptions();
   if (ClangTidyProvider)
 Inputs.ClangTidyProvider = ClangTidyProvider;
+  ClangFormatProvider =
+  getClangFormatProvider(FS, format::DefaultFallbackStyle);
+  Inputs.ClangFormatProvider = ClangFormatProvider;
   Inputs.Index = ExternalIndex;
   return Inputs;
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "FormatProvider.h"
 #include "Matchers.h"
 #include "Protocol.h"
 #include "Quality.h"
@@ -154,6 +155,7 @@
   MockFS FS;
   Annotations Test(Text);
   ParseInputs ParseInput{tooling::CompileCommand(), , Test.code().str()};
+  ParseInput.ClangFormatProvider = 
   return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
   Opts);
 }
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -9,6 +9,7 @@
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Features.inc"
+#include "FormatProvider.h"
 #include "PathMapping.h"
 #include "Protocol.h"
 #include "TidyProvider.h"
@@ -860,6 +861,9 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  FormatProvider ClangFormatOptProvider =
+  getClangFormatProvider(TFS, FallbackStyle);
+  Opts.ClangFormatProvider = ClangFormatOptProvider;
   Opts.AsyncPreambleBuilds = AsyncPreamble;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak ) {
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -76,7 +76,7 @@
   // from buildInvocation
   ParseInputs Inputs;
   std::unique_ptr Invocation;
-  format::FormatStyle Style;
+  std::shared_ptr Style;
   // from buildAST
   std::shared_ptr Preamble;
   llvm::Optional AST;
@@ -135,6 +135,7 @@
 return false;
   }
 }
+Inputs.ClangFormatProvider = Opts.ClangFormatProvider;
 log("Parsing command...");
 Invocation =
 buildCompilerInvocation(Inputs, CaptureInvocationDiags, );
@@ -148,7 +149,7 @@
 
 // FIXME: Check that resource-dir/built-in-headers exist?
 
-Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+Style = Inputs.ClangFormatProvider(File, Inputs.Contents);
 
 return true;
   }
@@ -216,7 +217,7 @@
   unsigned Definitions = locateSymbolAt(*AST, Pos, ).size();
   vlog("definition: {0}", Definitions);
 
-  auto Hover = 

[PATCH] D95737: [NFC][Docs] Fix RAVFrontendAction doc's CMakelists.txt for Shared build

2021-01-30 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta created this revision.
xgupta added reviewers: awarzynski, stephenkelly.
xgupta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Example tutorial  giving 
undefine reference error while building with ```BUILD_SHARED_LIBS=ON```.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95737

Files:
  clang/docs/RAVFrontendAction.rst


Index: clang/docs/RAVFrontendAction.rst
===
--- clang/docs/RAVFrontendAction.rst
+++ clang/docs/RAVFrontendAction.rst
@@ -207,7 +207,14 @@
 
 add_clang_executable(find-class-decls FindClassDecls.cpp)
 
-target_link_libraries(find-class-decls clangTooling)
+target_link_libraries(find-class-decls 
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFrontend
+  clangSerialization
+  clangTooling
+  )
 
 When running this tool over a small code snippet it will output all
 declarations of a class n::m::C it found:


Index: clang/docs/RAVFrontendAction.rst
===
--- clang/docs/RAVFrontendAction.rst
+++ clang/docs/RAVFrontendAction.rst
@@ -207,7 +207,14 @@
 
 add_clang_executable(find-class-decls FindClassDecls.cpp)
 
-target_link_libraries(find-class-decls clangTooling)
+target_link_libraries(find-class-decls 
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFrontend
+  clangSerialization
+  clangTooling
+  )
 
 When running this tool over a small code snippet it will output all
 declarations of a class n::m::C it found:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D95655: [AArch64] Adding Neon Sm3 & Sm4 Intrinsics

2021-01-30 Thread Alexandros Lamprineas via Phabricator via cfe-commits
labrinea accepted this revision.
labrinea added a comment.
This revision is now accepted and ready to land.

LGTM, thanks @rsanthir.quic !


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

https://reviews.llvm.org/D95655

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


[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-01-30 Thread Whisperity via Phabricator via cfe-commits
whisperity created this revision.
whisperity added a reviewer: aaron.ballman.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: martong, gamesh411, Szelethus, rnkovacs, xazax.hun.
whisperity requested review of this revision.

The base patch only deals with strict (canonical) type equality, which is 
merely a subset of all the dangerous function interfaces that we intend to 
find. In addition, in the base patch, canonical type equivalence is not 
diagnosed in a way that is immediately apparent to the user.

This patch extends the check with two features:

1. Proper `typedef` diagnostics and explanations to the user.
2. //"Reference bind power"// matching.

**Case 1** will tell the user //the common type of "SomeIntegerType" and 
"SomeOtherIntegerType" is "int"// or //"my_int" and "int" are the same//. This 
makes the (otherwise, through `CanonicalType` desugaring already handled and 
diagnosed, but not //explained//) typedef case obvious to the user without 
having to invoke code comprehension tools.

**Case 2** is a necessary addition because in every case someone encounters a 
function `f(T t, const T& tr)`, any expression that might be passed to either 
can be passed to both. Thus, such adjacent parameter sequences should be 
matched. This is //specifically// modelled for `const&` only, as any other pair 
combination from "value, ref, qualified ref, rref" have //some// broad value 
category in which some actual swap of the arguments at a call would be a 
compiler error. (E.g. `f(int, int&)`  and `f(2, Val)` breaks if swapped, 
similarly how rrefs cannot bind lvalues, etc.) This case **broadens** the 
result set, but only with the most dangerous and "Please think about this and 
refactor..." case.

(Note, that //Case 2// does not model things like `f(int, 
std::reference_wrapper)`.)

Due to the cases above always being a problem (and there isn't a good enough 
reason to say //typedefs should not be diagnosed//, everyone hates the weak 
typedef...), I decided **not** to make matching these configurable.

This patch also serves as a proof-of-concept on how I wish the check to evolve 
over some more subsequent patches, which all aim to introduce user-toggleable 
modelling.

> Originally, the implementation of this feature was part of the very first 
> patch related to this check, D69560  (diff 
> 259320 ). It has been factored out 
> for clarity and to allow more on-point discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -80,20 +80,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 

[PATCH] D95607: Fix traversal with hasDescendant into lambdas

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb57a3422a09: Fix traversal with hasDescendant into lambdas 
(authored by stephenkelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95607

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3220,6 +3220,12 @@
   float i = 42.0;
 }
 
+void func15() {
+  int count = 0;
+  auto l = [&] { ++count; };
+  (void)l;
+}
+
 )cpp";
 
   EXPECT_TRUE(
@@ -3404,6 +3410,15 @@
functionDecl(hasName("func14"), hasDescendant(floatLiteral(,
   langCxx20OrLater()));
 
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   compoundStmt(
+   hasDescendant(varDecl(hasName("count")).bind("countVar")),
+   hasDescendant(
+   
declRefExpr(to(varDecl(equalsBoundNode("countVar"))),
+  langCxx20OrLater()));
+
   Code = R"cpp(
 void foo() {
 int explicit_captured = 0;
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -295,7 +295,7 @@
 if (!match(*Node->getBody()))
   return false;
 
-return true;
+return VisitorBase::TraverseStmt(Node->getBody());
   }
 
   bool shouldVisitTemplateInstantiations() const { return true; }


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3220,6 +3220,12 @@
   float i = 42.0;
 }
 
+void func15() {
+  int count = 0;
+  auto l = [&] { ++count; };
+  (void)l;
+}
+
 )cpp";
 
   EXPECT_TRUE(
@@ -3404,6 +3410,15 @@
functionDecl(hasName("func14"), hasDescendant(floatLiteral(,
   langCxx20OrLater()));
 
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   compoundStmt(
+   hasDescendant(varDecl(hasName("count")).bind("countVar")),
+   hasDescendant(
+   declRefExpr(to(varDecl(equalsBoundNode("countVar"))),
+  langCxx20OrLater()));
+
   Code = R"cpp(
 void foo() {
 int explicit_captured = 0;
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -295,7 +295,7 @@
 if (!match(*Node->getBody()))
   return false;
 
-return true;
+return VisitorBase::TraverseStmt(Node->getBody());
   }
 
   bool shouldVisitTemplateInstantiations() const { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] bb57a34 - Fix traversal with hasDescendant into lambdas

2021-01-30 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-01-30T13:57:41Z
New Revision: bb57a3422a09dcdd572ccb42767a0dabb5f966dd

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

LOG: Fix traversal with hasDescendant into lambdas

Differential Revision: https://reviews.llvm.org/D95607

Added: 


Modified: 
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 89e83ee61574..41be3738e707 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -295,7 +295,7 @@ class MatchChildASTVisitor
 if (!match(*Node->getBody()))
   return false;
 
-return true;
+return VisitorBase::TraverseStmt(Node->getBody());
   }
 
   bool shouldVisitTemplateInstantiations() const { return true; }

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index cbea274cecc9..c67c40ed960a 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3220,6 +3220,12 @@ void func14() {
   float i = 42.0;
 }
 
+void func15() {
+  int count = 0;
+  auto l = [&] { ++count; };
+  (void)l;
+}
+
 )cpp";
 
   EXPECT_TRUE(
@@ -3404,6 +3410,15 @@ void func14() {
functionDecl(hasName("func14"), hasDescendant(floatLiteral(,
   langCxx20OrLater()));
 
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   compoundStmt(
+   hasDescendant(varDecl(hasName("count")).bind("countVar")),
+   hasDescendant(
+   
declRefExpr(to(varDecl(equalsBoundNode("countVar"))),
+  langCxx20OrLater()));
+
   Code = R"cpp(
 void foo() {
 int explicit_captured = 0;



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


[PATCH] D95562: [ASTMatchers] Fix traversal below range-for elements

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG79125085f165: [ASTMatchers] Fix traversal below range-for 
elements (authored by stephenkelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95562

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2820,6 +2820,36 @@
 EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
 
+  Code = R"cpp(
+  struct Range {
+int* begin() const;
+int* end() const;
+  };
+  Range getRange(int);
+
+  void rangeFor()
+  {
+for (auto i : getRange(42))
+{
+}
+  }
+  )cpp";
+  {
+auto M = integerLiteral(equals(42));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = callExpr(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = compoundStmt(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+
   Code = R"cpp(
   void rangeFor()
   {
@@ -2891,6 +2921,40 @@
 matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
  true, {"-std=c++20"}));
   }
+
+  Code = R"cpp(
+  struct Range {
+int* begin() const;
+int* end() const;
+  };
+  Range getRange(int);
+
+  int getNum(int);
+
+  void rangeFor()
+  {
+for (auto j = getNum(42); auto i : getRange(j))
+{
+}
+  }
+  )cpp";
+  {
+auto M = integerLiteral(equals(42));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = compoundStmt(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+
   Code = R"cpp(
 void hasDefaultArg(int i, int j = 0)
 {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -243,10 +243,14 @@
   return true;
 ScopedIncrement ScopedDepth();
 if (auto *Init = Node->getInit())
-  if (!match(*Init))
+  if (!traverse(*Init))
 return false;
-if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
-!match(*Node->getBody()))
+if (!match(*Node->getLoopVariable()))
+  return false;
+if (match(*Node->getRangeInit()))
+  if (!VisitorBase::TraverseStmt(Node->getRangeInit()))
+return false;
+if (!match(*Node->getBody()))
   return false;
 return VisitorBase::TraverseStmt(Node->getBody());
   }
@@ -488,15 +492,21 @@
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue) {
 if (auto *RF = dyn_cast(S)) {
-  for (auto *SubStmt : RF->children()) {
-if (SubStmt == RF->getInit() || SubStmt == RF->getLoopVarStmt() ||
-SubStmt == RF->getRangeInit() || SubStmt == RF->getBody()) {
-  TraverseStmt(SubStmt, Queue);
-} else {
-  ASTNodeNotSpelledInSourceScope RAII(this, true);
-  TraverseStmt(SubStmt, Queue);
+  {
+ASTNodeNotAsIsSourceScope RAII(this, true);
+TraverseStmt(RF->getInit());
+// Don't traverse under the loop variable
+match(*RF->getLoopVariable());
+TraverseStmt(RF->getRangeInit());
+  }
+  {
+ASTNodeNotSpelledInSourceScope RAII(this, true);
+for (auto *SubStmt : RF->children()) {
+  if (SubStmt != RF->getBody())
+TraverseStmt(SubStmt);
 }
   }
+  TraverseStmt(RF->getBody());
   return true;
 } else if (auto *RBO = dyn_cast(S)) {
   {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7912508 - [ASTMatchers] Fix traversal below range-for elements

2021-01-30 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-01-30T13:47:14Z
New Revision: 79125085f16540579d27c7e4987f63eef9c4aa23

URL: 
https://github.com/llvm/llvm-project/commit/79125085f16540579d27c7e4987f63eef9c4aa23
DIFF: 
https://github.com/llvm/llvm-project/commit/79125085f16540579d27c7e4987f63eef9c4aa23.diff

LOG: [ASTMatchers] Fix traversal below range-for elements

Differential Revision: https://reviews.llvm.org/D95562

Added: 


Modified: 
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 5034203840fc..89e83ee61574 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -243,10 +243,14 @@ class MatchChildASTVisitor
   return true;
 ScopedIncrement ScopedDepth();
 if (auto *Init = Node->getInit())
-  if (!match(*Init))
+  if (!traverse(*Init))
 return false;
-if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
-!match(*Node->getBody()))
+if (!match(*Node->getLoopVariable()))
+  return false;
+if (match(*Node->getRangeInit()))
+  if (!VisitorBase::TraverseStmt(Node->getRangeInit()))
+return false;
+if (!match(*Node->getBody()))
   return false;
 return VisitorBase::TraverseStmt(Node->getBody());
   }
@@ -488,15 +492,21 @@ class MatchASTVisitor : public 
RecursiveASTVisitor,
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue) {
 if (auto *RF = dyn_cast(S)) {
-  for (auto *SubStmt : RF->children()) {
-if (SubStmt == RF->getInit() || SubStmt == RF->getLoopVarStmt() ||
-SubStmt == RF->getRangeInit() || SubStmt == RF->getBody()) {
-  TraverseStmt(SubStmt, Queue);
-} else {
-  ASTNodeNotSpelledInSourceScope RAII(this, true);
-  TraverseStmt(SubStmt, Queue);
+  {
+ASTNodeNotAsIsSourceScope RAII(this, true);
+TraverseStmt(RF->getInit());
+// Don't traverse under the loop variable
+match(*RF->getLoopVariable());
+TraverseStmt(RF->getRangeInit());
+  }
+  {
+ASTNodeNotSpelledInSourceScope RAII(this, true);
+for (auto *SubStmt : RF->children()) {
+  if (SubStmt != RF->getBody())
+TraverseStmt(SubStmt);
 }
   }
+  TraverseStmt(RF->getBody());
   return true;
 } else if (auto *RBO = dyn_cast(S)) {
   {

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index a3a09c426673..cbea274cecc9 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2820,6 +2820,36 @@ struct CtorInitsNonTrivial : NonTrivial
 EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
 
+  Code = R"cpp(
+  struct Range {
+int* begin() const;
+int* end() const;
+  };
+  Range getRange(int);
+
+  void rangeFor()
+  {
+for (auto i : getRange(42))
+{
+}
+  }
+  )cpp";
+  {
+auto M = integerLiteral(equals(42));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = callExpr(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = compoundStmt(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+
   Code = R"cpp(
   void rangeFor()
   {
@@ -2891,6 +2921,40 @@ struct CtorInitsNonTrivial : NonTrivial
 matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
  true, {"-std=c++20"}));
   }
+
+  Code = R"cpp(
+  struct Range {
+int* begin() const;
+int* end() const;
+  };
+  Range getRange(int);
+
+  int getNum(int);
+
+  void rangeFor()
+  {
+for (auto j = getNum(42); auto i : getRange(j))
+{
+}
+  }
+  )cpp";
+  {
+auto M = integerLiteral(equals(42));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, 
{"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = compoundStmt(hasDescendant(integerLiteral(equals(42;
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, 
{"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+
  

[PATCH] D95735: [ASTMatchers] Fix matching after generic top-level matcher

2021-01-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision.
steveire added reviewers: aaron.ballman, alexfh.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With a matcher like

  expr(anyOf(integerLiteral(equals(42)), unless(expr(

and code such as

  struct B {
B(int);
  };
  
  B func1() { return 42; }

the top-level expr() would match each of the nodes which are not spelled
in the source and then ignore-traverse to match the integerLiteral node.
This would result in multiple results reported for the integerLiteral.

Fix that by only running matching logic on nodes which are not skipped
with the top-level matcher.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95735

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2519,6 +2519,78 @@
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+
+  Code = R"cpp(
+struct B {
+  B(int);
+};
+
+B func1() { return 42; }
+  )cpp";
+  {
+auto M = expr(ignoringImplicit(integerLiteral(equals(42)).bind("intLit")));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 1)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M = expr(unless(integerLiteral(equals(24.bind("intLit");
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 7)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M =
+expr(anyOf(integerLiteral(equals(42)).bind("intLit"), unless(expr(;
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 1)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M = expr(allOf(integerLiteral(equals(42)).bind("intLit"), expr()));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 1)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M = expr(integerLiteral(equals(42)).bind("intLit"), expr());
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 1)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M = expr(optionally(integerLiteral(equals(42)).bind("intLit")));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("intLit", 1)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("intLit", 1)));
+  }
+  {
+auto M = expr().bind("allExprs");
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_AsIs, M),
+std::make_unique>("allExprs", 7)));
+EXPECT_TRUE(matchAndVerifyResultTrue(
+Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+std::make_unique>("allExprs", 1)));
+  }
 }
 
 TEST(Traversal, traverseNoImplicit) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -837,6 +837,14 @@
   if (EnableCheckProfiling)
 Timer.setBucket([MP.second->getID()]);
   BoundNodesTreeBuilder Builder;
+
+  {
+TraversalKindScope RAII(getASTContext(), MP.first.getTraversalKind());
+if (getASTContext().getParentMapContext().traverseIgnored(DynNode) !=
+DynNode)
+  continue;
+  }
+
   if (MP.first.matches(DynNode, this, )) {
 MatchVisitor Visitor(ActiveASTContext, MP.second);
 Builder.visitMatches();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

2021-01-30 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

First of all, I find this patch to be nearly impossible to read. It seems to 
mix a lot of refactoring with a functional change, making it very hard to focus 
on the core.

The main difference to the jump table logic is that the latter knows that all 
referenced addresses are within a function and therefore well contained. 
Nothing of the like seems to be found here. E.g. if this is supposed to address 
only unnamed pointers, it should be grouping them together and compute the 
offsets and then pick the optimal size. That's a transformation that can be 
beneficial for all modes for a not too large table. But it is hard to see what 
is going on here with all the seemingly unrelated changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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


[PATCH] D95532: [clang][cli] Use variadic macros for parsing/generating

2021-01-30 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

Reverted this, because we're hitting weird preprocessor behavior in MSVC 2017. 
`__VA_ARGS__` form a single preprocessing token that cannot be expanded into 
the original arguments, in our case `PREFIX_TYPE`, `NAME`, `ID`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95532

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


Re: [PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-30 Thread MyDeveloper Day via cfe-commits
I have a script that runs clang-format -n on various directories in clang
that are clang format clean, polly is one of them because they have clang
format as a unit test

I use this to ensure I don’t regress behaviour

Maybe we should formalise this with some sort of clang-format-check cmake
rule

Mydeveloperday

On Sat, 30 Jan 2021 at 10:04, Björn Schäpers via Phabricator <
revi...@reviews.llvm.org> wrote:

> HazardyKnusperkeks added a comment.
>
> In D92257#2532071 , @curdeius
> wrote:
>
> > LGTM. Could you please give us a link to the failing test in Polly? May
> be GitHub or buildbot.
>
> No problem:
>
> http://lab.llvm.org:8011/#builders/10/builds/2294
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D92257/new/
>
> https://reviews.llvm.org/D92257
>
>
___
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-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

My advice leave it out of the release, the next release comes round pretty 
quickly. This gives us 6 months of people who use the snapshots to iron out any 
issues.


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] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-30 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D92257#2532071 , @curdeius wrote:

> LGTM. Could you please give us a link to the failing test in Polly? May be 
> GitHub or buildbot.

No problem:

http://lab.llvm.org:8011/#builders/10/builds/2294


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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


[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.

LGTM. Could you please give us a link to the failing test in Polly? May be 
GitHub or buildbot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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


[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

2021-01-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D94355#2531504 , @gulfem wrote:

> In D94355#2530225 , @lebedev.ri 
> wrote:
>
>> Can you please add an explanation to the patch's description as to why
>> we don't want to instead convert non-relative/relative LUT's elsewhere,
>> please.
>
> @mcgrathr gave some explanation to that:
>
>>> Many backends generate PIC-friendly jump tables. This is about generating 
>>> IR initializers that translate to the same kind of backend assembly 
>>> expressions as those backends use for their jump tables.
>
> I also want to add to that:
> This task specifically tries make switch-to-lookup tables PIC-friendly, but 
> it does not necessarily try to make all the jump tables PIC-friendly.
> There is also another work going on to generate PIC-friendly vtables.
> Therefore, converting non-relative lookup tables to relative tables elsewhere 
> can be explored as a separate task.

Personally, i read that as non-answer,
because this just reiterates that it can be done elsewhere,
and doesn't answer my question as to why that isn't the path taken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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