[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

2021-02-06 Thread David Crook via Phabricator via cfe-commits 
Vexthil updated this revision to Diff 321930.
Vexthil added a comment.

Fixed the update by doing a full diff rather than just the additive update


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

https://reviews.llvm.org/D96147

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-shadow.cpp

Index: clang/test/SemaCXX/warn-shadow.cpp
===
--- clang/test/SemaCXX/warn-shadow.cpp
+++ clang/test/SemaCXX/warn-shadow.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -std=c++11 -Wshadow-all %s
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++17 -Wshadow-all %s
 
 namespace {
   int i; // expected-note {{previous declaration is here}}
@@ -265,3 +265,86 @@
 PR24718_1 // Does not shadow a type.
   };
 };
+
+// MyTuple and std code is copied from live-bindings-test.cpp
+
+//#define USE_STD
+
+#ifndef USE_STD
+// Machinery required for custom structured bindings decomposition.
+typedef unsigned long size_t;
+
+namespace std {
+template  class tuple_size;
+template 
+constexpr size_t tuple_size_v = tuple_size::value;
+template  class tuple_element;
+
+template 
+struct integral_constant {
+  static constexpr T value = v;
+  typedef T value_type;
+  typedef integral_constant type;
+  constexpr operator value_type() const noexcept { return value; }
+};
+} // namespace std
+
+struct MyTuple {
+  int a;
+  int b;
+
+  template 
+  int get() const {
+if constexpr (N == 0)
+  return a;
+else if constexpr (N == 1)
+  return b;
+  }
+};
+
+namespace std {
+template <>
+struct tuple_size
+: std::integral_constant {};
+
+template 
+struct tuple_element {
+  using type = int;
+};
+} // namespace std
+
+MyTuple getMyTuple();
+#else
+
+#include 
+std::tuple getMyTuple();
+#endif
+
+
+namespace structured_binding_tests {
+int x; // expected-note {{previous declaration is here}}
+int y; // expected-note {{previous declaration is here}}
+
+void test1() {
+  const auto [x, y] = getMyTuple(); // expected-warning 2 {{declaration shadows a variable in namespace 'structured_binding_tests'}}
+}
+
+void test2() {
+  int a; // expected-note {{previous declaration is here}}
+  bool b; // expected-note {{previous declaration is here}}
+  {
+auto [a, b] = getMyTuple(); // expected-warning 2 {{declaration shadows a local variable}}
+  }
+}
+
+class A
+{
+  int m_a; // expected-note {{previous declaration is here}}
+  int m_b; // expected-note {{previous declaration is here}}
+
+  void test3() {
+auto [m_a, m_b] = getMyTuple(); // expected-warning 2 {{declaration shadows a field of 'structured_binding_tests::A'}}
+  }
+};
+
+}; // namespace structured_binding_tests
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -857,17 +857,28 @@
   Previous.clear();
 }
 
+auto *BD = BindingDecl::Create(Context, DC, B.NameLoc, B.Name);
+
+// Find the shadowed declaration before filtering for scope.
+NamedDecl *ShadowedDecl = D.getCXXScopeSpec().isEmpty()
+  ? getShadowedDeclaration(BD, Previous)
+  : nullptr;
+
 bool ConsiderLinkage = DC->isFunctionOrMethod() &&
DS.getStorageClassSpec() == DeclSpec::SCS_extern;
 FilterLookupForScope(Previous, DC, S, ConsiderLinkage,
  /*AllowInlineNamespace*/false);
+
+// Diagnose shadowed variables if this isn't a redeclaration.
+if (ShadowedDecl && !D.isRedeclaration())
+  CheckShadow(BD, ShadowedDecl, Previous);
+
 if (!Previous.empty()) {
   auto *Old = Previous.getRepresentativeDecl();
   Diag(B.NameLoc, diag::err_redefinition) << B.Name;
   Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
-auto *BD = BindingDecl::Create(Context, DC, B.NameLoc, B.Name);
 PushOnScopeChains(BD, S, true);
 Bindings.push_back(BD);
 ParsingInitForAutoVars.insert(BD);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7560,6 +7560,19 @@
   return isa(ShadowedDecl) ? ShadowedDecl : nullptr;
 }
 
+/// Return the declaration shadowed by the given variable \p D, or null
+/// if it doesn't shadow any declaration or shadowing warnings are disabled.
+NamedDecl *Sema::getShadowedDeclaration(const BindingDecl *D,
+const LookupResult &R) {
+  if (!shouldWarnIfShadowedDecl(Diags, R))
+return nullptr;
+
+  NamedDecl *ShadowedDecl = R.getFoundDecl();
+  return isa(ShadowedDecl) || isa(ShadowedDecl)
+ ? ShadowedDecl
+ : nullptr;
+}
+
 /// Diagnose variable or built-in function shadowing.

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

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93800

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

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

2021-02-06 Thread Melanie Blower via Phabricator via cfe-commits 
mibintc created this revision.
mibintc added reviewers: Sanitizers, aaron.ballman, echristo, MaskRay.
Herald added subscribers: dexonsmith, dang.
Herald added a reviewer: jansvoboda11.
mibintc requested review of this revision.
Herald added a project: clang.

This changes the option names that include substring blacklist to blocklist. 
Also there are some "resource directory" builtin file names. I changed those 
from blacklist to blocklist. I looked in the doc to see if the builtin file 
names appear in the documentation, I couldn't find hits using e.g. 
"asan_blacklist.txt site:llvm.org" so I cautiously think they aren't documented 
and therefore fair game.

In options.td there is a way to officially mark an option deprecated by adding 
it to a specific group, I didn't do that yet.  When could we actually eliminate 
the old spelling?  How about December 2021?

I thought it would be best to start with the external clang interface, but i 
also want to make more patches to eliminate whitelist and blacklist in the 
comments and in the object names, file names etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96203

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/SanitizerBlacklist.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Basic/SanitizerBlacklist.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt
  clang/test/Driver/Inputs/resource_dir/share/asan_blocklist.txt
  clang/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt
  clang/test/Driver/Inputs/resource_dir/share/hwasan_blocklist.txt
  clang/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt
  clang/test/Driver/Inputs/resource_dir/share/ubsan_blocklist.txt
  clang/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt
  clang/test/Driver/Inputs/resource_dir/share/vtables_blocklist.txt
  clang/test/Driver/fsanitize-blacklist.c
  clang/test/Driver/print-file-name.c
  
clang/test/Frontend/Inputs/resource_dir_with_sanitizer_blacklist/share/ubsan_blacklist.txt
  
clang/test/Frontend/Inputs/resource_dir_with_sanitizer_blacklist/share/ubsan_blocklist.txt
  clang/test/Frontend/dependency-gen.c
  clang/unittests/Driver/SanitizerArgsTest.cpp

Index: clang/unittests/Driver/SanitizerArgsTest.cpp
===
--- clang/unittests/Driver/SanitizerArgsTest.cpp
+++ clang/unittests/Driver/SanitizerArgsTest.cpp
@@ -92,30 +92,30 @@
   std::unique_ptr CompilationJob;
 };
 
-TEST_F(SanitizerArgsTest, Blacklists) {
+TEST_F(SanitizerArgsTest, Blocklists) {
   const std::string ResourceDir = "/opt/llvm/lib/resources";
-  const std::string UserBlacklist = "/source/my_blacklist.txt";
-  const std::string ASanBlacklist =
-  concatPaths({ResourceDir, "share", "asan_blacklist.txt"});
+  const std::string UserBlocklist = "/source/my_blocklist.txt";
+  const std::string ASanBlocklist =
+  concatPaths({ResourceDir, "share", "asan_blocklist.txt"});
 
   auto &Command = emulateSingleCompilation(
   /*ExtraArgs=*/{"-fsanitize=address", "-resource-dir", ResourceDir,
- std::string("-fsanitize-blacklist=") + UserBlacklist},
-  /*ExtraFiles=*/{ASanBlacklist, UserBlacklist});
+ std::string("-fsanitize-blocklist=") + UserBlocklist},
+  /*ExtraFiles=*/{ASanBlocklist, UserBlocklist});
 
-  // System blacklists are added based on resource-dir.
+  // System blocklists are added based on resource-dir.
   EXPECT_THAT(Command.getArguments(),
-  Contains(StrEq(std::string("-fsanitize-system-blacklist=") +
- ASanBlacklist)));
-  // User blacklists should also be added.
+  Contains(StrEq(std::string("-fsanitize-system-blocklist=") +
+ ASanBlocklist)));
+  // User blocklists should also be added.
   EXPECT_THAT(
   Command.getArguments(),
-  Contains(StrEq(std::string("-fsanitize-blacklist=") + UserBlacklist)));
+  Contains(StrEq(std::string("-fsanitize-blocklist=") + UserBlocklist)));
 }
 
 TEST_F(SanitizerArgsTest, XRayLists) {
   const std::string XRayWhitelist = "/source/xray_whitelist.txt";
-  const std::string XRayBlacklist = "/source/xray_blacklist.txt";
+  const std::string XRayBlocklist = "/source/xray_blocklist.txt";
   const std::string XRayAttrList = "/source/xray_attr_list.txt";
 
   auto &Command = emulateSingleCompilation(
@@ -123,17 +123,17 @@
   {
   "-fxray-instrument",
   "-fxray-a

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 created this revision.
njames93 added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, mgorny.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Currently Clang tidy provider searches from the root directory up to the target 
directory, this is the opposite of how clang-tidy searches for config files.
The result of this is .clang-tidy files are ignored in any subdirectory of a 
directory containing a .clang-tidy file.

It'd be nice to get this in the 12 branch as its breaking behaviour introduced 
in that branch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96204

Files:
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/TidyProviderTests.cpp


Index: clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
@@ -0,0 +1,60 @@
+//===-- TidyProviderTests.cpp - Clang tidy configuration provider tests 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestFS.h"
+#include "TidyProvider.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+TEST(TidyProvider, NestedDirectories) {
+  MockFS FS;
+  FS.Files[testPath(".clang-tidy")] = R"yaml(
+  Checks: 'llvm-*'
+  CheckOptions:
+- key: TestKey
+  value: 1
+)yaml";
+  FS.Files[testPath("sub1/.clang-tidy")] = R"yaml(
+  Checks: 'misc-*'
+  CheckOptions:
+- key: TestKey
+  value: 2
+)yaml";
+  FS.Files[testPath("sub1/sub2/.clang-tidy")] = R"yaml(
+  Checks: 'bugprone-*'
+  CheckOptions:
+- key: TestKey
+  value: 3
+  InheritParentConfig: true
+)yaml";
+
+  TidyProvider Provider = provideClangTidyFiles(FS);
+
+  auto BaseOptions = getTidyOptionsForFile(Provider, testPath("File.cpp"));
+  ASSERT_TRUE(BaseOptions.Checks.hasValue());
+  EXPECT_EQ(*BaseOptions.Checks, "llvm-*");
+  EXPECT_EQ(BaseOptions.CheckOptions.lookup("TestKey").Value, "1");
+
+  auto Sub1Options = getTidyOptionsForFile(Provider, 
testPath("sub1/File.cpp"));
+  ASSERT_TRUE(Sub1Options.Checks.hasValue());
+  EXPECT_EQ(*Sub1Options.Checks, "misc-*");
+  EXPECT_EQ(Sub1Options.CheckOptions.lookup("TestKey").Value, "2");
+
+  auto Sub2Options =
+  getTidyOptionsForFile(Provider, testPath("sub1/sub2/File.cpp"));
+  ASSERT_TRUE(Sub2Options.Checks.hasValue());
+  EXPECT_EQ(*Sub2Options.Checks, "misc-*,bugprone-*");
+  EXPECT_EQ(Sub2Options.CheckOptions.lookup("TestKey").Value, "3");
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -93,6 +93,7 @@
   TestIndex.cpp
   TestTU.cpp
   TestWorkspace.cpp
+  TidyProviderTests.cpp
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -106,7 +106,7 @@
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::begin(Parent), E = path::end(Parent); I != E; ++I) {
+  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) 
{
 assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
"Canonical path components should be substrings");
 llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());


Index: clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
@@ -0,0 +1,60 @@
+//===-- TidyProviderTests.cpp - Clang tidy configuration provider tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestFS.h"
+#include "TidyProvider.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+TEST(TidyProvider, NestedDirectories) {
+  MockFS FS;
+  FS.Files[testPath(".clang-tidy")] = R"yaml(
+  Checks: 'llvm-*'
+  CheckOptions:
+- key: TestKey
+  value: 1
+)ya

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 321937.
njames93 added a comment.

Newline at eof.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96204

Files:
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/TidyProviderTests.cpp


Index: clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
@@ -0,0 +1,60 @@
+//===-- TidyProviderTests.cpp - Clang tidy configuration provider tests 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestFS.h"
+#include "TidyProvider.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+TEST(TidyProvider, NestedDirectories) {
+  MockFS FS;
+  FS.Files[testPath(".clang-tidy")] = R"yaml(
+  Checks: 'llvm-*'
+  CheckOptions:
+- key: TestKey
+  value: 1
+)yaml";
+  FS.Files[testPath("sub1/.clang-tidy")] = R"yaml(
+  Checks: 'misc-*'
+  CheckOptions:
+- key: TestKey
+  value: 2
+)yaml";
+  FS.Files[testPath("sub1/sub2/.clang-tidy")] = R"yaml(
+  Checks: 'bugprone-*'
+  CheckOptions:
+- key: TestKey
+  value: 3
+  InheritParentConfig: true
+)yaml";
+
+  TidyProvider Provider = provideClangTidyFiles(FS);
+
+  auto BaseOptions = getTidyOptionsForFile(Provider, testPath("File.cpp"));
+  ASSERT_TRUE(BaseOptions.Checks.hasValue());
+  EXPECT_EQ(*BaseOptions.Checks, "llvm-*");
+  EXPECT_EQ(BaseOptions.CheckOptions.lookup("TestKey").Value, "1");
+
+  auto Sub1Options = getTidyOptionsForFile(Provider, 
testPath("sub1/File.cpp"));
+  ASSERT_TRUE(Sub1Options.Checks.hasValue());
+  EXPECT_EQ(*Sub1Options.Checks, "misc-*");
+  EXPECT_EQ(Sub1Options.CheckOptions.lookup("TestKey").Value, "2");
+
+  auto Sub2Options =
+  getTidyOptionsForFile(Provider, testPath("sub1/sub2/File.cpp"));
+  ASSERT_TRUE(Sub2Options.Checks.hasValue());
+  EXPECT_EQ(*Sub2Options.Checks, "misc-*,bugprone-*");
+  EXPECT_EQ(Sub2Options.CheckOptions.lookup("TestKey").Value, "3");
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -93,6 +93,7 @@
   TestIndex.cpp
   TestTU.cpp
   TestWorkspace.cpp
+  TidyProviderTests.cpp
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -106,7 +106,7 @@
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::begin(Parent), E = path::end(Parent); I != E; ++I) {
+  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) 
{
 assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
"Canonical path components should be substrings");
 llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());


Index: clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
@@ -0,0 +1,60 @@
+//===-- TidyProviderTests.cpp - Clang tidy configuration provider tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestFS.h"
+#include "TidyProvider.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+TEST(TidyProvider, NestedDirectories) {
+  MockFS FS;
+  FS.Files[testPath(".clang-tidy")] = R"yaml(
+  Checks: 'llvm-*'
+  CheckOptions:
+- key: TestKey
+  value: 1
+)yaml";
+  FS.Files[testPath("sub1/.clang-tidy")] = R"yaml(
+  Checks: 'misc-*'
+  CheckOptions:
+- key: TestKey
+  value: 2
+)yaml";
+  FS.Files[testPath("sub1/sub2/.clang-tidy")] = R"yaml(
+  Checks: 'bugprone-*'
+  CheckOptions:
+- key: TestKey
+  value: 3
+  InheritParentConfig: true
+)yaml";
+
+  TidyProvider Provider = provideClangTidyFiles(FS);
+
+  auto BaseOptions = getTidyOptionsForFile(Provider, testPath("File.cpp"));
+  ASSERT_TRUE(BaseOptions.Checks.hasValue());
+  EXPECT_EQ(*BaseOptions.C

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

2021-02-06 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Thank you for working on this!

> This changes the option names that include substring blacklist to blocklist.

I think this change works in some places, but in other places we say things 
like "blocklisted" which feels a bit awkward. I don't super love the name 
`blocklist`, but I don't super hate it either. All the alternative names I come 
up with aren't really great either, like `UnsanitiziedEntities` or 
`IgnoredObjects`. Maybe someone will have a better idea here than me, but I do 
think the current name is an improvement over the old name.

> In options.td there is a way to officially mark an option deprecated by 
> adding it to a specific group, I didn't do that yet. When could we actually 
> eliminate the old spelling? How about December 2021?

I think we want to leave the deprecated options in for a full release cycle, so 
I'd say that (assuming this lands in Clang 13) we could remove support once we 
start work on Clang 14.

> I thought it would be best to start with the external clang interface, but i 
> also want to make more patches to eliminate whitelist and blacklist in the 
> comments and in the object names, file names etc.

Fantastic, thank you!




Comment at: clang/include/clang/AST/ASTContext.h:565
 
   /// Blacklist object that is used by sanitizers to decide which
   /// entities should not be instrumented.

The comment should be updated as well.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:141
 SanitizerMask Mask;
-  } Blacklists[] = {{"asan_blacklist.txt", SanitizerKind::Address},
-{"hwasan_blacklist.txt", SanitizerKind::HWAddress},
-{"memtag_blacklist.txt", SanitizerKind::MemTag},
-{"msan_blacklist.txt", SanitizerKind::Memory},
-{"tsan_blacklist.txt", SanitizerKind::Thread},
+  } Blacklists[] = {{"asan_blocklist.txt", SanitizerKind::Address},
+{"hwasan_blocklist.txt", SanitizerKind::HWAddress},

Do we want to retain support for the old file names and warn about using a 
deprecated name if one is used? This would keep folks working who are relying 
on this (even if it's undocumented).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96203

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

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

I have made an extra test case that compares the output of our provider to the 
output of a `clang::tidy::FileOptionsProvider`, This would ensure coherent 
behaviour between our implementation and clang-tidy implementation.
However I'm not sure it should be included as any changes made on the clang 
tidy side may break that test as we shouldn't be enforcing clang-tidy 
maintainers to also ensure compliance with clangd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96204

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

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

2021-02-06 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

@MaskRay Sounds like this should be reverted (& that revert picked up by the 12 
release branch) & some investigation can continue after that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94735

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

[PATCH] D96209: [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in template paramaters

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added subscribers: nullptr.cpp, xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://bugs.llvm.org/show_bug.cgi?id=49063


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96209

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
@@ -59,6 +59,25 @@
 // CHECK-FIXES: int F13(bool b = true);
 int f13 = F13();
 
+template 
+struct A {};
+
+void F14(const A);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F14(A);
+
+void F15(const A Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F15(A Named);
+
+void F16(const A *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F16(const A *);
+
+void F17(const A *const Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F17(const A *Named);
+
 struct Foo {
   Foo(const int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "AvoidConstParamsInDecls.h"
+#include "../utils/LexerUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
@@ -20,10 +21,8 @@
 namespace {
 
 SourceRange getTypeRange(const ParmVarDecl &Param) {
-  if (Param.getIdentifier() != nullptr)
-return SourceRange(Param.getBeginLoc(),
-   Param.getEndLoc().getLocWithOffset(-1));
-  return Param.getSourceRange();
+  return SourceRange(Param.getBeginLoc(),
+ Param.getLocation().getLocWithOffset(-1));
 }
 
 } // namespace
@@ -46,34 +45,6 @@
   this);
 }
 
-// Re-lex the tokens to get precise location of last 'const'
-static llvm::Optional constTok(CharSourceRange Range,
-  const MatchFinder::MatchResult &Result) {
-  const SourceManager &Sources = *Result.SourceManager;
-  std::pair LocInfo =
-  Sources.getDecomposedLoc(Range.getBegin());
-  StringRef File = Sources.getBufferData(LocInfo.first);
-  const char *TokenBegin = File.data() + LocInfo.second;
-  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first),
- Result.Context->getLangOpts(), File.begin(), TokenBegin,
- File.end());
-  Token Tok;
-  llvm::Optional ConstTok;
-  while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
-  break;
-if (Tok.is(tok::raw_identifier)) {
-  IdentifierInfo &Info = Result.Context->Idents.get(StringRef(
-  Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
-  Tok.setIdentifierInfo(&Info);
-  Tok.setKind(Info.getTokenID());
-}
-if (Tok.is(tok::kw_const))
-  ConstTok = Tok;
-  }
-  return ConstTok;
-}
-
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs("func");
   const auto *Param = Result.Nodes.getNodeAs("param");
@@ -109,7 +80,8 @@
   if (!FileRange.isValid())
 return;
 
-  auto Tok = constTok(FileRange, Result);
+  auto Tok = tidy::utils::lexer::getQualifyingToken(
+  tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
   if (!Tok)
 return;
   Diag << FixItHint::CreateRemoval(


Index: clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
@@ -59,6 +59,25 @@
 // CHECK-FIXES: int F13(bool b = true);
 int f13 = F13();
 
+template 
+struct A {};
+
+void F14(const A);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F14(A);
+
+void F15(const A Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named'

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

2021-02-06 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a subscriber: ABataev.
MaskRay added a comment.

Perhaps we should just drop `llvm/IR/Verifier.cpp:1075` (which is not tested):

  if (N.getTag() == dwarf::DW_TAG_class_type ||
  N.getTag() == dwarf::DW_TAG_union_type) {
AssertDI(N.getFile() && !N.getFile()->getFilename().empty(),
 "class/union requires a filename", &N, N.getFile());
  }

This patch is about a clang synthesized `struct`. 
`clang/lib/CodeGen/CGOpenMPRuntime.cpp:3463` has a synthesized `union` 
(`distinct !DICompositeType(tag: DW_TAG_union_type, name: "kmp_cmplrdata_t", 
size: 64, elements: <0x62b690>)`, which has no filename/line with this patch). 
It does not make sense to attach the current filename/line (which can be 
entirely irrelevant) to the synthesized `union`. @ABataev @jdoerfert ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94735

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

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.
Herald added a subscriber: nullptr.cpp.

In D95771#2540598 , @poelmanc wrote:

> @njames93 Thanks for the review and for accepting this revision. I lack 
> llvm-project commit access so if it's good to go I would greatly appreciate 
> it if you or someone could push this whenever you have have a chance. Thanks!

Should this be committed using `poelmanc `?


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

https://reviews.llvm.org/D95771

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

[PATCH] D96044: DebugInfo: Emit "LocalToUnit" flag on local member function decls.

2021-02-06 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Sounds good. Out of curiosity, how'd you come across this/do you have any 
particular use case that uses DW_AT_external?




Comment at: clang/test/CodeGenCXX/debug-info-class.cpp:27-34
+namespace {
 struct D {
-  D();
-  virtual ~D();
+  D() {}
+  virtual ~D() {}
   void func() {
   }
 };

Might be better to add a separate class to test this case - modifying this 
class may reduce coverage of existing features (the vtable homing (well, lack 
of homing due to `~D` being virtual and inline) would no longer be  tested 
here, since the type is internal and that'd override the other logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96044

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

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

2021-02-06 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a reviewer: klimek.
njames93 added a comment.

I have no issues here, but see what the others say as well.
Also related post to cfe-dev - 
https://lists.llvm.org/pipermail/cfe-dev/2021-February/067629.html




Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:148-151
+  for (auto *P : FD->parameters())
+if (!TraverseDecl(P)) {
+  return false;
+}

nit: Inner If doesn't need braces, but I'd prefer the loop to have them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96114

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

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

2021-02-06 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc added a comment.

In D95771#2547102 , @njames93 wrote:

> Should this be committed using `poelmanc `?

That or `Conrad Poelman ` would be great, thanks.


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

https://reviews.llvm.org/D95771

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

LLVM buildmaster will be restarted tonight

2021-02-06 Thread Galina Kistanova via cfe-commits 
 Hello everyone,

LLVM buildmaster will be restarted after 7PM PST today.

Thanks

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

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

2021-02-06 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc added a comment.

On 2 Feb Harbormaster found a bug  from my 
switching some char* code to use StringRef. I uploaded a new patch on 4 Feb, 
but Harbormaster does not appear to have rerun. What triggers Harbormaster - do 
I need to take some action? I haven't been able to find such options myself.

Please holler if there's a better place to ask this, thanks!


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

https://reviews.llvm.org/D68682

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

[PATCH] D96215: [clang-tidy] Recognize captures as a form of aliasing.

2021-02-06 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr2, aaron.ballman, vsavchenko.
Herald added subscribers: nullptr.cpp, martong, mgehre, Charusso, xazax.hun.
NoQ requested review of this revision.

The utility function `clang::tidy::utils::`()` scans the function for 
pointer/reference aliases to a given variable. It currently scans for operator 
`&` over that variable and for declarations of references to that variable.

I'm arguing that it should scan for lambda captures by reference as well. If a 
lambda captures a variable by reference it basically has a reference to that 
variable that the user of the lambda can access at any time, including on 
background thread, which may have meaningful impact on checkers. The same 
applies to blocks (an Apple extension that brings closures to C and 
Objective-C; blocks are also implicitly convertible with lambdas when both 
facilities are available).

The patch fixes false positives in both checkers that use this functionality: 
`bugprone-infinite-loop` and `bugprone-redundant-branch-condition`. There are a 
few false negatives it introduces as a tradeoff: if the lambda captures a 
variable by reference but never actually mutates it we'll no longer warn but we 
should (as demonstrated by FIXME tests). These false negatives are 
architecturally annoying to deal with because `hasPtrOrReferenceInFunc()` 
detects aliasing rather than mutation, so it'll have to be a different facility 
entirely. They're also rudimentary because there's rarely a good reason to 
write such code; it probably deserves a separate warning to relax capture kind 
to a by-value or by-const-reference capture (which is often explicit). That 
said, C++'s `[&]` would probably capture a lot of stuff by reference 
unnecessarily and it'll often make their code worse if developers are forced to 
write down all captures manually instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D96215

Files:
  clang-tools-extra/clang-tidy/utils/Aliasing.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -1179,6 +1179,86 @@
   }
 }
 
+// Lambda / block captures.
+
+template  void accept_callback(T t) {
+  // Potentially call the callback.
+  // Possibly on a background thread or something.
+}
+
+void accept_block(void (^)(void)) {
+  // Potentially call the callback.
+  // Possibly on a background thread or something.
+}
+
+void wait(void) {
+  // Wait for the previously passed callback to be called.
+}
+
+void capture_and_mutate_by_lambda() {
+  bool x = true;
+  accept_callback([&]() { x = false; });
+  if (x) {
+wait();
+if (x) {
+}
+  }
+}
+
+void lambda_capture_by_value() {
+  bool x = true;
+  accept_callback([x]() { if (x) {} });
+  if (x) {
+wait();
+if (x) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'x' [bugprone-redundant-branch-condition]
+}
+  }
+}
+
+void capture_by_lambda_but_not_mutate() {
+  bool x = true;
+  accept_callback([&]() { if (x) {} });
+  if (x) {
+wait();
+// FIXME: Should warn.
+if (x) {
+}
+  }
+}
+
+void capture_and_mutate_by_block() {
+  __block bool x = true;
+  accept_block(^{ x = false; });
+  if (x) {
+wait();
+if (x) {
+}
+  }
+}
+
+void block_capture_by_value() {
+  bool x = true;
+  accept_block(^{ if (x) {} });
+  if (x) {
+wait();
+if (x) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'x' [bugprone-redundant-branch-condition]
+}
+  }
+}
+
+void capture_by_block_but_not_mutate() {
+  __block bool x = true;
+  accept_callback(^{ if (x) {} });
+  if (x) {
+wait();
+// FIXME: Should warn.
+if (x) {
+}
+  }
+}
+
 // GNU Expression Statements
 
 void negative_gnu_expression_statement() {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- \
+// RUN: -fexceptions -fblocks
 
 void simple_infinite_loop1() {
   int i = 0;
@@ -378,6 +379,84 @@
   } while (i < Limit);
 }
 
+template  void accept_callback(T t) {
+  // Potentially call the callback.
+  // Possibly on a background thread or something.
+}
+
+void accept_block(void (^)(void)) {
+  // Potentially call the callback.
+  // Possibly on

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

2021-02-06 Thread Michael Kruse via Phabricator via cfe-commits 
Meinersbur added inline comments.



Comment at: clang/include/clang/AST/StmtOpenMP.h:41
+/// purposes:
+/// * Loop variable: The user-accessible variable with different value for each
+///   iteration.

jdenny wrote:
> I suggest the terms "loop user variable" and "loop iteration variable".  In 
> particular, I find "loop counter" to be misleading because "counter" suggests 
> to me it's always an integer, so it's easy to get it confused with the 
> "logical iteration counter".  If you're using standard terminology that I'm 
> not aware of, then never mind.
Good suggestion



Comment at: clang/include/clang/AST/StmtOpenMP.h:92
+/// 
+///   [&,__begin](std::vector::iterator &Result, size_t Logical) {
+///   Result = __begin + Logical; }

jdenny wrote:
> Why is `__begin` an explicit capture here but not for the distance function?
Because `__begin` is captured by-value, everything lese uses the `&`default 
capture. This is the loop counter variable is modified inside the loop body.



Comment at: clang/include/clang/AST/StmtOpenMP.h:101
+  enum {
+LOOPY_STMT,
+DISTANCE_FUNC,

jdenny wrote:
> Why "loopy" with a "y"?
`loopy` was meant to refer to the property of the statement (ForStmt, 
CXXForRangeStmt, potentially others such as WhileStmt, DoStmt, functions from 
`#include `, etc) instead of a specific loop node (such as 
OMPLoopDirective or OMPCanonicalLoop itself), although I did not apply this 
idea consistently. Do you prefer a plain 'LOOP_STMT'?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

2021-02-06 Thread Pengfei Wang via Phabricator via cfe-commits 
pengfei added a comment.

In D93179#2535702 , @pengfei wrote:

> Hi @RKSimon, what's the status of updating these reduce intrinsics? Is there 
> any difficulty for always assigning them fast math flag? I received bug 
> report for the previous change D92940 . Can 
> we revert it if the problem is not easy to fix?

Any thoughts @RKSimon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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