[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Looks like there's only an `llc` test, but the option is in clang too? Should 
we have a test there?

Is there any documentation for clang this option needs adding to? Also, this 
likely deserves a release note, I feel.




Comment at: clang/include/clang/Driver/Options.td:3307-3310
+def fbinutils_version_EQ : Joined<["-"], "fbinutils-version=">, Group,
+  HelpText<"Produced object files can use ELF features only supported by ld 
newer than the specified version. If"
+  "-fno-integrated-as is specified, produced assembly can use newer ELF 
features. "
+  "'future' means that features not implemented by any known release can be 
used">;

It might be helpful to indicate what the default is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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

[PATCH] D77598: Integral template argument suffix and cast printing

[Pratyush Das via Phabricator via cfe-commits]

reikdas updated this revision to Diff 285565.
reikdas added a comment.

Made some changes to SemaTemplate.cpp based on 
http://clang-developers.42468.n3.nabble.com/Using-clang-tool-to-Exact-type-names-in-template-specification-arguments-td4069210.html


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

https://reviews.llvm.org/D77598

Files:
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/temp_arg_nontype.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -459,3 +459,13 @@
   X y;
   int n = y.call(); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
 }
+
+namespace PR9227 {
+  template  struct S {};
+  template <> struct S<1> { using type = int; }; // expected-note {{'S<1>::type' declared here}}
+  S<1L>::type t; // expected-error {{no type named 'type' in 'PR9227::S<1L>'; did you mean 'S<1>::type'?}}
+
+  template  struct A {};
+  template <> struct A<1> { using type = int; }; // expected-note {{'A<1>::type' declared here}}
+  A<2>::type x; // expected-error {{no type named 'type' in 'PR9227::A<2>'; did you mean 'A<1>::type'?}}
+}
Index: clang/test/SemaTemplate/temp_arg_nontype.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype.cpp
@@ -270,6 +270,23 @@
   void test_char_possibly_negative() { enable_if_char<'\x02'>::type i; } // expected-error{{enable_if_char<'\x02'>'; did you mean 'enable_if_char<'a'>::type'?}}
   void test_char_single_quote() { enable_if_char<'\''>::type i; } // expected-error{{enable_if_char<'\''>'; did you mean 'enable_if_char<'a'>::type'?}}
   void test_char_backslash() { enable_if_char<'\\'>::type i; } // expected-error{{enable_if_char<'\\'>'; did you mean 'enable_if_char<'a'>::type'?}}
+
+  template  struct enable_if_int {};
+  template <> struct enable_if_int<1> { typedef int type; }; // expected-note{{'enable_if_int<1>::type' declared here}}
+  void test_int() { enable_if_int<2>::type i; } // expected-error{{enable_if_int<2>'; did you mean 'enable_if_int<1>::type'?}}
+
+  template  struct enable_if_unsigned_int {};
+  template <> struct enable_if_unsigned_int<1> { typedef int type; }; // expected-note{{'enable_if_unsigned_int<1>::type' declared here}}
+  void test_unsigned_int() { enable_if_unsigned_int<2>::type i; } // expected-error{{enable_if_unsigned_int<2>'; did you mean 'enable_if_unsigned_int<1>::type'?}}
+
+  template  struct enable_if_unsigned_long_long {};
+  template <> struct enable_if_unsigned_long_long<1> { typedef int type; }; // expected-note{{'enable_if_unsigned_long_long<1>::type' declared here}}
+  void test_unsigned_long_long() { enable_if_unsigned_long_long<2>::type i; } // expected-error{{enable_if_unsigned_long_long<2>'; did you mean 'enable_if_unsigned_long_long<1>::type'?}}
+
+  template  struct enable_if_long_long {};
+  template <> struct enable_if_long_long<1> { typedef int type; }; // expected-note{{'enable_if_long_long<1>::type' declared here}}
+  void test_long_long() { enable_if_long_long<2>::type i; } // expected-error{{enable_if_long_long<2>'; did you mean 'enable_if_long_long<1>::type'?}}
+
 }
 
 namespace PR10579 {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6822,6 +6822,14 @@
 
 QualType CanonParamType = Context.getCanonicalType(ParamType);
 
+// Note: this renders CanonParamType non-canonical, but since every
+// instantiation of this argument will be wrapped in an AutoType (since
+// Param->getType() will always be an AutoType for this template), there
+// should be no difference in how arguments are distinguished.
+if (Param->getType()->getAs())
+  CanonParamType = Context.getAutoType(CanonParamType,
+   AutoTypeKeyword::Auto, false, false);
+
 // Convert the APValue to a TemplateArgument.
 switch (Value.getKind()) {
 case APValue::None:
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -70,13 +70,51 @@
 
   if (T->isBooleanType() && !Policy.MSVCFormatting) {
 Out << (Val.getBoolValue() ? "true" : "false");
+  } else if (T->isBooleanType()) {
+Out << Val;
   } else if (T->isCharType()) {
 const char Ch = Val.getZExtValue();
 Out << ((Ch == '\'') ? "'\\" : "'");
 Out.write_escaped(StringRef(&Ch, 1), /*UseHexEscapes=*/ true);
 Out << "'";
   } else {
-Out << Val;
+bool flag = false;
+if (auto *autoT = T->g

[PATCH] D85913: [SyntaxTree] Split `TreeTestBase` into header and source

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 285566.
eduucaldas added a comment.

remove static inside anonymous namespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85913

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/MutationsTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -1,4 +1,4 @@
-//===- TreeTestBase.cpp ---===//
+//===- TreeTestBase.h -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -10,184 +10,33 @@
 //
 //===--===//
 
-#include "clang/AST/ASTConsumer.h"
+#ifndef LLVM_CLANG_UNITTESTS_TOOLING_SYNTAX_TREETESTBASE_H
+#define LLVM_CLANG_UNITTESTS_TOOLING_SYNTAX_TREETESTBASE_H
+
 #include "clang/Basic/LLVM.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendAction.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
-#include "clang/Lex/PreprocessorOptions.h"
-#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Testing/TestClangConfig.h"
-#include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Nodes.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "clang/Tooling/Syntax/Tree.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace syntax {
-static ArrayRef tokens(syntax::Node *N) {
-  assert(N->isOriginal() && "tokens of modified nodes are not well-defined");
-  if (auto *L = dyn_cast(N))
-return llvm::makeArrayRef(L->token(), 1);
-  auto *T = cast(N);
-  return llvm::makeArrayRef(T->firstLeaf()->token(),
-T->lastLeaf()->token() + 1);
-}
-
 class SyntaxTreeTest : public ::testing::Test,
public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *buildTree(StringRef Code,
- const TestClangConfig &ClangConfig) {
-// FIXME: this code is almost the identical to the one in TokensTest. Share
-//it.
-class BuildSyntaxTree : public ASTConsumer {
-public:
-  BuildSyntaxTree(syntax::TranslationUnit *&Root,
-  std::unique_ptr &TB,
-  std::unique_ptr &Arena,
-  std::unique_ptr Tokens)
-  : Root(Root), TB(TB), Arena(Arena), Tokens(std::move(Tokens)) {
-assert(this->Tokens);
-  }
-
-  void HandleTranslationUnit(ASTContext &Ctx) override {
-TB =
-std::make_unique(std::move(*Tokens).consume());
-Tokens = nullptr; // make sure we fail if this gets called twice.
-Arena = std::make_unique(Ctx.getSourceManager(),
-Ctx.getLangOpts(), *TB);
-Root = syntax::buildSyntaxTree(*Arena, *Ctx.getTranslationUnitDecl());
-  }
-
-private:
-  syntax::TranslationUnit *&Root;
-  std::unique_ptr &TB;
-  std::unique_ptr &Arena;
-  std::unique_ptr Tokens;
-};
-
-class BuildSyntaxTreeAction : public ASTFrontendAction {
-public:
-  BuildSyntaxTreeAction(syntax::TranslationUnit *&Root,
-std::unique_ptr &TB,
-std::unique_ptr &Arena)
-  : Root(Root), TB(TB), Arena(Arena) {}
-
-  std::unique_ptr
-  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
-// We start recording the tokens, ast consumer will take on the result.
-auto Tokens =
-std::make_unique(CI.getPreprocessor());
-return std::make_unique(Root, TB, Arena,
- std::move(Tokens));
-  }
-
-private:
-  syntax::TranslationUnit *&Root;
-  std::unique_ptr &TB;
-  std::unique_ptr &Arena;
-};
-
-constexpr const char *FileName = "./input.cpp";
-FS->addFile(FileName, time_t(), llvm::MemoryBuffer::getMemBufferCopy(""));
-
-if (!Diags->getClient())
-  Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), DiagOpts.get()));
-Diags->setSeverityForGroup(diag::Flavor::WarningOrError, "unused-value",
-   diag::Severity::Ignored, SourceLocation());
+  Transl

[PATCH] D77229: [Analyzer] Avoid handling of LazyCompundVals in IteratorModeling

[Endre Fülöp via Phabricator via cfe-commits]

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

Thanks! LGTM now.


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

https://reviews.llvm.org/D77229

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

[clang] 2e4a20f - [SyntaxTree] Split `TreeTestBase` into header and source

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-08-14T07:29:07Z
New Revision: 2e4a20fd7062f65c06b438953de3d340df00b7a7

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

LOG: [SyntaxTree] Split `TreeTestBase` into header and source

* Switch to using directive on source files.
* Remove unused `SyntaxTreeTest::addFile`

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

Added: 
clang/unittests/Tooling/Syntax/TreeTestBase.cpp

Modified: 
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
clang/unittests/Tooling/Syntax/CMakeLists.txt
clang/unittests/Tooling/Syntax/MutationsTest.cpp
clang/unittests/Tooling/Syntax/TreeTestBase.h

Removed: 




diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index deffd48da085..211e8b1ae901 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -12,8 +12,9 @@
 
 #include "TreeTestBase.h"
 
-namespace clang {
-namespace syntax {
+using namespace clang;
+using namespace clang::syntax;
+
 namespace {
 
 TEST_P(SyntaxTreeTest, Simple) {
@@ -4836,26 +4837,4 @@ void x(char a, short (*b)(int), long (**c)(long long));
 )txt"));
 }
 
-static std::vector allTestClangConfigs() {
-  std::vector all_configs;
-  for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
-Lang_CXX14, Lang_CXX17, Lang_CXX20}) {
-TestClangConfig config;
-config.Language = lang;
-config.Target = "x86_64-pc-linux-gnu";
-all_configs.push_back(config);
-
-// Windows target is interesting to test because it enables
-// `-fdelayed-template-parsing`.
-config.Target = "x86_64-pc-win32-msvc";
-all_configs.push_back(config);
-  }
-  return all_configs;
-}
-
-INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(allTestClangConfigs()), );
-
 } // namespace
-} // namespace syntax
-} // namespace clang

diff  --git a/clang/unittests/Tooling/Syntax/CMakeLists.txt 
b/clang/unittests/Tooling/Syntax/CMakeLists.txt
index c7634798fb2b..46ff4c9c3e27 100644
--- a/clang/unittests/Tooling/Syntax/CMakeLists.txt
+++ b/clang/unittests/Tooling/Syntax/CMakeLists.txt
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(SyntaxTests
+  TreeTestBase.cpp
   BuildTreeTest.cpp
   MutationsTest.cpp
   TokensTest.cpp

diff  --git a/clang/unittests/Tooling/Syntax/MutationsTest.cpp 
b/clang/unittests/Tooling/Syntax/MutationsTest.cpp
index 088dbe3afbdc..6ef71e3a8090 100644
--- a/clang/unittests/Tooling/Syntax/MutationsTest.cpp
+++ b/clang/unittests/Tooling/Syntax/MutationsTest.cpp
@@ -12,9 +12,11 @@
 
 #include "clang/Tooling/Syntax/Mutations.h"
 #include "TreeTestBase.h"
+#include "clang/Tooling/Syntax/BuildTree.h"
+
+using namespace clang;
+using namespace clang::syntax;
 
-namespace clang {
-namespace syntax {
 namespace {
 
 TEST_P(SyntaxTreeTest, Mutations) {
@@ -81,5 +83,3 @@ TEST_P(SyntaxTreeTest, SynthesizedNodes) {
   EXPECT_TRUE(S->isDetached());
 }
 } // namespace
-} // namespace syntax
-} // namespace clang

diff  --git a/clang/unittests/Tooling/Syntax/TreeTestBase.cpp 
b/clang/unittests/Tooling/Syntax/TreeTestBase.cpp
new file mode 100644
index ..6d2efeaaa8eb
--- /dev/null
+++ b/clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -0,0 +1,200 @@
+//===- TreeTestBase.cpp 
---===//
+//
+// 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
+//
+//===--===//
+//
+// This file provides the test infrastructure for syntax trees.
+//
+//===--===//
+
+#include "TreeTestBase.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Testing/TestClangConfig.h"
+#include "clang/Tooling/Syntax/BuildTree.h"
+#include "clang/Tooling/Syntax/Nodes.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/Tooling/Syntax/Tree.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::syntax;
+
+namespace {
+ArrayRef tokens(syntax::N

[PATCH] D85913: [SyntaxTree] Split `TreeTestBase` into header and source

[Eduardo Caldas 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 rG2e4a20fd7062: [SyntaxTree] Split `TreeTestBase` into header 
and source (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85913

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/MutationsTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -1,4 +1,4 @@
-//===- TreeTestBase.cpp ---===//
+//===- TreeTestBase.h -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -10,184 +10,33 @@
 //
 //===--===//
 
-#include "clang/AST/ASTConsumer.h"
+#ifndef LLVM_CLANG_UNITTESTS_TOOLING_SYNTAX_TREETESTBASE_H
+#define LLVM_CLANG_UNITTESTS_TOOLING_SYNTAX_TREETESTBASE_H
+
 #include "clang/Basic/LLVM.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendAction.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
-#include "clang/Lex/PreprocessorOptions.h"
-#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Testing/TestClangConfig.h"
-#include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Nodes.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "clang/Tooling/Syntax/Tree.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace syntax {
-static ArrayRef tokens(syntax::Node *N) {
-  assert(N->isOriginal() && "tokens of modified nodes are not well-defined");
-  if (auto *L = dyn_cast(N))
-return llvm::makeArrayRef(L->token(), 1);
-  auto *T = cast(N);
-  return llvm::makeArrayRef(T->firstLeaf()->token(),
-T->lastLeaf()->token() + 1);
-}
-
 class SyntaxTreeTest : public ::testing::Test,
public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *buildTree(StringRef Code,
- const TestClangConfig &ClangConfig) {
-// FIXME: this code is almost the identical to the one in TokensTest. Share
-//it.
-class BuildSyntaxTree : public ASTConsumer {
-public:
-  BuildSyntaxTree(syntax::TranslationUnit *&Root,
-  std::unique_ptr &TB,
-  std::unique_ptr &Arena,
-  std::unique_ptr Tokens)
-  : Root(Root), TB(TB), Arena(Arena), Tokens(std::move(Tokens)) {
-assert(this->Tokens);
-  }
-
-  void HandleTranslationUnit(ASTContext &Ctx) override {
-TB =
-std::make_unique(std::move(*Tokens).consume());
-Tokens = nullptr; // make sure we fail if this gets called twice.
-Arena = std::make_unique(Ctx.getSourceManager(),
-Ctx.getLangOpts(), *TB);
-Root = syntax::buildSyntaxTree(*Arena, *Ctx.getTranslationUnitDecl());
-  }
-
-private:
-  syntax::TranslationUnit *&Root;
-  std::unique_ptr &TB;
-  std::unique_ptr &Arena;
-  std::unique_ptr Tokens;
-};
-
-class BuildSyntaxTreeAction : public ASTFrontendAction {
-public:
-  BuildSyntaxTreeAction(syntax::TranslationUnit *&Root,
-std::unique_ptr &TB,
-std::unique_ptr &Arena)
-  : Root(Root), TB(TB), Arena(Arena) {}
-
-  std::unique_ptr
-  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
-// We start recording the tokens, ast consumer will take on the result.
-auto Tokens =
-std::make_unique(CI.getPreprocessor());
-return std::make_unique(Root, TB, Arena,
- std::move(Tokens));
-  }
-
-private:
-  syntax::TranslationUnit *&Root;
-  std::unique_ptr &TB;
-  std::unique_ptr &Arena;
-};
-
-constexpr const char *FileName = "./input.cpp";
-FS->addFile(FileName, time_t(), llvm::MemoryBuffer::getMemBufferCopy(""));
-
-if (!Diags->getClient())
-  Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), DiagOpts.get()));
-Diags->setSeverityForGroup(diag

[PATCH] D85573: [CGAtomic] Mark atomic libcall functions `nounwind`

[Gui Andrade via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG909a851dbffe: [CGAtomic] Mark atomic libcall functions 
`nounwind` (authored by guiand).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85573

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomic_ops.c


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -26,6 +26,9 @@
 
 }
 
+// LIBCALL: declare void @__atomic_load(i32, i8*, i8*, i32) 
[[LC_ATTRS:#[0-9]+]]
+// LIBCALL: declare i1 @__atomic_compare_exchange(i32, i8*, i8*, i8*, i32, 
i32) [[LC_ATTRS:#[0-9]+]]
+
 extern _Atomic _Bool b;
 
 _Bool bar() {
@@ -53,6 +56,8 @@
   x += y;
 }
 
+// LIBCALL: declare void @__atomic_store(i32, i8*, i8*, i32) 
[[LC_ATTRS:#[0-9]+]]
+
 _Atomic(int) compound_add(_Atomic(int) in) {
 // CHECK-LABEL: @compound_add
 // CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
@@ -107,3 +112,5 @@
 
   return (in *= 5);
 }
+
+// LIBCALL: [[LC_ATTRS]] = { nounwind willreturn }
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -307,7 +307,14 @@
   const CGFunctionInfo &fnInfo =
 CGF.CGM.getTypes().arrangeBuiltinFunctionCall(resultType, args);
   llvm::FunctionType *fnTy = CGF.CGM.getTypes().GetFunctionType(fnInfo);
-  llvm::FunctionCallee fn = CGF.CGM.CreateRuntimeFunction(fnTy, fnName);
+  llvm::AttrBuilder fnAttrB;
+  fnAttrB.addAttribute(llvm::Attribute::NoUnwind);
+  fnAttrB.addAttribute(llvm::Attribute::WillReturn);
+  llvm::AttributeList fnAttrs = llvm::AttributeList::get(
+  CGF.getLLVMContext(), llvm::AttributeList::FunctionIndex, fnAttrB);
+
+  llvm::FunctionCallee fn =
+  CGF.CGM.CreateRuntimeFunction(fnTy, fnName, fnAttrs);
   auto callee = CGCallee::forDirect(fn);
   return CGF.EmitCall(fnInfo, callee, ReturnValueSlot(), args);
 }


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -26,6 +26,9 @@
 
 }
 
+// LIBCALL: declare void @__atomic_load(i32, i8*, i8*, i32) [[LC_ATTRS:#[0-9]+]]
+// LIBCALL: declare i1 @__atomic_compare_exchange(i32, i8*, i8*, i8*, i32, i32) [[LC_ATTRS:#[0-9]+]]
+
 extern _Atomic _Bool b;
 
 _Bool bar() {
@@ -53,6 +56,8 @@
   x += y;
 }
 
+// LIBCALL: declare void @__atomic_store(i32, i8*, i8*, i32) [[LC_ATTRS:#[0-9]+]]
+
 _Atomic(int) compound_add(_Atomic(int) in) {
 // CHECK-LABEL: @compound_add
 // CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
@@ -107,3 +112,5 @@
 
   return (in *= 5);
 }
+
+// LIBCALL: [[LC_ATTRS]] = { nounwind willreturn }
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -307,7 +307,14 @@
   const CGFunctionInfo &fnInfo =
 CGF.CGM.getTypes().arrangeBuiltinFunctionCall(resultType, args);
   llvm::FunctionType *fnTy = CGF.CGM.getTypes().GetFunctionType(fnInfo);
-  llvm::FunctionCallee fn = CGF.CGM.CreateRuntimeFunction(fnTy, fnName);
+  llvm::AttrBuilder fnAttrB;
+  fnAttrB.addAttribute(llvm::Attribute::NoUnwind);
+  fnAttrB.addAttribute(llvm::Attribute::WillReturn);
+  llvm::AttributeList fnAttrs = llvm::AttributeList::get(
+  CGF.getLLVMContext(), llvm::AttributeList::FunctionIndex, fnAttrB);
+
+  llvm::FunctionCallee fn =
+  CGF.CGM.CreateRuntimeFunction(fnTy, fnName, fnAttrs);
   auto callee = CGCallee::forDirect(fn);
   return CGF.EmitCall(fnInfo, callee, ReturnValueSlot(), args);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 909a851 - [CGAtomic] Mark atomic libcall functions `nounwind`

[Gui Andrade via cfe-commits]

Author: Gui Andrade
Date: 2020-08-14T07:46:43Z
New Revision: 909a851dbffeb3637c19268e12e10fdf3dae2add

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

LOG: [CGAtomic] Mark atomic libcall functions `nounwind`

These functions won't ever unwind. This is useful for MemorySanitizer
as it simplifies handling __atomic_load in particular.

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

Added: 


Modified: 
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic_ops.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index b7ada4ac7e3b..d7720a23dd72 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -307,7 +307,14 @@ static RValue emitAtomicLibcall(CodeGenFunction &CGF,
   const CGFunctionInfo &fnInfo =
 CGF.CGM.getTypes().arrangeBuiltinFunctionCall(resultType, args);
   llvm::FunctionType *fnTy = CGF.CGM.getTypes().GetFunctionType(fnInfo);
-  llvm::FunctionCallee fn = CGF.CGM.CreateRuntimeFunction(fnTy, fnName);
+  llvm::AttrBuilder fnAttrB;
+  fnAttrB.addAttribute(llvm::Attribute::NoUnwind);
+  fnAttrB.addAttribute(llvm::Attribute::WillReturn);
+  llvm::AttributeList fnAttrs = llvm::AttributeList::get(
+  CGF.getLLVMContext(), llvm::AttributeList::FunctionIndex, fnAttrB);
+
+  llvm::FunctionCallee fn =
+  CGF.CGM.CreateRuntimeFunction(fnTy, fnName, fnAttrs);
   auto callee = CGCallee::forDirect(fn);
   return CGF.EmitCall(fnInfo, callee, ReturnValueSlot(), args);
 }

diff  --git a/clang/test/CodeGen/atomic_ops.c b/clang/test/CodeGen/atomic_ops.c
index c1eb1d005dba..79a1e5dba780 100644
--- a/clang/test/CodeGen/atomic_ops.c
+++ b/clang/test/CodeGen/atomic_ops.c
@@ -26,6 +26,9 @@ void foo(int x)
 
 }
 
+// LIBCALL: declare void @__atomic_load(i32, i8*, i8*, i32) 
[[LC_ATTRS:#[0-9]+]]
+// LIBCALL: declare i1 @__atomic_compare_exchange(i32, i8*, i8*, i8*, i32, 
i32) [[LC_ATTRS:#[0-9]+]]
+
 extern _Atomic _Bool b;
 
 _Bool bar() {
@@ -53,6 +56,8 @@ void baz(int y) {
   x += y;
 }
 
+// LIBCALL: declare void @__atomic_store(i32, i8*, i8*, i32) 
[[LC_ATTRS:#[0-9]+]]
+
 _Atomic(int) compound_add(_Atomic(int) in) {
 // CHECK-LABEL: @compound_add
 // CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
@@ -107,3 +112,5 @@ _Atomic(int) compound_mul(_Atomic(int) in) {
 
   return (in *= 5);
 }
+
+// LIBCALL: [[LC_ATTRS]] = { nounwind willreturn }



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

[PATCH] D85559: [MSAN] Reintroduce libatomic load/store instrumentation

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 285572.
guiand added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85559

Files:
  compiler-rt/test/msan/libatomic.c
  compiler-rt/test/msan/libatomic_load_exceptions.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/libatomic.ll

Index: llvm/test/Instrumentation/MemorySanitizer/libatomic.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/MemorySanitizer/libatomic.ll
@@ -0,0 +1,69 @@
+; RUN: opt < %s -msan-check-access-address=0 -S -passes=msan 2>&1 | FileCheck %s
+; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=2 -S -passes=msan 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-ORIGIN
+; RUN: opt < %s -msan -msan-check-access-address=0 -S | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @__atomic_load(i64, i8*, i8*, i32)
+declare void @__atomic_store(i64, i8*, i8*, i32)
+
+define i24 @odd_sized_load(i24* %ptr) sanitize_memory {
+; CHECK: @odd_sized_load(i24* {{.*}}[[PTR:%.+]])
+; CHECK: [[VAL_PTR:%.*]] = alloca i24, align 1
+; CHECK-ORIGIN: @__msan_set_alloca_origin
+; CHECK: [[VAL_PTR_I8:%.*]] = bitcast i24* [[VAL_PTR]] to i8*
+; CHECK: [[PTR_I8:%.*]] = bitcast i24* [[PTR]] to i8*
+; CHECK: call void @__atomic_load(i64 3, i8* [[PTR_I8]], i8* [[VAL_PTR_I8]], i32 2)
+
+; CHECK: ptrtoint i8* [[PTR_I8]]
+; CHECK: xor
+; CHECK: [[SPTR_I8:%.*]] = inttoptr
+; CHECK-ORIGIN: add
+; CHECK-ORIGIN: and
+; CHECK-ORIGIN: [[OPTR:%.*]] = inttoptr
+
+; CHECK: ptrtoint i8* [[VAL_PTR_I8]]
+; CHECK: xor
+; CHECK: [[VAL_SPTR_I8:%.*]] = inttoptr
+; CHECK-ORIGIN: add
+; CHECK-ORIGIN: and
+; CHECK-ORIGIN: [[VAL_OPTR:%.*]] = inttoptr
+
+; CHECK: call void @llvm.memcpy{{.*}}(i8* align 1 [[VAL_SPTR_I8]], i8* align 1 [[SPTR_I8]], i64 3
+
+; CHECK-ORIGIN: [[ARG_ORIGIN:%.*]] = load i32, i32* [[OPTR]]
+; CHECK-ORIGIN: [[VAL_ORIGIN:%.*]] = call i32 @__msan_chain_origin(i32 [[ARG_ORIGIN]])
+; CHECK-ORIGIN: call void @__msan_set_origin(i8* [[VAL_PTR_I8]], i64 3, i32 [[VAL_ORIGIN]])
+
+; CHECK: [[VAL:%.*]] = load i24, i24* [[VAL_PTR]]
+; CHECK: ret i24 [[VAL]]
+  %val_ptr = alloca i24, align 1
+  %val_ptr_i8 = bitcast i24* %val_ptr to i8*
+  %ptr_i8 = bitcast i24* %ptr to i8*
+  call void @__atomic_load(i64 3, i8* %ptr_i8, i8* %val_ptr_i8, i32 0)
+  %val = load i24, i24* %val_ptr
+  ret i24 %val
+}
+
+define void @odd_sized_store(i24* %ptr, i24 %val) sanitize_memory {
+; CHECK: @odd_sized_store(i24* {{.*}}[[PTR:%.+]], i24 {{.*}}[[VAL:%.+]])
+; CHECK: [[VAL_PTR:%.*]] = alloca i24, align 1
+; CHECK: store i24 [[VAL]], i24* [[VAL_PTR]]
+; CHECK: [[VAL_PTR_I8:%.*]] = bitcast i24* [[VAL_PTR]] to i8*
+; CHECK: [[PTR_I8:%.*]] = bitcast i24* [[PTR]] to i8*
+
+; CHECK: ptrtoint i8* [[PTR_I8]]
+; CHECK: xor
+; CHECK: [[SPTR_I8:%.*]] = inttoptr
+; CHECK: call void @llvm.memset{{.*}}(i8* align 1 [[SPTR_I8]], i8 0, i64 3
+
+; CHECK: call void @__atomic_store(i64 3, i8* [[VAL_PTR_I8]], i8* [[PTR_I8]], i32 3)
+; CHECK: ret void
+  %val_ptr = alloca i24, align 1
+  store i24 %val, i24* %val_ptr
+  %val_ptr_i8 = bitcast i24* %val_ptr to i8*
+  %ptr_i8 = bitcast i24* %ptr to i8*
+  call void @__atomic_store(i64 3, i8* %val_ptr_i8, i8* %ptr_i8, i32 0)
+  ret void
+}
+
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -573,6 +573,9 @@
   /// uninitialized value and returns an updated origin id encoding this info.
   FunctionCallee MsanChainOriginFn;
 
+  /// Run-time helper that paints an origin over a region.
+  FunctionCallee MsanSetOriginFn;
+
   /// MSan runtime replacements for memmove, memcpy and memset.
   FunctionCallee MemmoveFn, MemcpyFn, MemsetFn;
 
@@ -851,6 +854,9 @@
   // instrumentation.
   MsanChainOriginFn = M.getOrInsertFunction(
 "__msan_chain_origin", IRB.getInt32Ty(), IRB.getInt32Ty());
+  MsanSetOriginFn =
+  M.getOrInsertFunction("__msan_set_origin", IRB.getVoidTy(),
+IRB.getInt8PtrTy(), IntptrTy, IRB.getInt32Ty());
   MemmoveFn = M.getOrInsertFunction(
 "__msan_memmove", IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
 IRB.getInt8PtrTy(), IntptrTy);
@@ -1818,6 +1824,24 @@
 llvm_unreachable("Unknown ordering");
   }
 
+  Value *makeAddReleaseOrderingTable(IRBuilder<> &IRB) {
+constexpr int NumOrderings = (int)AtomicOrderingCABI::seq_cst + 1;
+uint32_t OrderingTable[NumOrderings] = {};
+
+OrderingTable[(int)AtomicOrderingCABI::relaxed] =
+OrderingTable[(int)AtomicOrderingCABI::release] =
+(

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 2 inline comments as done.
c-rhodes added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3330
+// appendices to the Procedure Call Standard for the Arm Architecture, see:
+// 
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-c-mangling
+void CXXNameMangler::mangleAArch64FixedSveVectorType(const VectorType *T) {

efriedma wrote:
> Mangling them the same way is going to cause practical issues; they're 
> different types from a C++ perspective, so they need distinct manglings.  For 
> example, you'll crash the compiler if you refer to both foo and 
> foo.
> Mangling them the same way is going to cause practical issues; they're 
> different types from a C++ perspective, so they need distinct manglings. For 
> example, you'll crash the compiler if you refer to both foo and 
> foo.

The ACLE is yet to define the mangling scheme for fixed-length SVE types so I 
kept the mangling the same, which is also what GCC currently does. After 
speaking with @rsandifo-arm yesterday we agreed to come up with a mangling 
scheme where the types are mangled in the same way as:

```__SVE_VLS```

where the first argument is the underlying variable-length type and the second 
argument is the SVE vector length in bits.  For example:

```#if __ARM_FEATURE_SVE_BITS==512
// Mangled as 9__SVE_VLSIu11__SVInt32_tLj512EE
typedef svint32_t vec __attribute__((arm_sve_vector_bits(512)));  
// Mangled as 9__SVE_VLSIu10__SVBool_tLj512EE  
typedef svbool_t pred __attribute__((arm_sve_vector_bits(512)));  
#endif```

let us know if you have any feedback/concerns about this approach.



Comment at: clang/lib/CodeGen/CGCall.cpp:1361
+Tmp.getAlignment().getAsAlign(),
+llvm::ConstantInt::get(CGF.IntPtrTy, DstSize.getKnownMinSize()));
   }

@efriedma If we're happy with the element bitcast above this can also be fixed 
but I wasn't if that was ok, although it's pretty much what was implemented in 
the original codegen patch.


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

https://reviews.llvm.org/D85743

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rjmccall, mibintc, shafik, kpn.
Herald added a subscriber: martong.
Herald added a project: clang.
sepavloff requested review of this revision.

This change allow a CallExpr to have optional FPOptionsOverride object,
stored in trailing storage. The implementaion is made similar to the way
used in CallExpr. Of all cast nodes only ImplicitCastExpr, CStyleCastExpr,
CXXFunctionalCastExpr and CXXStaticCastExpr are allowed to have
FPOptions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85960

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp

Index: clang/test/AST/ast-dump-fpfeatures.cpp
===
--- clang/test/AST/ast-dump-fpfeatures.cpp
+++ clang/test/AST/ast-dump-fpfeatures.cpp
@@ -34,4 +34,48 @@
 // CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT:   ReturnStmt
-// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
\ No newline at end of file
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
+
+int func_04(float x) {
+#pragma STDC FP_CONTRACT ON
+  return x;
+}
+
+// CHECK:  FunctionDecl {{.*}} func_04 'int (float)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   ImplicitCastExpr {{.*}} 'int'  FPContractMode=1
+
+float func_05(double x) {
+#pragma STDC FP_CONTRACT ON
+  return (float)x;
+}
+
+// CHECK:  FunctionDecl {{.*}} func_05 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CStyleCastExpr {{.*}} FPContractMode=1
+
+float func_06(double x) {
+#pragma STDC FP_CONTRACT ON
+  return float(x);
+}
+
+// CHECK:  FunctionDecl {{.*}} func_06 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CXXFunctionalCastExpr {{.*}} FPContractMode=1
+
+float func_07(double x) {
+#pragma STDC FP_CONTRACT ON
+  return static_cast(x);
+}
+
+// CHECK:  FunctionDecl {{.*}} func_07 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CXXStaticCastExpr {{.*}} FPContractMode=1
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -946,12 +946,16 @@
 void ASTStmtWriter::VisitCastExpr(CastExpr *E) {
   VisitExpr(E);
   Record.push_back(E->path_size());
+  Record.push_back(E->hasStoredFPFeatures());
   Record.AddStmt(E->getSubExpr());
   Record.push_back(E->getCastKind()); // FIXME: stable encoding
 
   for (CastExpr::path_iterator
  PI = E->path_begin(), PE = E->path_end(); PI != PE; ++PI)
 Record.AddCXXBaseSpecifier(**PI);
+
+  if (E->hasStoredFPFeatures())
+Record.push_back(E->getFPFeatures().getAsOpaqueInt());
 }
 
 void ASTStmtWriter::VisitBinaryOperator(BinaryOperator *E) {
@@ -1003,7 +1007,7 @@
   VisitCastExpr(E);
   Record.push_back(E->isPartOfExplicitCast());
 
-  if (E->path_size() == 0)
+  if (E->path_size() == 0 && !E->hasStoredFPFeatures())
 AbbrevToUse = Writer.getExprImplicitCastAbbrev();
 
   Code = serialization::EXPR_IMPLICIT_CAST;
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2346,6 +2346,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   // CastExpr
   Abv->Add(BitCodeAbbrevOp(0)); // PathSize
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fix

[PATCH] D85920: [FPEnv][AST] WIP!!! For casts, keep FP options in trailing storage of CastExpr

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Unfortunately I also prepared similar patch: https://reviews.llvm.org/D85960.
I didn't look into your patch in details yet, but I think they are close, as 
the implementation looks straightforward.
We need to merge our patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85920

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Similar patch is here: https://reviews.llvm.org/D85920.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46
+CheckFactories.registerCheck(
+"misc-redundant-condition");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> baloghadamsoftware wrote:
> > aaron.ballman wrote:
> > > baloghadamsoftware wrote:
> > > > aaron.ballman wrote:
> > > > > baloghadamsoftware wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > I think this check should probably live in the `bugprone` module, 
> > > > > > > WDYT?
> > > > > > Based on my experience, `bugpronbe` is for checks whose findings 
> > > > > > are bugs that lead to undefined illegal memory access, behavior 
> > > > > > etc. This one is somewhere between that and readability. For 
> > > > > > example, `redundant-expression` is also in `misc`. But if you wish, 
> > > > > > I can move this checker into `bugprone`.
> > > > > The `bugprone` module has less to do with memory access or undefined 
> > > > > behavior specifically and more to do with checks that should expose 
> > > > > bugs in your code but don't belong to other categories. We try to 
> > > > > keep checks out of `misc` as much as possible these days and this 
> > > > > code pattern is attempting to find cases where the user potentially 
> > > > > has a bug, so I think `bugprone` is the correct home for it.
> > > > > 
> > > > > However, `bugprone` has a similar check and I sort of wonder whether 
> > > > > we should be extending that check rather than adding a separate one. 
> > > > > See `bugprone-branch-clone` which catches the highly related 
> > > > > situation where you have a chain of conditionals and one of the 
> > > > > conditions is repeated. e.g.,
> > > > > ```
> > > > > if (foo) {
> > > > >   if (foo) { // Caught by misc-redundant-condition
> > > > >   }
> > > > > } else if (foo) { // Caught by bugprone-branch-clone
> > > > > }
> > > > > ```
> > > > > Even if we don't combine the checks, we should ensure their behaviors 
> > > > > work well together (catch the same scenarios, don't repeat 
> > > > > diagnostics, etc).
> > > > OK, I will put this into `bugprone`. The two checks may look similar, 
> > > > but this one is more complex because it does not check for the same 
> > > > condition in multiple branches of the same branch statement but checks 
> > > > whether the condition expression could be mutated between the two 
> > > > branch statements. Therefore the the whole logic is totally different, 
> > > > I see no point in merging the two. Should I create a test case then, 
> > > > where both are enabled?
> > > > Therefore the the whole logic is totally different, I see no point in 
> > > > merging the two. 
> > > 
> > > I'm approaching the question from the perspective of the user, not a 
> > > check author. These two checks do the same thing (find redundant 
> > > conditions in flow control which look like they could be a logical 
> > > mistake), so why should they be two separate checks? "Because the code 
> > > looks different" isn't super compelling from that perspective, so I'm 
> > > trying to figure out what the underlying principles are for the checks. 
> > > If they're the same principle, they should be the same check. If they're 
> > > fundamentally different principles, we should be able to explain when to 
> > > use each check as part of their documentation without it sounding 
> > > contrived. (Note, I'm not saying the checks have to be combined, but I am 
> > > pushing back on adding an entirely new check that seems to be redundant 
> > > from a user perspective.)
> > > 
> > > As a litmus test: can you think of a situation where you'd want only one 
> > > of these two checks enabled? I can't think of a case where I'd care about 
> > > redundancy in nested conditionals but not in chained conditionals (or 
> > > vice versa) unless one of the checks had a really high false positive 
> > > rate (which isn't much of a reason to split the checks anyway).
> > > 
> > > > Should I create a test case then, where both are enabled?
> > > 
> > > If we wind up keeping the checks separate, then probably yes (also, the 
> > > documentation for the checks should be updated to explain how they're 
> > > different and why that's useful).
> > There are many checks that users almost always keep enabled together, but 
> > they are still separate checks. Now I looked into the branch clone check, 
> > combining them means simply copying them together because the logic is so 
> > much different.
> > 
> > Even from the user's perspective I see that branches with identical 
> > conditions are different from redundant checks. While the first one is a 
> > more serious bug (the second branch with the same condition is never 
> > executed) this one is slightly more than a readability error.
> > There are many checks that users almost always keep enabled together, but 
> > they are still separate checks. 
> 
> I cannot find an instance with two checks

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 285606.
baloghadamsoftware retitled this revision from "[clang-tidy] New check 
`misc-redundant-condition`" to "[clang-tidy] New check 
`bugprone-redundant-branch-condition`".
baloghadamsoftware edited the summary of this revision.
baloghadamsoftware added a comment.

Check renamed.


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

https://reviews.llvm.org/D81272

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-redundant-branch-condition.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
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
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -0,0 +1,1153 @@
+// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void tryPutFireOut();
+bool callTheFD();
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --===//
+
+void positive_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+}
+  }
+}
+
+void positive_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire && peopleInTheBuilding > 0) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire && peopleInTheBuilding > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (peopleInTheBuilding > 0 && onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (peopleInTheBuilding > 0 && onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire || isCollapsing()) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire || isCollapsing()) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+  }
+  // CHECK-FIXES: {{^\ *$}}
+}
+  }
+}
+
+void positive_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (isCollapsing() || onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (isCollapsing() || onFire) {
+// CHECK-

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 285608.
baloghadamsoftware added a comment.

It seems that //release notes// were not renamed automatically. I updated it 
manually now.


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

https://reviews.llvm.org/D81272

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-redundant-branch-condition.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
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
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -0,0 +1,1153 @@
+// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void tryPutFireOut();
+bool callTheFD();
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --===//
+
+void positive_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+}
+  }
+}
+
+void positive_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire && peopleInTheBuilding > 0) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire && peopleInTheBuilding > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (peopleInTheBuilding > 0 && onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (peopleInTheBuilding > 0 && onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire || isCollapsing()) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire || isCollapsing()) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+  }
+  // CHECK-FIXES: {{^\ *$}}
+}
+  }
+}
+
+void positive_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (isCollapsing() || onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (isCollapsing() || onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+s

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

In this process we also create some other tests, in order to not lose
coverage when focusing on the annotated code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85962

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -437,34 +437,17 @@
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_Identifier) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test(int a) {
-  a;
+  [[a]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-int
-  |   | `-SimpleDeclarator
-  |   |   `-a
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-a
-| `-;
-`-}
-)txt"));
+  {R"txt(
+IdExpression
+`-UnqualifiedId
+  `-a
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_OperatorFunctionId) {
@@ -656,104 +639,85 @@
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 struct X { };
 void test(X x) {
   // TODO: Expose `id-expression` from `MemberExpr`
-  x.~X();
+  [[x.~X()]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-struct
-| |-X
-| |-{
-| |-}
-| `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-X
-  |   | `-SimpleDeclarator
-  |   |   `-x
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | |-~
-| | | `-X
-| | |-(
-| | `-)
-| `-;
-`-}
-)txt"));
+  {R"txt(
+UnknownExpression
+|-UnknownExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-x
+| |-.
+| |-~
+| `-X
+|-(
+`-)
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_DecltypeDestructor) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 struct X { };
 void test(X x) {
   // TODO: Expose `id-expression` from `MemberExpr`
-  x.~decltype(x)();
-}
+  [[x.~decltype(x)()]];
+}
+)cpp",
+  {R"txt(
+UnknownExpression
+|-UnknownExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-x
+| |-.
+| `-~
+|-decltype
+|-(
+|-x
+|-)
+|-(
+`-)
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ClassTemplateDeclaration) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+template
+struct ST {};
 )cpp",
   R"txt(
 *: TranslationUnit
-|-SimpleDeclaration
-| |-struct
-| |-X
-| |-{
-| |-}
-| `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-X
-  |   | `-SimpleDeclarator
-  |   |   `-x
-  |   `-)
-  `-CompoundStatement
+`-TemplateDeclaration
+  |-template
+  |-<
+  |-UnknownDeclaration
+  | |-typename
+  | `-T
+  |->
+  `-SimpleDeclaration
+|-struct
+|-ST
 |-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | `-~
-| | |-decltype
-| | |-(
-| | |-x
-| | |-)
-| | |-(
-| | `-)
-| `-;
-`-}
+|-}
+`-;
 )txt"));
 }
 
-TEST_P(SyntaxTreeTest, UnqualifiedId_TemplateId) {
+TEST_P(SyntaxTreeTest, FunctionTemplateDeclaration) {
   if (!GetParam().isCXX()) {
 return;
   }
@@ -761,51 +725,52 @@
   R"cpp(
 template
 T f();
-void test() {
-  f();
-}
 )cpp",
   R"txt(
 *: TranslationUnit
-|-TemplateDeclaration
-| |-template
-| |-<
-| |-UnknownDeclaration
-| | |-typename
-| | `-T
-| |->
-| `-SimpleDeclaration
-|   |-T
-|   |-SimpleDeclarator
-|   | |-f
-|   | `-ParametersAndQualifiers
-|   |   |-(
-|   |   `-)
-|   `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-f
-| | |   |-<
-| | |   |-int
-| | |   `->
-| | |-(
-| | `-)
-| `-;
-`-}
+`-TemplateDeclaration
+  |-template
+  |-<
+  |-UnknownDeclaration
+  | |-typename
+  | `-T
+  |->
+  `-SimpleDeclaration
+|-T
+|-SimpleDeclarator
+| |-f
+| `-ParametersAndQualifiers
+|   |-(
+|   `-)
+`-;
 )

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 285613.
eduucaldas added a comment.

Previous diff had only the last commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85962

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -34,6 +34,9 @@
 
   ::testing::AssertionResult treeDumpEqual(StringRef Code, StringRef Tree);
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps);
   /// Finds the deepest node in the tree that covers exactly \p R.
   /// FIXME: implement this efficiently and move to public syntax tree API.
   syntax::Node *nodeByRange(llvm::Annotations::Range R, syntax::Node *Root);
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -180,6 +180,35 @@
   return ::testing::AssertionSuccess();
 }
 
+::testing::AssertionResult
+SyntaxTreeTest::treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+   ArrayRef TreeDumps) {
+  SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+  auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+  auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+  if (Diags->getClient()->getNumErrors() != 0) {
+return ::testing::AssertionFailure()
+   << "Source file has syntax errors, they were printed to the test "
+  "log";
+  }
+
+  bool failed = false;
+  auto AnnotatedRanges = AnnotatedCode.ranges();
+  assert(AnnotatedRanges.size() == TreeDumps.size());
+  for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+assert(AnnotatedNode);
+auto AnnotatedNodeDump =
+std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+// EXPECT_EQ shows the diff between the two strings if they are different.
+EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+  failed = true;
+  }
+  return failed ? ::testing::AssertionFailure() : ::testing::AssertionSuccess();
+}
 syntax::Node *SyntaxTreeTest::nodeByRange(llvm::Annotations::Range R,
   syntax::Node *Root) {
   ArrayRef Toks = tokens(Root);
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -437,34 +437,17 @@
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_Identifier) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test(int a) {
-  a;
+  [[a]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-int
-  |   | `-SimpleDeclarator
-  |   |   `-a
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-a
-| `-;
-`-}
-)txt"));
+  {R"txt(
+IdExpression
+`-UnqualifiedId
+  `-a
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_OperatorFunctionId) {
@@ -656,104 +639,85 @@
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 struct X { };
 void test(X x) {
   // TODO: Expose `id-expression` from `MemberExpr`
-  x.~X();
+  [[x.~X()]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-struct
-| |-X
-| |-{
-| |-}
-| `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-X
-  |   | `-SimpleDeclarator
-  |   |   `-x
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | |-~
-| | | `-X
-| | |-(
-| | `-)
-| `-;
-`-}
-)txt"));
+  {R"txt(
+UnknownExpression
+|-UnknownExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-x
+| |-.
+| |-~
+| `-X
+|-(
+`-)
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_DecltypeDestructor) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

In D84602#2206512 , @aaron.ballman 
wrote:

> I still think this case needs some work to keep the bots happy. I would 
> probably go with:
>
> case CC_MSP430Builtin:
>
>   // FIXME: Currently unsupported
>   break;
>
> But perhaps @echristo has opinions since this involves debugging information.

Agree, unless there exist some straightforward way to add a new vendor-specific 
extension to `include/llvm/BinaryFormat/Dwarf.def`. But I suspect there would 
be some standardization stuff and not sure it will actually be that useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

[Amara Emerson via Phabricator via cfe-commits]

aemerson abandoned this revision.
aemerson added a comment.

Seems no one is enthusiastic about this change, so I'm going to drop it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59615

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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D84415#2215780 , @vsavchenko wrote:

> Off-topic: I really think that we should have some sort of DSL for that kind 
> of stuff.

In a sense the API provided in the Checker itself is an (embedded) DSL. Or do 
you think about being able to provide the summaries by describing them in a 
JSON or a YAML file?




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2013
+if (Pthread_tTy) {
+  Pthread_tPtrTy = ACtx.getPointerType(*Pthread_tTy);
+  Pthread_tPtrRestrictTy = getRestrictTy(*Pthread_tPtrTy);

vsavchenko wrote:
> It feels like the readability of the code here can be drastically improved by 
> introducing functions `getPointerType`, `getRestrictType`, and similar 
> accepting `Optional` arguments. 
Yeah, we already have `getRestrictTy`, but never though to have `getPointerTy`, 
which indeed simplifies the code, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 285626.
martong marked an inline comment as done.
martong added a comment.

- Use the shorter 'getPointerTy'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -781,6 +781,7 @@
   auto getRestrictTy = [&ACtx](QualType Ty) {
 return ACtx.getLangOpts().C99 ? ACtx.getRestrictType(Ty) : Ty;
   };
+  auto getPointerTy = [&ACtx](QualType Ty) { return ACtx.getPointerType(Ty); };
 
   // These types are useful for writing specifications quickly,
   // New specifications should probably introduce more types.
@@ -797,21 +798,23 @@
   const QualType SizeTy = ACtx.getSizeType();
 
   const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
-  const QualType IntPtrTy = ACtx.getPointerType(IntTy); // int *
+  const QualType IntPtrTy = getPointerTy(IntTy); // int *
   const QualType UnsignedIntPtrTy =
-  ACtx.getPointerType(Unsigned

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 285629.
eduucaldas added a comment.

Move added Declaration tests to an appropriate place


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85962

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -34,6 +34,9 @@
 
   ::testing::AssertionResult treeDumpEqual(StringRef Code, StringRef Tree);
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps);
   /// Finds the deepest node in the tree that covers exactly \p R.
   /// FIXME: implement this efficiently and move to public syntax tree API.
   syntax::Node *nodeByRange(llvm::Annotations::Range R, syntax::Node *Root);
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -180,6 +180,35 @@
   return ::testing::AssertionSuccess();
 }
 
+::testing::AssertionResult
+SyntaxTreeTest::treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+   ArrayRef TreeDumps) {
+  SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+  auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+  auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+  if (Diags->getClient()->getNumErrors() != 0) {
+return ::testing::AssertionFailure()
+   << "Source file has syntax errors, they were printed to the test "
+  "log";
+  }
+
+  bool failed = false;
+  auto AnnotatedRanges = AnnotatedCode.ranges();
+  assert(AnnotatedRanges.size() == TreeDumps.size());
+  for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+assert(AnnotatedNode);
+auto AnnotatedNodeDump =
+std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+// EXPECT_EQ shows the diff between the two strings if they are different.
+EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+  failed = true;
+  }
+  return failed ? ::testing::AssertionFailure() : ::testing::AssertionSuccess();
+}
 syntax::Node *SyntaxTreeTest::nodeByRange(llvm::Annotations::Range R,
   syntax::Node *Root) {
   ArrayRef Toks = tokens(Root);
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -437,34 +437,17 @@
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_Identifier) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test(int a) {
-  a;
+  [[a]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-int
-  |   | `-SimpleDeclarator
-  |   |   `-a
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-a
-| `-;
-`-}
-)txt"));
+  {R"txt(
+IdExpression
+`-UnqualifiedId
+  `-a
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_OperatorFunctionId) {
@@ -656,154 +639,80 @@
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 struct X { };
 void test(X x) {
   // TODO: Expose `id-expression` from `MemberExpr`
-  x.~X();
+  [[x.~X()]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-struct
-| |-X
-| |-{
-| |-}
-| `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-X
-  |   | `-SimpleDeclarator
-  |   |   `-x
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | |-~
-| | | `-X
-| | |-(
-| | `-)
-| `-;
-`-}
-)txt"));
+  {R"txt(
+UnknownExpression
+|-UnknownExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-x
+| |-.
+| |-~
+| `-X
+|-(
+`-)
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_DecltypeDestructor) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 285632.
eduucaldas added a comment.

Add annotations to the last missing tests in expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85962

Files:
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -34,6 +34,9 @@
 
   ::testing::AssertionResult treeDumpEqual(StringRef Code, StringRef Tree);
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps);
   /// Finds the deepest node in the tree that covers exactly \p R.
   /// FIXME: implement this efficiently and move to public syntax tree API.
   syntax::Node *nodeByRange(llvm::Annotations::Range R, syntax::Node *Root);
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -180,6 +180,35 @@
   return ::testing::AssertionSuccess();
 }
 
+::testing::AssertionResult
+SyntaxTreeTest::treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+   ArrayRef TreeDumps) {
+  SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+  auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+  auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+  if (Diags->getClient()->getNumErrors() != 0) {
+return ::testing::AssertionFailure()
+   << "Source file has syntax errors, they were printed to the test "
+  "log";
+  }
+
+  bool failed = false;
+  auto AnnotatedRanges = AnnotatedCode.ranges();
+  assert(AnnotatedRanges.size() == TreeDumps.size());
+  for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+assert(AnnotatedNode);
+auto AnnotatedNodeDump =
+std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+// EXPECT_EQ shows the diff between the two strings if they are different.
+EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+  failed = true;
+  }
+  return failed ? ::testing::AssertionFailure() : ::testing::AssertionSuccess();
+}
 syntax::Node *SyntaxTreeTest::nodeByRange(llvm::Annotations::Range R,
   syntax::Node *Root) {
   ArrayRef Toks = tokens(Root);
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -437,664 +437,400 @@
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_Identifier) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test(int a) {
-  a;
+  [[a]];
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-int
-  |   | `-SimpleDeclarator
-  |   |   `-a
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-a
-| `-;
-`-}
-)txt"));
+  {R"txt(
+IdExpression
+`-UnqualifiedId
+  `-a
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, UnqualifiedId_OperatorFunctionId) {
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 struct X {
   friend X operator+(const X&, const X&);
 };
 void test(X x) {
-  operator+(x, x);
-}
-)cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-struct
-| |-X
-| |-{
-| |-UnknownDeclaration
-| | `-SimpleDeclaration
-| |   |-friend
-| |   |-X
-| |   |-SimpleDeclarator
-| |   | |-operator
-| |   | |-+
-| |   | `-ParametersAndQualifiers
-| |   |   |-(
-| |   |   |-SimpleDeclaration
-| |   |   | |-const
-| |   |   | |-X
-| |   |   | `-SimpleDeclarator
-| |   |   |   `-&
-| |   |   |-,
-| |   |   |-SimpleDeclaration
-| |   |   | |-const
-| |   |   | |-X
-| |   |   | `-SimpleDeclarator
-| |   |   |   `-&
-| |   |   `-)
-| |   `-;
-| |-}
-| `-;
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   |-SimpleDeclaration
-  |   | |-X
-  |   | `-SimpleDeclarator
-  |   |   `-x
-  |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-operator
-| | |   `-+
-| | |-(
-

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:197
+
+  bool failed = false;
+  auto AnnotatedRanges = AnnotatedCode.ranges();





Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:199
+  auto AnnotatedRanges = AnnotatedCode.ranges();
+  assert(AnnotatedRanges.size() == TreeDumps.size());
+  for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {

ASSERT_EQ I think would be better.



Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:200
+  assert(AnnotatedRanges.size() == TreeDumps.size());
+  for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);

LLVM style is usually:

unsigned i = 0; ...



Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:206
+// EXPECT_EQ shows the diff between the two strings if they are different.
+EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+if (AnnotatedNodeDump != TreeDumps[i].trim().str())

Could you dump the annotated source code substring to make debugging failing 
tests easier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85962

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks to the new info, I think the check basically LGTM. Can you add some 
negative tests and documentation wording to make it clear that the check 
doesn't currently handle all logically equivalent predicates, like:

  if (foo) {
  } else {
if (!foo) {
}
  }
  
  // or
  if (foo > 5) {
if (foo > 3) {
}
  }
  
  // or
  if (foo > 5) {
if (5 < foo) {
}
  }

(I'm assuming these cases aren't handled currently and that handling them isn't 
necessary to land the patch.)




Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46
+CheckFactories.registerCheck(
+"misc-redundant-condition");
 CheckFactories.registerCheck(

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > baloghadamsoftware wrote:
> > > aaron.ballman wrote:
> > > > baloghadamsoftware wrote:
> > > > > aaron.ballman wrote:
> > > > > > baloghadamsoftware wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > I think this check should probably live in the `bugprone` 
> > > > > > > > module, WDYT?
> > > > > > > Based on my experience, `bugpronbe` is for checks whose findings 
> > > > > > > are bugs that lead to undefined illegal memory access, behavior 
> > > > > > > etc. This one is somewhere between that and readability. For 
> > > > > > > example, `redundant-expression` is also in `misc`. But if you 
> > > > > > > wish, I can move this checker into `bugprone`.
> > > > > > The `bugprone` module has less to do with memory access or 
> > > > > > undefined behavior specifically and more to do with checks that 
> > > > > > should expose bugs in your code but don't belong to other 
> > > > > > categories. We try to keep checks out of `misc` as much as possible 
> > > > > > these days and this code pattern is attempting to find cases where 
> > > > > > the user potentially has a bug, so I think `bugprone` is the 
> > > > > > correct home for it.
> > > > > > 
> > > > > > However, `bugprone` has a similar check and I sort of wonder 
> > > > > > whether we should be extending that check rather than adding a 
> > > > > > separate one. See `bugprone-branch-clone` which catches the highly 
> > > > > > related situation where you have a chain of conditionals and one of 
> > > > > > the conditions is repeated. e.g.,
> > > > > > ```
> > > > > > if (foo) {
> > > > > >   if (foo) { // Caught by misc-redundant-condition
> > > > > >   }
> > > > > > } else if (foo) { // Caught by bugprone-branch-clone
> > > > > > }
> > > > > > ```
> > > > > > Even if we don't combine the checks, we should ensure their 
> > > > > > behaviors work well together (catch the same scenarios, don't 
> > > > > > repeat diagnostics, etc).
> > > > > OK, I will put this into `bugprone`. The two checks may look similar, 
> > > > > but this one is more complex because it does not check for the same 
> > > > > condition in multiple branches of the same branch statement but 
> > > > > checks whether the condition expression could be mutated between the 
> > > > > two branch statements. Therefore the the whole logic is totally 
> > > > > different, I see no point in merging the two. Should I create a test 
> > > > > case then, where both are enabled?
> > > > > Therefore the the whole logic is totally different, I see no point in 
> > > > > merging the two. 
> > > > 
> > > > I'm approaching the question from the perspective of the user, not a 
> > > > check author. These two checks do the same thing (find redundant 
> > > > conditions in flow control which look like they could be a logical 
> > > > mistake), so why should they be two separate checks? "Because the code 
> > > > looks different" isn't super compelling from that perspective, so I'm 
> > > > trying to figure out what the underlying principles are for the checks. 
> > > > If they're the same principle, they should be the same check. If 
> > > > they're fundamentally different principles, we should be able to 
> > > > explain when to use each check as part of their documentation without 
> > > > it sounding contrived. (Note, I'm not saying the checks have to be 
> > > > combined, but I am pushing back on adding an entirely new check that 
> > > > seems to be redundant from a user perspective.)
> > > > 
> > > > As a litmus test: can you think of a situation where you'd want only 
> > > > one of these two checks enabled? I can't think of a case where I'd care 
> > > > about redundancy in nested conditionals but not in chained conditionals 
> > > > (or vice versa) unless one of the checks had a really high false 
> > > > positive rate (which isn't much of a reason to split the checks anyway).
> > > > 
> > > > > Should I create a test case then, where both are enabled?
> > > > 
> > > > If we wind up keeping the checks separate, then probably yes (also, the 
> > > > documentation for the checks should be updated to explain how they're 
> > > > different and why that's useful).
> > > There are many checks that users almost alwa

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Even some macro-like construct can improve readability. The current way of 
adding functions is source of copy-paste errors because more things are 
repeated, these should be written only once. And the `if`s for the existence of 
types can be eliminated (automated) somehow. So the final way of specifying the 
functions is to only list (get) the types first, then add the functions with 
name, arguments and constraints in a way that nothing needs to be repeated and 
computable parts are done automatically.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2013
+if (Pthread_tTy) {
+  Pthread_tPtrTy = ACtx.getPointerType(*Pthread_tTy);
+  Pthread_tPtrRestrictTy = getRestrictTy(*Pthread_tPtrTy);

martong wrote:
> vsavchenko wrote:
> > It feels like the readability of the code here can be drastically improved 
> > by introducing functions `getPointerType`, `getRestrictType`, and similar 
> > accepting `Optional` arguments. 
> Yeah, we already have `getRestrictTy`, but never though to have 
> `getPointerTy`, which indeed simplifies the code, thanks!
I think the improvement would be if these `get` functions accept `Optional` 
arguments and if the argument is empty return an empty value. This can make the 
`if`s unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[David Sherwood via Phabricator via cfe-commits]

david-arm added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1342
+  if (SrcSize.getKnownMinSize() <= DstSize.getKnownMinSize() ||
+  (isa(SrcTy) ||
+   isa(DstTy))) {

I think if you restructure the code here you could do:

if (isa(SrcTy) || 
isa(DstTy) ||
SrcSize.getFixedSize() <= DstSize.getFixedSize())

since you know that the scalable types have been eliminated by the time we do 
the "<=" comparison.




Comment at: clang/lib/CodeGen/CGCall.cpp:1361
+Tmp.getAlignment().getAsAlign(),
+llvm::ConstantInt::get(CGF.IntPtrTy, DstSize.getKnownMinSize()));
   }

c-rhodes wrote:
> @efriedma If we're happy with the element bitcast above this can also be 
> fixed but I wasn't if that was ok, although it's pretty much what was 
> implemented in the original codegen patch.
Given the if statement above has eliminated scalable vector types I think it's 
safe to use DstSize.getFixedSize() here.


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

https://reviews.llvm.org/D85743

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D81272#2218050 , @aaron.ballman 
wrote:

> Thanks to the new info, I think the check basically LGTM. Can you add some 
> negative tests and documentation wording to make it clear that the check 
> doesn't currently handle all logically equivalent predicates, like:
>
>   if (foo) {
>   } else {
> if (!foo) {
> }
>   }
>   
>   // or
>   if (foo > 5) {
> if (foo > 3) {
> }
>   }
>   
>   // or
>   if (foo > 5) {
> if (5 < foo) {
> }
>   }
>
> (I'm assuming these cases aren't handled currently and that handling them 
> isn't necessary to land the patch.)

Not even equality is handled yet, just single booleans.


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

https://reviews.llvm.org/D81272

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D74361#1927931 , @JonChesterfield 
wrote:

> In D74361#1927863 , @thakis wrote:
>
>> This breaks tests on Windows: http://45.33.8.238/win/10664/step_7.txt
>>
>> Please take a look, and if it takes some time please revert while you 
>> investigate.
>
> Thanks! It seems Windows inserts 'dso_local' into the middle of the generated 
> IR.
>
> I can't test on Windows so the two that failed CI are now marked as 
> "UNSUPPORTED: system-windows".
>
> Do you know a usual work around for variation in symbol visibility? I'm happy 
> to copy it from another test but am wary of guessing what might work on 
> Windows.

Our windows builds always just add dso_local, so we typically just do a 
wildcard there to handle those cases (or, separate check lines for windows).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Also, see this bug:  This crashes immediately when used on a template 
instantiation: https://bugs.llvm.org/show_bug.cgi?id=47169


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D85611: Improve dynamic AST matching diagnostics for conversion errors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Ping


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

https://reviews.llvm.org/D85611

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

[clang] 07c3348 - [OpenMP][NFC] Update test check lines with new script version

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-08-14T08:59:25-05:00
New Revision: 07c33487faff3067953d61e5e968b6c3d1b845d6

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

LOG: [OpenMP][NFC] Update test check lines with new script version

Added: 


Modified: 
clang/test/OpenMP/irbuilder_nested_parallel_for.c

Removed: 




diff  --git a/clang/test/OpenMP/irbuilder_nested_parallel_for.c 
b/clang/test/OpenMP/irbuilder_nested_parallel_for.c
index 929a92827689..2ca6fe711e28 100644
--- a/clang/test/OpenMP/irbuilder_nested_parallel_for.c
+++ b/clang/test/OpenMP/irbuilder_nested_parallel_for.c
@@ -11,10 +11,10 @@
 
 // CHECK-LABEL: @_Z14parallel_for_0v(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* @1)
+// CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* [[GLOB1:@.*]])
 // CHECK-NEXT:br label [[OMP_PARALLEL:%.*]]
 // CHECK:   omp_parallel:
-// CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, 
...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* 
bitcast (void (i32*, i32*)* @_Z14parallel_for_0v..omp_par to void (i32*, i32*, 
...)*))
+// CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, 
...) @__kmpc_fork_call(%struct.ident_t* [[GLOB1]], i32 0, void (i32*, i32*, 
...)* bitcast (void (i32*, i32*)* @_Z14parallel_for_0v..omp_par to void (i32*, 
i32*, ...)*))
 // CHECK-NEXT:br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
 // CHECK:   omp.par.outlined.exit:
 // CHECK-NEXT:br label [[OMP_PAR_EXIT_SPLIT:%.*]]
@@ -23,15 +23,15 @@
 //
 // CHECK-DEBUG-LABEL: @_Z14parallel_for_0v(
 // CHECK-DEBUG-NEXT:  entry:
-// CHECK-DEBUG-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* @1), !dbg !{{[0-9]*}}
+// CHECK-DEBUG-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* [[GLOB1:@.*]]), [[DBG10:!dbg !.*]]
 // CHECK-DEBUG-NEXT:br label [[OMP_PARALLEL:%.*]]
 // CHECK-DEBUG:   omp_parallel:
-// CHECK-DEBUG-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, 
...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, 
...)* bitcast (void (i32*, i32*)* @_Z14parallel_for_0v..omp_par to void (i32*, 
i32*, ...)*)), !dbg !{{[0-9]*}}
+// CHECK-DEBUG-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, 
...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[GLOB1]], i32 0, void (i32*, 
i32*, ...)* bitcast (void (i32*, i32*)* @_Z14parallel_for_0v..omp_par to void 
(i32*, i32*, ...)*)), [[DBG11:!dbg !.*]]
 // CHECK-DEBUG-NEXT:br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
 // CHECK-DEBUG:   omp.par.outlined.exit:
 // CHECK-DEBUG-NEXT:br label [[OMP_PAR_EXIT_SPLIT:%.*]]
 // CHECK-DEBUG:   omp.par.exit.split:
-// CHECK-DEBUG-NEXT:ret void, !dbg !{{[0-9]*}}
+// CHECK-DEBUG-NEXT:ret void, [[DBG14:!dbg !.*]]
 //
 void parallel_for_0(void) {
 #pragma omp parallel
@@ -50,10 +50,10 @@ void parallel_for_0(void) {
 // CHECK-NEXT:store float* [[R:%.*]], float** [[R_ADDR]], align 8
 // CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
 // CHECK-NEXT:store double [[B:%.*]], double* [[B_ADDR]], align 8
-// CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* @1)
+// CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* [[GLOB1]])
 // CHECK-NEXT:br label [[OMP_PARALLEL:%.*]]
 // CHECK:   omp_parallel:
-// CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, 
...) @__kmpc_fork_call(%struct.ident_t* @1, i32 3, void (i32*, i32*, ...)* 
bitcast (void (i32*, i32*, i32*, double*, float**)* 
@_Z14parallel_for_1Pfid..omp_par.1 to void (i32*, i32*, ...)*), i32* 
[[A_ADDR]], double* [[B_ADDR]], float** [[R_ADDR]])
+// CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, 
...) @__kmpc_fork_call(%struct.ident_t* [[GLOB1]], i32 3, void (i32*, i32*, 
...)* bitcast (void (i32*, i32*, i32*, double*, float**)* 
@_Z14parallel_for_1Pfid..omp_par.1 to void (i32*, i32*, ...)*), i32* 
[[A_ADDR]], double* [[B_ADDR]], float** [[R_ADDR]])
 // CHECK-NEXT:br label [[OMP_PAR_OUTLINED_EXIT19:%.*]]
 // CHECK:   omp.par.outlined.exit19:
 // CHECK-NEXT:br label [[OMP_PAR_EXIT_SPLIT:%.*]]
@@ -66,20 +66,20 @@ void parallel_for_0(void) {
 // CHECK-DEBUG-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
 // CHECK-DEBUG-NEXT:[[B_ADDR:%.*]] = alloca double, align 8
 // CHECK-DEBUG-NEXT:store float* [[R:%.*]], float** [[R_ADDR]], align 8
-// CHECK-DEBUG-NEXT:call void @llvm.dbg.declare(metadata float** 
[[R_ADDR]], metadata !{{[0-9]*}}, metadata !DIExpre

[PATCH] D85938: [OpenMP][OMPIRBuilder] Use the source (=directory + filename) for locations

[Johannes Doerfert 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 rG9240e48a588c: [OpenMP][OMPIRBuilder] Use the source 
(=directory + filename) for locations (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D85938?vs=285551&id=285647#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85938

Files:
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp


Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -34,7 +34,8 @@
 BB = BasicBlock::Create(Ctx, "", F);
 
 DIBuilder DIB(*M);
-auto File = DIB.createFile("test.dbg", "/");
+auto File = DIB.createFile("test.dbg", "/src", llvm::None,
+   Optional("/src/test.dbg"));
 auto CU =
 DIB.createCompileUnit(dwarf::DW_LANG_C, File, "llvm-C", true, "", 0);
 auto Type = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
@@ -301,7 +302,7 @@
   dyn_cast(SrcStrGlob->getInitializer());
   if (!SrcSrc)
 return;
-  EXPECT_EQ(SrcSrc->getAsCString(), ";test.dbg;foo;3;7;;");
+  EXPECT_EQ(SrcSrc->getAsCString(), ";/src/test.dbg;foo;3;7;;");
 }
 
 TEST_F(OpenMPIRBuilderTest, ParallelSimple) {
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -263,8 +263,10 @@
   DILocation *DIL = Loc.DL.get();
   if (!DIL)
 return getOrCreateDefaultSrcLocStr();
-  StringRef FileName =
-  !DIL->getFilename().empty() ? DIL->getFilename() : M.getName();
+  StringRef FileName = M.getName();
+  if (DIFile *DIF = DIL->getFile())
+if (Optional Source = DIF->getSource())
+  FileName = *Source;
   StringRef Function = DIL->getScope()->getSubprogram()->getName();
   Function =
   !Function.empty() ? Function : Loc.IP.getBlock()->getParent()->getName();


Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -34,7 +34,8 @@
 BB = BasicBlock::Create(Ctx, "", F);
 
 DIBuilder DIB(*M);
-auto File = DIB.createFile("test.dbg", "/");
+auto File = DIB.createFile("test.dbg", "/src", llvm::None,
+   Optional("/src/test.dbg"));
 auto CU =
 DIB.createCompileUnit(dwarf::DW_LANG_C, File, "llvm-C", true, "", 0);
 auto Type = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
@@ -301,7 +302,7 @@
   dyn_cast(SrcStrGlob->getInitializer());
   if (!SrcSrc)
 return;
-  EXPECT_EQ(SrcSrc->getAsCString(), ";test.dbg;foo;3;7;;");
+  EXPECT_EQ(SrcSrc->getAsCString(), ";/src/test.dbg;foo;3;7;;");
 }
 
 TEST_F(OpenMPIRBuilderTest, ParallelSimple) {
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -263,8 +263,10 @@
   DILocation *DIL = Loc.DL.get();
   if (!DIL)
 return getOrCreateDefaultSrcLocStr();
-  StringRef FileName =
-  !DIL->getFilename().empty() ? DIL->getFilename() : M.getName();
+  StringRef FileName = M.getName();
+  if (DIFile *DIF = DIL->getFile())
+if (Optional Source = DIF->getSource())
+  FileName = *Source;
   StringRef Function = DIL->getScope()->getSubprogram()->getName();
   Function =
   !Function.empty() ? Function : Loc.IP.getBlock()->getParent()->getName();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong marked an inline comment as done.
martong added a comment.

In D84415#2218084 , @balazske wrote:

> Even some macro-like construct can improve readability. The current way of 
> adding functions is source of copy-paste errors because more things are 
> repeated, these should be written only once. And the `if`s for the existence 
> of types can be eliminated (automated) somehow. So the final way of 
> specifying the functions is to only list (get) the types first, then add the 
> functions with name, arguments and constraints in a way that nothing needs to 
> be repeated and computable parts are done automatically.

Yes, this totally makes sense, thank you guys for this suggestion!

Now I've started working on to reach this form, what do you think?

  // Taking an optional as arg.
  Optional Pthread_cond_tPtrTy = getPointerTy(Pthread_cond_tTy);
  addToFunctionSummaryMap(
  {"pthread_cond_signal", "pthread_cond_broadcast"},
  // Create a signature with Optionals, addToFunctionSummaryMap will skip
  // the summary if the optional is not set.
  Signature(ArgTypes{Pthread_cond_tPtrTy}, RetType{IntTy}),
  Summary(NoEvalCall)
  .ArgConstraint(NotNull(ArgNo(0;

Side note, one day hopefully we could have the same API to fill a 
`CallDescriptionMap`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Aaron Ballman via Phabricator via cfe-commits]

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

In D81272#2218090 , 
@baloghadamsoftware wrote:

> In D81272#2218050 , @aaron.ballman 
> wrote:
>
>> Thanks to the new info, I think the check basically LGTM. Can you add some 
>> negative tests and documentation wording to make it clear that the check 
>> doesn't currently handle all logically equivalent predicates, like:
>>
>>   if (foo) {
>>   } else {
>> if (!foo) {
>> }
>>   }
>>   
>>   // or
>>   if (foo > 5) {
>> if (foo > 3) {
>> }
>>   }
>>   
>>   // or
>>   if (foo > 5) {
>> if (5 < foo) {
>> }
>>   }
>>
>> (I'm assuming these cases aren't handled currently and that handling them 
>> isn't necessary to land the patch.)
>
> Not even equality is handled yet, just single booleans.

That's what I was understanding too, thanks! LG with additional negative tests 
+ doc changes.


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

https://reviews.llvm.org/D81272

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

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber marked 12 inline comments as done.
bernhardmgruber added a comment.

Thank you for the time to review this!

Could you please also commit it for me? Thank you!




Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430
+  AT->getKeyword() == AutoTypeKeyword::Auto &&
+  !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+return;

aaron.ballman wrote:
> bernhardmgruber wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > bernhardmgruber wrote:
> > > > > aaron.ballman wrote:
> > > > > > Why do we need to check that there aren't any nested local 
> > > > > > qualifiers?
> > > > > Because I would like the check to rewrite e.g. `const auto f();` into 
> > > > > `auto f() -> const auto;`. If I omit the check for nested local 
> > > > > qualifiers, then those kind of declarations would be skipped.
> > > > I'm still wondering about this.
> > > > Because I would like the check to rewrite e.g. const auto f(); into 
> > > > auto f() -> const auto;. If I omit the check for nested local 
> > > > qualifiers, then those kind of declarations would be skipped.
> > > 
> > > I don't think I understand why that's desirable though? What is it about 
> > > the qualifier that makes it worthwhile to repeat the type like that?
> > Thank you for insisting on that peculiarity! The choice is stylistically 
> > motivated to align function names:
> > 
> > ```
> > auto f() -> int;
> > auto g() -> std::vector;
> > auto& h();
> > const auto k();
> > decltype(auto) l();
> > ```
> > vs.
> > ```
> > auto f() -> int;
> > auto g() -> std::vector;
> > auto h() -> auto&;
> > auto k() -> const auto; 
> > auto l() -> decltype(auto);
> > ```
> > 
> > But judging from your response, this might be a surprise to users. Would 
> > you prefer having an option to enable/disable this behavior?
> > But judging from your response, this might be a surprise to users. Would 
> > you prefer having an option to enable/disable this behavior?
> 
> Maybe it will be, maybe it won't. :-D The reason I was surprised was because 
> it feels like a formatting related choice rather than a modernization related 
> choice. However, I've always struggled to understand the utility of this 
> check (it's one I  disable in every .clang-tidy configuration file I can), so 
> my reasoning may be flawed. I feel like the goal of this check isn't to 
> format code nicely, it's to modernize to using a trailing return type where 
> that adds clarity. But I don't think `auto& func()` changing into `auto 
> func() -> auto&` adds clarity (I think it removes clarity because the second 
> signature is strictly more complex than the first), and similar for 
> qualifiers. However, I think the exact same thing about `int func()` changing 
> into `auto func() -> int`.
> 
> Given that we document this function to rewrite all functions to a trailing 
> return type signature, I guess the behavior you've proposed here is 
> consistent with that goal and so I'm fine with it.
> However, I've always struggled to understand the utility of this check (it's 
> one I disable in every .clang-tidy configuration file I can)

I am sorry to hear that, but I have heard this from many other people as well. 
I am sometimes questioning myself whether it was a mistake to put this check 
into clang-tidy and annoy a lot of people. It might have been better as a 
standalone tool.

> I feel like the goal of this check isn't to format code nicely, it's to 
> modernize to using a trailing return type where that adds clarity.

I like that thinking! I started with trailing return types as a stylistic 
choice, but I soon realized that it indeed can add clarity by shifting away 
complicated return types to the end of a line where they bother me less.

> But I don't think `auto& func()` changing into `auto func() -> auto&` adds 
> clarity (I think it removes clarity because the second signature is strictly 
> more complex than the first).

With regard to clarity, you are right. And I noted down now that I shall add an 
option to prevent some of these rewrites. Thank you for the feedback!



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

https://reviews.llvm.org/D80514

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D81272#2218050 , @aaron.ballman 
wrote:

> Thanks to the new info, I think the check basically LGTM. Can you add some 
> negative tests and documentation wording to make it clear that the check 
> doesn't currently handle all logically equivalent predicates, like:
>
>   if (foo) {
>   } else {
> if (!foo) {
> }
>   }
>   
>   // or
>   if (foo > 5) {
> if (foo > 3) {
> }
>   }
>   
>   // or
>   if (foo > 5) {
> if (5 < foo) {
> }
>   }
>
> (I'm assuming these cases aren't handled currently and that handling them 
> isn't necessary to land the patch.)

I'm afraid handling such cases will eventually invoke the same problems the 
RangeConstraintSolver has, and then the discussion about whether this is good 
in Tidy instead of CSA will be resurrected... 😦

One could come up with even more elaborate cases. Just hit something like below 
a few minutes ago while working:

  const CallExpr* const C;
  
  if (auto* CalledFun = dyn_cast_or_null(C->getCalleeDecl())) {
// ...
if (const auto* Fun = dyn_cast_or_null(C->getCalleeDecl())) {
  // ...
}
// ...
  }

and the rabbit hole goes deeper. But it's pretty interesting how many cases the 
current check found as-is.


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

https://reviews.llvm.org/D81272

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D81272#2218173 , @whisperity wrote:

> In D81272#2218050 , @aaron.ballman 
> wrote:
>
>> Thanks to the new info, I think the check basically LGTM. Can you add some 
>> negative tests and documentation wording to make it clear that the check 
>> doesn't currently handle all logically equivalent predicates, like:
>>
>>   if (foo) {
>>   } else {
>> if (!foo) {
>> }
>>   }
>>   
>>   // or
>>   if (foo > 5) {
>> if (foo > 3) {
>> }
>>   }
>>   
>>   // or
>>   if (foo > 5) {
>> if (5 < foo) {
>> }
>>   }
>>
>> (I'm assuming these cases aren't handled currently and that handling them 
>> isn't necessary to land the patch.)
>
> I'm afraid handling such cases will eventually invoke the same problems the 
> RangeConstraintSolver has, and then the discussion about whether this is good 
> in Tidy instead of CSA will be resurrected... 😦
>
> One could come up with even more elaborate cases. Just hit something like 
> below a few minutes ago while working:
>
>   const CallExpr* const C;
>   
>   if (auto* CalledFun = dyn_cast_or_null(C->getCalleeDecl())) {
> // ...
> if (const auto* Fun = dyn_cast_or_null(C->getCalleeDecl())) 
> {
>   // ...
> }
> // ...
>   }
>
> and the rabbit hole goes deeper. But it's pretty interesting how many cases 
> the current check found as-is.

While I agree with your observation about data and flow sensitivity, I approved 
on the belief that the check as-is provides enough utility to warrant adding it 
as-is. If someone wants to improve the check into being a CSA check, we can 
always deprecate this one at that point. However, if there are strong opinions 
that the check should start out as a CSA check because it requires that 
sensitivity for your needs, now's a good time to bring up those concerns.


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

https://reviews.llvm.org/D81272

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

[PATCH] D81254: [analyzer] Produce symbolic values for C-array elements

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal

> If index1 and index2 has the same value, we should not be confident that the 
> x == y holds.

Thanks! Now I see. Shame on me =)

> We know where it points to (aka. the pointer's value is conjured), but we 
> don't know what the value if there.

Absolutely right. I looked at this patch after a while and don't currently see 
a proper solution.

> I feel like this problem should be handled by some sort of invalidation 
> mechanism.

Definitely it should be some criteria/mark/flag about the region invalidation. 
Maybe someone more confident could prompt us how to.




Comment at: clang/test/Analysis/PR9289.cpp:13-21
+int index_int(const int *a, int index) {
+  int var;
+  int ret = 0;
+  if (a[42] < 2)
+var = 1;
+  if (a[42] < 2)
+ret = var; // no warning about garbage value

steakhal wrote:
> Why do we test this?
> Here we can //reason// about the index.
> This patch preserves the current behavior relating to this.
This is a root case of the bug which this patch came from. I decided to vary it 
by using an explicit index.



Comment at: clang/test/Analysis/PR9289.cpp:36-40
+  if (a[index] < 2)
+var = 1;
+  index = index2;
+  if (a[index] < 2)
+ret = var; // expected-warning{{Assigned value is garbage or undefined 
[core.uninitialized.Assign]}}

steakhal wrote:
> I'm not sure why do you overwrite the `index` if you could simply index with 
> `index2` instead in the following expression. That would make no difference, 
> only make the test more readable IMO.
> Same comment for all the`*_change` test cases.
My intention is to retain `index` inside brackets. Just to check that `index` 
changed indeed.


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

https://reviews.llvm.org/D81254

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D74361#2218143 , @erichkeane wrote:

> Also, see this bug:  This crashes immediately when used on a template 
> instantiation: https://bugs.llvm.org/show_bug.cgi?id=47169

Thanks for the bug report! Template instantiations are missing from the test 
cases, I will go debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D82317#2212145 , @jdoerfert wrote:

> In D82317#2211643 , @guiand wrote:
>
>> After discussing with @eugenis, for the meantime it might be best to do the 
>> following:
>>
>> - Change the masking attribute to be `-fdisable-noundef-analysis` (name 
>> notwithstanding), and have it completely turn off all `noundef`s
>> - Change the llvm-lit configuration to use the new codegen flag for all the 
>> tests by default
>> - Have `noundef` emitted in the frontend by default (when the codegen flag 
>> isn't present)
>
> TBH, I don't see how this solves any problem. It just makes it a problem for 
> someone in the future... (FWIW, I say this being in full support of `noundef`)

It solves the problem of merge conflicts in downstream forks.

This seems the best we can do given the constraints. I understand the concern 
that now we are not testing exactly the same mode that is normally used, but on 
the other hand, the majority of clang tests do not care about noundef - it's an 
orthogonal feature that just happens to affect their CHECK lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 285656.
baloghadamsoftware added a comment.

Tests andd for not yet handled cases. Known limiatations included in the docs.


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

https://reviews.llvm.org/D81272

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-redundant-branch-condition.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
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
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -0,0 +1,1190 @@
+// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void tryPutFireOut();
+bool callTheFD();
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --===//
+
+void positive_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+}
+  }
+}
+
+void positive_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire && peopleInTheBuilding > 0) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire && peopleInTheBuilding > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (peopleInTheBuilding > 0 && onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (peopleInTheBuilding > 0 && onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: if (peopleInTheBuilding > 0 ) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire || isCollapsing()) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire || isCollapsing()) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+  }
+  // CHECK-FIXES: {{^\ *$}}
+}
+  }
+}
+
+void positive_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (isCollapsing() || onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (isCollapsing() || onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+   

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 285657.
baloghadamsoftware added a comment.

Updated according to a comment.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,454 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional l

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D81272#2218175 , @aaron.ballman 
wrote:

> While I agree with your observation about data and flow sensitivity, I 
> approved on the belief that the check as-is provides enough utility to 
> warrant adding it as-is. If someone wants to improve the check into being a 
> CSA check, we can always deprecate this one at that point. However, if there 
> are strong opinions that the check should start out as a CSA check because it 
> requires that sensitivity for your needs, now's a good time to bring up those 
> concerns.

It's generally harder to create big logic mistakes when it comes to more 
complex expressions, assuming the user does't copy-paste (which I might have 
done, in the above example). We do not need to solve //every// potentially 
equivalent conditional (it is unsolvable in the generic case anyways). I'm sure 
this check can be improved later with handling trivial comparisons (such as 
standard library `int` results not being 0, -1, etc.).


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

https://reviews.llvm.org/D81272

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I did a little debugging, and the problem is the template itself isn't a 
complete type until you require it with RequireCompleteType.  You have this 
same problem with incomplete types: https://godbolt.org/z/hvf3M4

I would suspect you either add a RequireCompleteType for your 
loader_uninitialized checks, or move the check somewhere where it is already 
complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D85810: [clang] Pass-through remarks options to lld

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:71
+
+  bool isLLD = llvm::sys::path::filename(LinkerPath) == "ld.lld" ||
+   llvm::sys::path::stem(LinkerPath) != "ld.lld";

MaskRay wrote:
> Checking the path is brittle. Consider 
> `Args.getLastArgValue(options::OPT_fuse_ld_EQ) == "lld"`
Would be better to add equivalent support to gold-plugin.cpp, which uses the 
same lto::Config, and then you don't need the lld check. Just use --plugin-opt 
instead of -mllvm for passing down the internal options, that is recognized by 
lld as well (see other examples here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D74361#2218258 , @erichkeane wrote:

> I did a little debugging, and the problem is the template itself isn't a 
> complete type

That's clear cut then, thanks. This patch was limited to trivially 
constructible types, and we don't know whether the type is trivially 
constructible if it's incomplete. Thus it does require complete types and we're 
missing a check in sema


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Yep!  Declaring a global variable that isn't 'extern' with an incomplete type 
is disallowed anyway, so if you call RequireCompleteType, you're likely just 
diagnosing that early.

You MIGHT have to do some work to allow:

  struct S;
  extern S foo __attribute__((loader_uninitialized));


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D85798: Split Preprocessor/init.c test. NFC.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Preprocessor/init-ppc.c:390
+// PPC:#define __ppc__ 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-AIX %s

Some `//` may be deleted. Without `//`, for example, in Vim, you can jump over 
these blocks with `{` and `}`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85798

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

[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

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

[PATCH] D85977: [release][docs] Update contributions to LLVM 11 for SVE.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli created this revision.
Herald added subscribers: llvm-commits, cfe-commits, tschuett.
Herald added projects: clang, LLVM.
fpetrogalli requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85977

Files:
  clang/test/CodeGen/aarch64-sve-acle-example.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -66,7 +66,9 @@
   added to describe the mapping between scalar functions and vector
   functions, to enable vectorization of call sites. The information
   provided by the attribute is interfaced via the API provided by the
-  ``VFDatabase`` class.
+  ``VFDatabase`` class. When scanning through the set of vector
+  functions associated to a scalar call, the loop vectorizer now
+  relies on ``VFDatabase``, instead of ``TargetLibraryInfo``.
 
 * `dereferenceable` attributes and metadata on pointers no longer imply
   anything about the alignment of the pointer in question. Previously, some
@@ -78,6 +80,17 @@
   information. This information is used to represent Fortran modules debug
   info at IR level.
 
+* LLVM IR now supports two distinct ``llvm::FixedVectorType`` and
+  ``llvm::ScalableVectorType``, both derived from the base class
+  ``llvm::VectorType``. A number of algorithms dealing with IR vector
+  types have been updated to make sure they work for both scalable and
+  fixed vector types. Where possible, the code has been made generic
+  to cover both cases using the base class. Specifically, places that
+  were using the type ``unsigned`` to count the number of lanes of a
+  vector are now using ``llvm::ElementCount``. In places where
+  ``uint64_t`` was used to denote the size in bits of a IR type we
+  have partially migrated the codebase to using ``llvm::TypeSize``.
+
 Changes to building LLVM
 
 
@@ -101,6 +114,55 @@
   default may wish to specify ``-fno-omit-frame-pointer`` to get the old
   behavior. This improves compatibility with GCC.
 
+* Clang supports to the following macros that enable the C-intrinsics
+  from the `Arm C language extensions for SVE
+  `_ (version
+  ``00bet5``, see section 2.1 for the list of intrinsics associated to
+  each macro):
+
+
+  =  =
+  Preprocessor macro Target feature
+  =  =
+  ``__ARM_FEATURE_SVE``  ``+sve``
+  ``__ARM_FEATURE_SVE_BF16`` ``+sve+bf16``
+  ``__ARM_FEATURE_SVE_MATMUL_FP32``  ``+sve+f32mm``
+  ``__ARM_FEATURE_SVE_MATMUL_FP64``  ``+sve+f64mm``
+  ``__ARM_FEATURE_SVE_MATMUL_INT8``  ``+sve+i8mm``
+  ``__ARM_FEATURE_SVE2`` ``+sve2``
+  ``__ARM_FEATURE_SVE2_AES`` ``+sve2-aes``
+  ``__ARM_FEATURE_SVE2_BITPERM`` ``+sve2-bitperm``
+  ``__ARM_FEATURE_SVE2_SHA3````+sve2-sha3``
+  ``__ARM_FEATURE_SVE2_SM4`` ``+sve2-sm4``
+  =  =
+
+  The macros enable users to write C/C++ `Vector Length Agnostic
+  (VLA)` loops, that can be executed on any CPU that implements the
+  underlying instructions supported by the C intrinsics, independently
+  of the hardware vector register size.
+
+  For example, the ``__ARM_FEATURE_SVE`` macro is enabled when
+  targeting AArch64 code generation by setting ``-march=armv8-a+sve``
+  on the command line.
+
+  .. code-block:: c
+ :caption: Example of VLA addition of two arrays with SVE ACLE.
+
+ // Compile with:
+ // `clang++ -march=armv8a+sve ...` (for c++)
+ // `clang -stc=c11 -march=armv8a+sve ...` (for c)
+ #include 
+
+ void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+   for (unsigned i = 0; i < N; i += svcntd()) {
+ svbool_t Pg = svwhilelt_b64(i, N);
+ svfloat64_t vx = svld1(Pg, &x[i]);
+ svfloat64_t vy = svld1(Pg, &y[i]);
+ svfloat64_t vout = svadd_x(Pg, vx, vy);
+svst1(Pg, &out[i], vout);
+   }
+ }
+
 Changes to the MIPS Target
 --
 
Index: clang/test/CodeGen/aarch64-sve-acle-example.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-example.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -x c++ -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=CPP
+// RUN: %clang -x c -std=c11 -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=C
+// REQUIRES: aarch64-registered-target
+
+#include 
+
+// CPP-LABEL: _Z14VLA_add_arraysPdS_S_j:
+// C-LABEL: VLA_add_arrays:
+void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+  for (unsigned i = 0; i < N; i += svcntd()) {
+svbool_t Pg = svwhilelt_b64(i, N);
+  

[PATCH] D85978: [clang-tools-extra] Added missing comma

[Zachary Selk via Phabricator via cfe-commits]

zacharyselk created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
zacharyselk requested review of this revision.

The new diagnostic tool (D85545 ) caught a 
missing comma, adding one to fix the warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85978

Files:
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -345,7 +345,7 @@
 $Class[[Foo]] *$LocalVariable[[FP]] = 
($Class[[Foo]]*)$LocalVariable[[B]];
 int $LocalVariable[[I]] = (int)$LocalVariable[[B]];
   }
-)cpp"
+)cpp",
   R"cpp(
   struct $Class[[B]] {};
   struct $Class[[A]] {


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -345,7 +345,7 @@
 $Class[[Foo]] *$LocalVariable[[FP]] = ($Class[[Foo]]*)$LocalVariable[[B]];
 int $LocalVariable[[I]] = (int)$LocalVariable[[B]];
   }
-)cpp"
+)cpp",
   R"cpp(
   struct $Class[[B]] {};
   struct $Class[[A]] {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -fsyntax-only 
-ast-dump %s | FileCheck %s
+

rsmith wrote:
> I don't think we really need a dedicated AST test for this. Such tests create 
> a maintenance burden, and they don't really capture what we care about here: 
> that all non-zero values are correctly converted to the `true` value of the 
> enumeration type.
I'll remove the test.



Comment at: clang/test/CodeGen/enum-bool.cpp:7-14
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381A1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0

rsmith wrote:
> Some general guidance for writing IR testcases:
> 
>  - Don't test things that aren't relevant to the test case; doing so will 
> make the test brittle as IR generation changes. In particular, don't test the 
> function attribute comments, the `#0` introducing function metadata, 
> instruction names, alignments.
>  - Use `CHECK-LABEL` for each function definition to improve matching 
> semantics and diagnostics on mismatches. (The `CHECK-LABEL`s are checked 
> first, then the input is sliced up into pieces between them, and those pieces 
> are checked independently.)
>  - Don't use `CHECK-NEXT` unless it's relevant to your test that no other 
> instructions appear in between.
Thanks for the information, I'll reduce the test case.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:597-609
+
+namespace dr2338 {
+namespace B {
+enum E : bool { Zero, One };
+consteval E c(int x) { return (E)x; }
+static_assert(static_cast(c(2)) == 1);
+} // namespace B

rsmith wrote:
> This DR test should go in `test/CXX/drs/dr23xx.cpp`, along with a suitable 
> "magic comment" `// dr2338: 12` to indicate this DR is implemented in Clang 
> 12 onwards. (We have tooling that generates the 
> `clang.llvm.org/cxx_dr_status.html` page from those magic comments.)
> 
> I don't think this has anything to do with `consteval`; a more-reduced test 
> should work just as well (eg, `static_assert((int)(E)2 == 1, "");`) and would 
> allow us to test this in C++11 onwards, not only in C++20.
I thought the magic comment could be done in any test file. It'll move the test 
and add the comment.


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

https://reviews.llvm.org/D85612

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

[PATCH] D85977: [release][docs] Update contributions to LLVM 11 for SVE.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 285678.
fpetrogalli added a comment.

Added context to the diff.


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

https://reviews.llvm.org/D85977

Files:
  clang/test/CodeGen/aarch64-sve-acle-example.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -66,7 +66,9 @@
   added to describe the mapping between scalar functions and vector
   functions, to enable vectorization of call sites. The information
   provided by the attribute is interfaced via the API provided by the
-  ``VFDatabase`` class.
+  ``VFDatabase`` class. When scanning through the set of vector
+  functions associated to a scalar call, the loop vectorizer now
+  relies on ``VFDatabase``, instead of ``TargetLibraryInfo``.
 
 * `dereferenceable` attributes and metadata on pointers no longer imply
   anything about the alignment of the pointer in question. Previously, some
@@ -78,6 +80,17 @@
   information. This information is used to represent Fortran modules debug
   info at IR level.
 
+* LLVM IR now supports two distinct ``llvm::FixedVectorType`` and
+  ``llvm::ScalableVectorType``, both derived from the base class
+  ``llvm::VectorType``. A number of algorithms dealing with IR vector
+  types have been updated to make sure they work for both scalable and
+  fixed vector types. Where possible, the code has been made generic
+  to cover both cases using the base class. Specifically, places that
+  were using the type ``unsigned`` to count the number of lanes of a
+  vector are now using ``llvm::ElementCount``. In places where
+  ``uint64_t`` was used to denote the size in bits of a IR type we
+  have partially migrated the codebase to using ``llvm::TypeSize``.
+
 Changes to building LLVM
 
 
@@ -101,6 +114,55 @@
   default may wish to specify ``-fno-omit-frame-pointer`` to get the old
   behavior. This improves compatibility with GCC.
 
+* Clang supports to the following macros that enable the C-intrinsics
+  from the `Arm C language extensions for SVE
+  `_ (version
+  ``00bet5``, see section 2.1 for the list of intrinsics associated to
+  each macro):
+
+
+  =  =
+  Preprocessor macro Target feature
+  =  =
+  ``__ARM_FEATURE_SVE``  ``+sve``
+  ``__ARM_FEATURE_SVE_BF16`` ``+sve+bf16``
+  ``__ARM_FEATURE_SVE_MATMUL_FP32``  ``+sve+f32mm``
+  ``__ARM_FEATURE_SVE_MATMUL_FP64``  ``+sve+f64mm``
+  ``__ARM_FEATURE_SVE_MATMUL_INT8``  ``+sve+i8mm``
+  ``__ARM_FEATURE_SVE2`` ``+sve2``
+  ``__ARM_FEATURE_SVE2_AES`` ``+sve2-aes``
+  ``__ARM_FEATURE_SVE2_BITPERM`` ``+sve2-bitperm``
+  ``__ARM_FEATURE_SVE2_SHA3````+sve2-sha3``
+  ``__ARM_FEATURE_SVE2_SM4`` ``+sve2-sm4``
+  =  =
+
+  The macros enable users to write C/C++ `Vector Length Agnostic
+  (VLA)` loops, that can be executed on any CPU that implements the
+  underlying instructions supported by the C intrinsics, independently
+  of the hardware vector register size.
+
+  For example, the ``__ARM_FEATURE_SVE`` macro is enabled when
+  targeting AArch64 code generation by setting ``-march=armv8-a+sve``
+  on the command line.
+
+  .. code-block:: c
+ :caption: Example of VLA addition of two arrays with SVE ACLE.
+
+ // Compile with:
+ // `clang++ -march=armv8a+sve ...` (for c++)
+ // `clang -stc=c11 -march=armv8a+sve ...` (for c)
+ #include 
+
+ void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+   for (unsigned i = 0; i < N; i += svcntd()) {
+ svbool_t Pg = svwhilelt_b64(i, N);
+ svfloat64_t vx = svld1(Pg, &x[i]);
+ svfloat64_t vy = svld1(Pg, &y[i]);
+ svfloat64_t vout = svadd_x(Pg, vx, vy);
+svst1(Pg, &out[i], vout);
+   }
+ }
+
 Changes to the MIPS Target
 --
 
Index: clang/test/CodeGen/aarch64-sve-acle-example.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-example.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -x c++ -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=CPP
+// RUN: %clang -x c -std=c11 -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=C
+// REQUIRES: aarch64-registered-target
+
+#include 
+
+// CPP-LABEL: _Z14VLA_add_arraysPdS_S_j:
+// C-LABEL: VLA_add_arrays:
+void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+  for (unsigned i = 0; i < N; i += svcntd()) {
+svbool_t Pg = svwhilelt_b64(i, N);
+svfloat64_t vx = svld1(Pg, &x[i]);
+svfl

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3307-3310
+def fbinutils_version_EQ : Joined<["-"], "fbinutils-version=">, Group,
+  HelpText<"Produced object files can use ELF features only supported by ld 
newer than the specified version. If"
+  "-fno-integrated-as is specified, produced assembly can use newer ELF 
features. "
+  "'future' means that features not implemented by any known release can be 
used">;

jhenderson wrote:
> It might be helpful to indicate what the default is.
The default can be overridden by targets. Don't know how to document it.

(If this ever gets wider use, people may want to add a configure time variable.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D85474#2217598 , @jhenderson wrote:

> Looks like there's only an `llc` test, but the option is in clang too? Should 
> we have a test there?

There are generally some dotted edges in the testing of such MC level options. 
For options affecting IR generating, clang/test/CodeGen can do the tests. For 
MC level options, IR is not affected at all. Such tests belong to 
`llvm/test/CodeGen/`. However, clang cannot be used due to layering concerns. 
The closest (which is used by other options, e.g. -fuse-integrated-as, 
-Wa,-mrelax-relocations=) is to have some llc options similar to the clang 
driver options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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

[PATCH] D85977: [release][docs] Update contributions to LLVM 11 for SVE.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:162
+ svfloat64_t vout = svadd_x(Pg, vx, vy);
+svst1(Pg, &out[i], vout);
+   }

Nit: indentation?


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

https://reviews.llvm.org/D85977

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

[PATCH] D83071: Add support for options with two flags for controlling the same field.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83071

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

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 285681.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added a subscriber: kristof.beyls.

Add release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/X86/explicit-section-mergeable.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -81,6 +81,14 @@
  cl::value_desc("N"),
  cl::desc("Repeat compilation N times for timing"));
 
+static cl::opt BinutilsVersion(
+"binutils-version", cl::Hidden,
+cl::desc(
+"Used ELF features need to be supported by GNU ld "
+"from the specified binutils version. 'future' means that "
+"features not implemented by any known release can be used. If "
+"-no-integrated-as is specified, this also affects assembly output"));
+
 static cl::opt
 NoIntegratedAssembler("no-integrated-as", cl::Hidden,
   cl::desc("Disable integrated assembler"));
@@ -425,6 +433,8 @@
   }
 
   TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags();
+  Options.BinutilsVersion =
+  TargetMachine::parseBinutilsVersion(BinutilsVersion);
   Options.DisableIntegratedAS = NoIntegratedAssembler;
   Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
   Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
Index: llvm/test/CodeGen/X86/explicit-section-mergeable.ll
===
--- llvm/test/CodeGen/X86/explicit-section-mergeable.ll
+++ llvm/test/CodeGen/X86/explicit-section-mergeable.ll
@@ -282,15 +282,19 @@
 ;; --no-integrated-as avoids the use of ",unique," for compatibility with older binutils.
 
 ;; Error if an incompatible symbol is explicitly placed into a mergeable section.
-; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as 2>&1 \
+; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
 ; RUN: | FileCheck %s --check-prefix=NO-I-AS-ERR
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_1' from module '' required a section with entry-size=0 but was placed in section '.rodata.cst16' with entry-size=16: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_4' from module '' required a section with entry-size=0 but was placed in section '.debug_str' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_2' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_4' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 
+;; For GNU as before 2.35,
 ;; Don't create mergeable sections for globals with an explicit section name.
 ; RUN: echo '@explicit = unnamed_addr constant [2 x i16] [i16 1, i16 1], section ".explicit"' > %t.no_i_as.ll
-; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as 2>&1 \
-; RUN: | FileCheck %s --check-prefix=NO-I-AS
-; NO-I-AS: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-OLD
+; NO-I-AS-OLD: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.35 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; NO-I-AS-NEW: .section .explicit,"aM",@progbits,4,unique,1
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -281,3 +281,12 @@
   return TargetIRAnalysis(
   [this](const Function &F) { return this->getTargetTransformInfo(F); });
 }
+
+std::pair TargetMachine::parseBinutilsVersion(StringRef Version) {
+  if (Version == "future")
+return {(int)TargetOptions::FutureBinutilsVersion, 0};
+  std::pair Ret;
+  if (!Version.consumeInteger(10

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 285682.
martong added a comment.

- Use Optionals through the Signature


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,7 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,20 +324,45 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-   

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Now I've update the patch to remove all the `if`s. And the optional types are 
now implicitly handled, so no such mistakes can be done that I had done 
previously.
However, the patch is getting to affect the core of this Checker, so, perhaps I 
should split this into two patches? One NFC for the core changes and another 
that involves only the addition of the pthread functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 285685.
Mordante added a comment.

Addresses review comments.


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

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CodeGen/enum-bool.cpp


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 
+#if __cplusplus >= 201103L
+namespace dr2338 { // dr2338: 12
+namespace B {
+enum E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace D
+} // namespace dr2338
+#endif
+
 namespace dr2346 { // dr2346: 11
   void test() {
 const int i2 = 0;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -1243,7 +1243,13 @@
   return TC_Failed;
 }
 if (SrcType->isIntegralOrEnumerationType()) {
-  Kind = CK_IntegralCast;
+  // [expr.static.cast]p10 If the enumeration type has a fixed underlying
+  // type, the value is first converted to that type by integral conversion
+  const EnumType *Enum = DestType->getAs();
+  Kind = Enum->getDecl()->isFixed() &&
+ Enum->getDecl()->getIntegerType()->isBooleanType()
+ ? CK_IntegralToBoolean
+ : CK_IntegralCast;
   return TC_Success;
 } else if (SrcType->isRealFloatingType())   {
   Kind = CK_FloatingToIntegral;


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_c

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:10317-10320
 IntRange L =
 GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
 IntRange R =
 GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);

rsmith wrote:
> It seems to me that the bug is here. `GetExprRange` is documented as 
> "Pseudo-evaluate the given **integer** expression", but the true and false 
> expressions here might not be integer expressions -- specifically, one of 
> them could be of type `void` if it's a //throw-expression//. In that case, we 
> should only call `GetExprRange` on the other expression. The expression range 
> of the `void`-typed expression is irrelevant in this case, because that 
> expression never returns.
Thanks for the hint I'll look into this solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

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

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:85
+//(note that we do not create implicit base functions here). To avoid
+//this clash we add a new trait to some of them that is always true
+//(this is LLVM after all ;)). It will only influence the mangled name

jdoerfert wrote:
> tra wrote:
> > jdoerfert wrote:
> > > tra wrote:
> > > > jdoerfert wrote:
> > > > > tra wrote:
> > > > > > If you just want to disable some existing declarations that get in 
> > > > > > the way, one way to do it would be to redeclare them with an 
> > > > > > `__arrtibute__((enable_if(false)))`
> > > > > > 
> > > > > > Having overloads with different return types will be observable.
> > > > > We need both overloads as we don't know what return type the system 
> > > > > uses. I modeled the test below this way, that is we don't know if 
> > > > > `isnan` has a `bool` or `int` return type.
> > > > > 
> > > > > > Having overloads with different return types will be observable.
> > > > > 
> > > > > Unsure what observable effect you expect, the variants are there, 
> > > > > yes, but they have different names (wrt the base function and the 
> > > > > other variant function). The variant without a base function is 
> > > > > simply an unused internal function. Could you elaborate what problem 
> > > > > you expect?
> > > > What will be the result of `sizeof(isinf(1.0f))` ? I would expect it to 
> > > > be the same on host and on the device.
> > > > I'm not quite sure what the pragma would do, so it's possible I'm 
> > > > barking at the wrong tree here.
> > > > 
> > > So, I actually had to run this to verify what I suspected would happen:
> > > 
> > > `sizeof(isinf(1.0f))` is in the AST usually:
> > > ```
> > >   | `-UnaryExprOrTypeTraitExpr 0x5586a27bd590  
> > > 'unsigned long' sizeof
> > >   
> > > 
> > >   |   `-ParenExpr 0x5586a27bd570  'bool'
> > >   | `-CallExpr 0x5586a27bd548  'bool'
> > >   |   |-ImplicitCastExpr 0x5586a27bd530  
> > > 'bool (*)(float)' 
> > >   |   | `-DeclRefExpr 0x5586a27bd500  'bool 
> > > (float)' lvalue Function 0x5586a276f9f0 'isinf' 'bool (float)' 
> > > non_odr_use_unevaluated
> > >   |   `-FloatingLiteral 0x5586a27bd290  'float' 
> > > 1.00e+00
> > > ```
> > > If `isinf` has an applicable variant, it will be picked up:
> > > ```
> > >   | `-UnaryExprOrTypeTraitExpr 0x55f9ac949a20  
> > > 'unsigned long' sizeof
> > >   
> > > 
> > >   |   `-ParenExpr 0x55f9ac949a00  'bool'
> > >   | `-PseudoObjectExpr 0x55f9ac9499e0  'bool'
> > >   |   |-CallExpr 0x55f9ac949978  'bool'
> > >   |   | |-ImplicitCastExpr 0x55f9ac949960  
> > > 'bool (*)(float)' 
> > >   |   | | `-DeclRefExpr 0x55f9ac949930  'bool 
> > > (float)' lvalue Function 0x55f9ac7cd1d0 'isinf' 'bool (float)' 
> > > non_odr_use_unevaluated
> > >   |   | `-FloatingLiteral 0x55f9ac9496c0  'float' 
> > > 1.00e+00
> > >   |   `-CallExpr 0x55f9ac9499b8 
> > >  > >  
> > > //data/src/llvm-project/clang/test/Headers/openmp_device_math_isnan.cpp:21:36>
> > >  'bool'
> > >   | |-ImplicitCastExpr 0x55f9ac9499a0 
> > > 
> > >  'bool (*)(float) __attribute__((nothrow))' 
> > >   | | `-DeclRefExpr 0x55f9ac939790  'bool (float) 
> > > __attribute__((nothrow))' Function 0x55f9ac939690 
> > > 'isinf[implementation={extension(disable_implicit_base, match_any, 
> > > allow_templates)}, device={arch(nvptx, nvptx64)}]' 'bool (float@
> > >   | `-FloatingLiteral 0x55f9ac9496c0 
> > > 
> > >  'float' 1.00e+00
> > > ```
> > > That is the behavior I expected, as it happens for any base function call 
> > > with an applicable variant.
> > > 
> > > This patch doesn't change any of this. We have two specialization that do 
> > > only differ in their return type but each will only be a variant of a 
> > > base function with that return type. In any context, when we have a call 
> > > to the original base function, then we try to specialize. Since only the 
> > > `bool` return *or* the `int` return specializations are variants of the 
> > > base function, we might replace the base call with a call, but consistent 
> > > on host and device. I hope this makes some sense, I don't think I did a 
> > > good job explaining.
> > It sounds like openmp's 'variant' is more of an 'overlay' rather than a 
> > CUDA-style target overload that I was thinking of (and overloads don't 
> > allow different return types at all). If I understand you correctly,

[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D74361#2218294 , @erichkeane wrote:

> Yep!  Declaring a global variable that isn't 'extern' with an incomplete type 
> is disallowed anyway, so if you call RequireCompleteType, you're likely just 
> diagnosing that early.
>
> You MIGHT have to do some work to allow:
>
>   struct S;
>   extern S foo __attribute__((loader_uninitialized));

I'll add that to the tests. Looking OK so far, added a check to SemaDecl and 
the crash is gone. No fix needed for C. Will have a patch up soon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D85978: [clang-tools-extra] Added missing comma

[Kamau Bridgeman via Phabricator via cfe-commits]

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

This LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85978

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

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

[Robert Widmann via Phabricator via cfe-commits]

CodaFi created this revision.
CodaFi added reviewers: vsapsai, Bigcheese, doug.gregor.
CodaFi added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.
CodaFi requested review of this revision.

The ModuleManager's use of FileEntry nodes as the keys for its map of
loaded modules is less than ideal. Uniqueness for FileEntry nodes is
maintained by FileManager, which in turn uses inode numbers on hosts
that support that. When coupled with the module cache's proclivity for
turning over and deleting stale PCMs, this means entries for different
module files can wind up reusing the same underlying inode. When this
happens, subsequent accesses to the Modules map will disagree on the
ModuleFile associated with a given file.

It's fine to use the file management utilities to guarantee the presence
of module data, but we need a better source of key material that is
invariant with respect to these OS-level details. Switch instead to use
the given name of the FileEntry node, which should provide separate entries
in the map in the event of inode reuse.

rdar://48443680


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85981

Files:
  clang/include/clang/Serialization/ModuleManager.h
  clang/lib/Serialization/ModuleManager.cpp


Index: clang/lib/Serialization/ModuleManager.cpp
===
--- clang/lib/Serialization/ModuleManager.cpp
+++ clang/lib/Serialization/ModuleManager.cpp
@@ -59,7 +59,7 @@
 }
 
 ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
-  auto Known = Modules.find(File);
+  auto Known = Modules.find(File->getName());
   if (Known == Modules.end())
 return nullptr;
 
@@ -133,7 +133,8 @@
   }
 
   // Check whether we already loaded this module, before
-  if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
+  assert(!Entry || Entry->getName() == FileName);
+  if (ModuleFile *ModuleEntry = Modules.lookup(FileName)) {
 // Check the stored signature.
 if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
   return OutOfDate;
@@ -208,7 +209,7 @@
 return OutOfDate;
 
   // We're keeping this module.  Store it everywhere.
-  Module = Modules[Entry] = NewModule.get();
+  Module = Modules[FileName] = NewModule.get();
 
   updateModuleImports(*NewModule, ImportedBy, ImportLoc);
 
@@ -255,7 +256,7 @@
 
   // Delete the modules and erase them from the various structures.
   for (ModuleIterator victim = First; victim != Last; ++victim) {
-Modules.erase(victim->File);
+Modules.erase(victim->File->getName());
 
 if (modMap) {
   StringRef ModuleName = victim->ModuleName;
Index: clang/include/clang/Serialization/ModuleManager.h
===
--- clang/include/clang/Serialization/ModuleManager.h
+++ clang/include/clang/Serialization/ModuleManager.h
@@ -59,8 +59,8 @@
   // to implement short-circuiting logic when running DFS over the 
dependencies.
   SmallVector Roots;
 
-  /// All loaded modules, indexed by name.
-  llvm::DenseMap Modules;
+  /// All loaded modules, indexed by file name.
+  llvm::StringMap Modules;
 
   /// FileManager that handles translating between filenames and
   /// FileEntry *.


Index: clang/lib/Serialization/ModuleManager.cpp
===
--- clang/lib/Serialization/ModuleManager.cpp
+++ clang/lib/Serialization/ModuleManager.cpp
@@ -59,7 +59,7 @@
 }
 
 ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
-  auto Known = Modules.find(File);
+  auto Known = Modules.find(File->getName());
   if (Known == Modules.end())
 return nullptr;
 
@@ -133,7 +133,8 @@
   }
 
   // Check whether we already loaded this module, before
-  if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
+  assert(!Entry || Entry->getName() == FileName);
+  if (ModuleFile *ModuleEntry = Modules.lookup(FileName)) {
 // Check the stored signature.
 if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
   return OutOfDate;
@@ -208,7 +209,7 @@
 return OutOfDate;
 
   // We're keeping this module.  Store it everywhere.
-  Module = Modules[Entry] = NewModule.get();
+  Module = Modules[FileName] = NewModule.get();
 
   updateModuleImports(*NewModule, ImportedBy, ImportLoc);
 
@@ -255,7 +256,7 @@
 
   // Delete the modules and erase them from the various structures.
   for (ModuleIterator victim = First; victim != Last; ++victim) {
-Modules.erase(victim->File);
+Modules.erase(victim->File->getName());
 
 if (modMap) {
   StringRef ModuleName = victim->ModuleName;
Index: clang/include/clang/Serialization/ModuleManager.h
===
--- clang/include/clang/Serialization/ModuleManager.h
+++ clang/include/clang/Serialization/ModuleManager.h
@@ -59,8 +59,8 @@
   // to implement short-circuiting logic when running DFS over the dependencies.
   SmallVect

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 285692.
tambre added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Pass CMAKE_ASM_COMPILER to compiler-rt in runtime build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85706

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
+  # Make sure ASM language is available.
+  # We explicitly mark the source files as ASM, so they don't get passed to the
+  # C/CXX compiler and hopes that it recognizes them as assembly.
+  enable_language(ASM)
+  set_source_files_properties(${ARGN} PROPERTIES LANGUAGE ASM)
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
+  # Make sure ASM language is available.
+  # We explicitly mark the source files as ASM, so they don't get passed to the
+  # C/CXX compiler and hopes that it recognizes them as assembly.
+  enable_language(ASM)
+  set_source_files_properties(${ARGN} PROPERTIES LANGUAGE ASM)
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

In D85706#2214009 , @phosek wrote:

> One concern I have with this change is that we may not always consistently 
> update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a 
> difference before, but it'll with this change. Can you check if these 
> variables are being updated as needed?

I searched and was able to find only one instance. I've fixed it in this 
change.  
I lack commit privileges, so whoever commits this should be ready to revert it, 
as there's a chance for breakage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85706

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83088#2213886 , @dblaikie wrote:

> In D83088#2213864 , @nhaehnle wrote:
>
>> In D83088#2213802 , @dblaikie wrote:
>>
>>> In D83088#2213797 , @nhaehnle 
>>> wrote:
>>>
 In D83088#2208611 , @dblaikie 
 wrote:

> This seems like a strange hybrid between a static-polymorphism (with 
> traits) and dynamic polymorphism (with base classes/virtual functions). 
> Could this more readily be just one or the other? (sounds like you're 
> leaning towards dynamic polymorphism)

 No, it's very much this way on purpose. The idea is to support the same 
 set of functionality as much as possible in both static **and** dynamic 
 polymorphism.
>>>
>>> Could it be implemented statically as a primary interface, with a dynamic 
>>> wrapper? (eg: a base class, then a derived class template that takes the 
>>> static CFG type to wrap into the dynamic type) keeping the two concepts 
>>> more clearly separated?
>>
>> That is how it is implemented. CfgTraits is the primary static interface, 
>> and then CfgInterface / CfgInterfaceImpl is the dynamic wrapper.
>
> Ah, fair enough. The inheritance details in the traits class confused me a 
> bit/I had a hard time following, with all the features being in the one 
> patch. Might be easier separately, but not sure.
>
> Would it be possible for this not to use traits - I know @asbirlea and I had 
> trouble with some things using GraphTraits owing to the traits API. An 
> alternative would be to describe a CFGGraph concept (same as a standard 
> container concept, for instance) - where there is a concrete graph object and 
> that object is queried for things like nodes, edges, etc. (actually one of 
> the significant things we tripped over was the API choice to navigate edges 
> from a node itself without any extra state - which meant nodes/edge iteration 
> had to carry state (potentially pointers back to the graph, etc) to be able 
> to manifest their edges - trait or concept could both address this by, for 
> traits, passing the graph as well as the node when querying the trait for 
> edges, or for a concept passing the node back to the graph to query for 
> edges).

So there is a bit of a part here where I may admittedly be a bit confused with 
the C++ lingo, since I don't actually like template programming that much :)  
(Which is part of the motivation for this to begin with... so that I can do the 
later changes in the stack here without *everything* being in templates.)

The way the `CfgTraits` is used is that you never use the `CfgTraits` class 
directly except to inherit from it using CRTP (curiously recurring template 
pattern). When writing algorithms that want to be generic over the type of CFG, 
those algorithms then have a derived class of CfgTraits as a template 
parameter. For example, D83094  adds a 
`GenericCycleInfo` template class, where the template parameter 
should be set to e.g. `IrCfgTraits`, if you want cycle info on LLVM IR, or to 
`MachineCfgTraits`, if you want cycle info on MachineIR. Both of these classes 
are derived from `CfgTraits`.

It is definitely different from how `GraphTraits` works, which you use it as 
`GraphTraits`, and then `GraphTraits` etc. are 
specialized implementations. If `GraphTraits` worked the way that `CfgTraits` 
works, then we'd instead have classes like `BasicBlockGraphTraits`.

So to sum it up, all this sounds a bit to me like maybe calling `CfgTraits` 
"traits" is wrong? Is that what you're saying here? You can't just call it 
`Cfg` though, because it's *not* a CFG -- it's a kind of interface to a CFG 
which is designed for static polymorphism, unlike `CfgInterface` which is 
designed for dynamic polymorphism. Getting the names right is important, 
unfortunately I admit that I'm a bit lost there. "Traits" seemed like the 
closest thing to what I want, but I'm definitely open to suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Overall, looks like a reasonable approach to solve inode reuse. The problem 
with filenames is that they might not be canonicalized and the same file can be 
known by different filenames. I'm trying to think through the consequences in 
the following scenarios:

- same name but file content has changed;
- different names but refer to the same file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85981

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, vsavchenko, xazax.hun, dcoughlin.
ASDenysPetrov added a project: clang.
Herald added subscribers: cfe-commits, steakhal, martong, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
ASDenysPetrov requested review of this revision.

This checker is made above PthreadLockChecker and works the same. It is adapted 
for **std::mutex** primitive.

Next I want to add support **std::recursive_mutex**.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85984

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp

Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+struct recursive_mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+template 
+struct guard_lock {
+  T &t;
+  guard_lock(T &m) : t(m) {
+t.lock();
+  }
+  ~guard_lock() {
+t.unlock();
+  }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+std::recursive_mutex rm1;
+std::recursive_mutex rm2;
+
+// ok
+
+void ok1() {
+  m1.lock(); // no-warning
+}
+
+void ok2() {
+  m1.unlock(); // no-warning
+}
+
+void ok3() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void ok4() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void ok5() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+}
+
+void ok6(void) {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+  m1.unlock(); // no-warning
+}
+
+void ok7(void) {
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void ok8(void) {
+  m1.unlock();   // no-warning
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void ok9(void) {
+  if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+  m1.unlock();// no-warning
+}
+
+void ok10() {
+  std::guard_lock gl(m1);
+}
+
+// bad
+
+void bad1() {
+  m1.lock(); // no-warning
+  m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad2() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void bad3() {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  m2.unlock(); // no-warning
+}
+
+void bad5() {
+  while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void bad6() {
+  while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -70,11 +70,17 @@
 class PthreadLockChecker : public Checker {
 public:
-  enum LockingSemantics { NotApplicable = 0, PthreadSemantics, XNUSemantics };
+  enum LockingSemantics {
+NotApplicable = 0,
+PthreadSemantics,
+XNUSemantics,
+CPlusPlusSemantics,
+  };
   enum CheckerKind {
 CK_PthreadLockChecker,
 CK_FuchsiaLockChecker,
 CK_C11LockChecker,
+CK_CPlusPlus11LockChecker,
 CK_NumCheckKinds
   };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
@@ -83,7 +89,7 @@
 private:
   typedef void (PthreadLockChecker::*FnCheck)(const CallEvent &Call,
   CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   CallDescriptionMap PThreadCallbacks = {
   // Init.
   {{"pthread_mutex_init", 2}, &PthreadLockChecker::InitAnyLock},
@@ -164,49 +170,82 @@
   {{"mtx_destroy", 1}, &PthreadLockChecker::DestroyPthreadLock},
   };
 
+  CallDescriptionMap CPlusPlus11Callbacks = {
+
+  // Acquire.
+  {{{"std", "mutex", "lock"}, 0, 0},
+   &PthreadLockChecker::AcquireCPlusPlus11Lock},
+  // TODO {{{"std", "recursive_mutex", "lock"}, 0, 0},
+  // &PthreadLockChecker::AcquireCPlusPlus11RecursiveLock},
+
+  // Try.
+  {{{"std", "mutex", "try_lock"}, 0, 0},
+   &PthreadLockChecker::TryCPlusPlus11Lock},
+  // TODO {{{"std", "recursive_m

[PATCH] D81254: [analyzer] Produce symbolic values for C-array elements

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Hey, folk, welcome to https://reviews.llvm.org/D85984
I've moved the logic of this checker in `PthreadLockChecker`

Should this revision be //closed //or //rejected //somehow?


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

https://reviews.llvm.org/D81254

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

[clang-tools-extra] caac40f - [clang-tools-extra] Added missing comma

[via cfe-commits]

Author: zacharyselk
Date: 2020-08-14T12:27:30-06:00
New Revision: caac40fa5a6158aad93a20eb9aae82513fc5a368

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

LOG: [clang-tools-extra] Added missing comma

The new diagnostic tool (D85545) caught a missing comma, adding one to fix the 
warning.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index f9c2e7433cc4..06743080166b 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -345,7 +345,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
 $Class[[Foo]] *$LocalVariable[[FP]] = 
($Class[[Foo]]*)$LocalVariable[[B]];
 int $LocalVariable[[I]] = (int)$LocalVariable[[B]];
   }
-)cpp"
+)cpp",
   R"cpp(
   struct $Class[[B]] {};
   struct $Class[[A]] {



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

[PATCH] D85978: [clang-tools-extra] Added missing comma

[Zachary Selk via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaac40fa5a61: [clang-tools-extra] Added missing comma 
(authored by zacharyselk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85978

Files:
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -345,7 +345,7 @@
 $Class[[Foo]] *$LocalVariable[[FP]] = 
($Class[[Foo]]*)$LocalVariable[[B]];
 int $LocalVariable[[I]] = (int)$LocalVariable[[B]];
   }
-)cpp"
+)cpp",
   R"cpp(
   struct $Class[[B]] {};
   struct $Class[[A]] {


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -345,7 +345,7 @@
 $Class[[Foo]] *$LocalVariable[[FP]] = ($Class[[Foo]]*)$LocalVariable[[B]];
 int $LocalVariable[[I]] = (int)$LocalVariable[[B]];
   }
-)cpp"
+)cpp",
   R"cpp(
   struct $Class[[B]] {};
   struct $Class[[A]] {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 285704.
Mordante marked an inline comment as done.
Mordante added a comment.

Addresses review comments.


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

https://reviews.llvm.org/D85601

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/conditional-expr.cpp


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10308,10 +10308,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10308,10 +10308,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

Unless anyone else objects, I'm happy to see this landed! Nice work! ^-^


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

https://reviews.llvm.org/D67422

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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D67422#2216137 , @NoQ wrote:

> I agree that there's something here and also that it's not that 
> urgent/critical to rename things at this point. It's not that people suffer 
> horribly from having to link to only some of these things.

In the long run, if we end up piling more of the common stuff in `libAnalysis`, 
it might be worth to create a new library.


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

https://reviews.llvm.org/D67422

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

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 285711.
nhaehnle added a comment.

- use llvm_unreachable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85607

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -20,6 +20,7 @@
 public:
   using ParentType = Region;
   using BlockRef = Block *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static llvm::CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -27,6 +27,7 @@
 public:
   using ParentType = VPRegionBlock;
   using BlockRef = VPBlockBase *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/IR/CFG.cpp
===
--- llvm/lib/IR/CFG.cpp
+++ llvm/lib/IR/CFG.cpp
@@ -36,6 +36,11 @@
   }
 }
 
+void IrCfgTraits::Printer::printInstruction(raw_ostream &out,
+InstructionRef instruction) const {
+  printValue(out, instruction);
+}
+
 void IrCfgTraits::Printer::printBlockName(raw_ostream &out,
   BlockRef block) const {
   if (block->hasName()) {
Index: llvm/lib/CodeGen/MachineCfgTraits.cpp
===
--- llvm/lib/CodeGen/MachineCfgTraits.cpp
+++ llvm/lib/CodeGen/MachineCfgTraits.cpp
@@ -24,6 +24,11 @@
   }
 }
 
+void MachineCfgTraits::Printer::printInstruction(
+raw_ostream &out, MachineInstr *instruction) const {
+  instruction->print(out);
+}
+
 void MachineCfgTraits::Printer::printBlockName(raw_ostream &out,
MachineBasicBlock *block) const {
   block->printName(out);
Index: llvm/include/llvm/Support/CfgTraits.h
===
--- llvm/include/llvm/Support/CfgTraits.h
+++ llvm/include/llvm/Support/CfgTraits.h
@@ -79,6 +79,9 @@
 class CfgBlockRefTag;
 using CfgBlockRef = CfgOpaqueType;
 
+class CfgInstructionRefTag;
+using CfgInstructionRef = CfgOpaqueType;
+
 class CfgValueRefTag;
 using CfgValueRef = CfgOpaqueType;
 
@@ -113,8 +116,10 @@
   //
   // - Static methods for converting BlockRef and ValueRef to and from
   //   static CfgBlockRef wrapRef(BlockRef);
+  //   static CfgInstructionRef wrapRef(InstructionRef);
   //   static CfgValueRef wrapRef(ValueRef);
   //   static BlockRef unwrapRef(CfgBlockRef);
+  //   static InstructionRef unwrapRef(CfgInstructionRef);
   //   static ValueRef unwrapRef(CfgValueRef);
 };
 
@@ -133,6 +138,7 @@
 class CfgTraits : public BaseTraits {
 public:
   using typename BaseTraits::BlockRef;
+  using typename BaseTraits::InstructionRef;
   using typename BaseTraits::ParentType;
   using typename BaseTraits::ValueRef;
 
@@ -159,6 +165,10 @@
   // a range of iterators dereferencing to ValueRef):
   //   static auto blockdefs(BlockRef block);
 
+  // Given two instructions in the same block, this returns true if lhs is
+  // strictly before rhs.
+  //   static bool comesBefore(InstructionRef lhs, InstructionRef rhs);
+
   // Get the block in which a given value is defined. Returns a null-like
   // BlockRef if the value is not defined in a block (e.g. it is a constant or
   // function argument).
@@ -168,6 +178,8 @@
   //   explicit Printer(const CfgTraits &traits);
   //   void printBlockName(raw_ostream &out, BlockRef block) const;
   //   void printValue(raw_ostream &out, ValueRef value) const;
+  //   void printInstruction(raw_ostream &out,
+  // InstructionRef instruction) const;
   // };
 
   ///@}
@@ -295,6 +307,9 @@
   virtual void appendBlocks(CfgParentRef parent,
 SmallVectorImpl &list) const = 0;
 
+  virtual bool comesBefore(CfgInstructionRef lhs,
+   CfgInstructionRef rhs) const = 0;
+
   virtual void appendPredecessors(CfgBlockRef block,
   SmallVectorImpl &list) const = 0;
   virtual void appendSuccessors(CfgBlockRef block,
@@ -331,6 +346,8 @@
 
   virtual void printBlockName(raw_ostream &out, CfgBlockRef block) const = 0;
   virtual void printValue(raw_ostream &out, CfgValueRef value) const = 0;
+  virtual void printInstruction(raw_ostream &out,
+ 

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle marked an inline comment as done.
nhaehnle added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:100
+// provide one at all. We don't want to lay a subtle performance trap here.
+abort();
+  }

arsenm wrote:
> llvm_unreachable?
Makes sense, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85607

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

[PATCH] D85933: Don't track consteval references for dependent members

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

thanks for this,

here is a few comments aside from that this seems fine.




Comment at: clang/lib/Sema/SemaOverload.cpp:1761
+  llvm::SaveAndRestore DisableIITracking(
+  S.RebuildingImmediateInvocation, true);
+

I don't think RebuildingImmediateInvocation should be used here since we are 
not rebuilding immediate invocations

setSuppressAllDiagnostics or TentativeAnalysisScope seems more adapted.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:131
+  consteval A() = default;
+  consteval ~A() = default; // expected-error {{destructor cannot be declared 
consteval}}
+};

could you check that this diagnostic and those like it doesn't appear when the 
struct A isn't instantiated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85933

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

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks!


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

https://reviews.llvm.org/D85601

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

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Thanks for the comments @vitalybuka and @davidxl, will address those shortly.

In D85948#2217476 , @davidxl wrote:

> one nit: since the same instrumentation can be used to profiling global 
> variable accesses (especially those indirect accessed), the option name seems 
> excluding those cases. Shall it be renamed to fmem-prof?

Hmm, good point. Do you think all the internals (pass name, runtime names, etc) 
need to be renamed similarly? Or just the external facing option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85948

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

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:995
 
+def fheapprof : Flag<["-"], "fheapprof">, Group,
+  Flags<[CoreOption, CC1Option]>, HelpText<"Enable heap profiling">;

`OptInFFlag<"heapprof", "Enable", "Disable", " heap profiling">`



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1027
 
+  Opts.HeapProf = Args.hasFlag(OPT_fheapprof, OPT_fno_heapprof, false);
+

If there is a default, the convention is to check just that flag:

`Args.hasArg(OPT_fheapprof);`

and remove -fno-heapprof as a CC1 option.



Comment at: clang/test/Driver/fheapprof.cpp:1
+// RUN: %clang++ -target x86_64-linux-gnu -fheapprof %s -### 2>&1 | FileCheck 
%s
+// CHECK: -cc1{{.*}}-fheapprof

`%clang++` does not work on Windows -> `clang.exe++`

Use `%clangxx`



Comment at: clang/test/Driver/fheapprof.cpp:2
+// RUN: %clang++ -target x86_64-linux-gnu -fheapprof %s -### 2>&1 | FileCheck 
%s
+// CHECK: -cc1{{.*}}-fheapprof
+// CHECK: ld{{.*}}libclang_rt.heapprof{{.*}}libclang_rt.heapprof_cxx

It is better surrounding the argument with double quotes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85948

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

You mentioned in D85920  a need to merge this 
review with that review. I don't think that's needed. This code here is farther 
along. It does everything that D85920  does 
and has necessary pieces implemented as well.

I think this ticket should go into the tree instead of D85920 
. I'd sign off on it this minute if I had more 
experience in clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D85977: [release][docs] Update contributions to LLVM 11 for SVE.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 285731.
fpetrogalli added a comment.

Fix indentation of the code exmaple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85977

Files:
  clang/test/CodeGen/aarch64-sve-acle-example.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -66,7 +66,9 @@
   added to describe the mapping between scalar functions and vector
   functions, to enable vectorization of call sites. The information
   provided by the attribute is interfaced via the API provided by the
-  ``VFDatabase`` class.
+  ``VFDatabase`` class. When scanning through the set of vector
+  functions associated to a scalar call, the loop vectorizer now
+  relies on ``VFDatabase``, instead of ``TargetLibraryInfo``.
 
 * `dereferenceable` attributes and metadata on pointers no longer imply
   anything about the alignment of the pointer in question. Previously, some
@@ -78,6 +80,17 @@
   information. This information is used to represent Fortran modules debug
   info at IR level.
 
+* LLVM IR now supports two distinct ``llvm::FixedVectorType`` and
+  ``llvm::ScalableVectorType``, both derived from the base class
+  ``llvm::VectorType``. A number of algorithms dealing with IR vector
+  types have been updated to make sure they work for both scalable and
+  fixed vector types. Where possible, the code has been made generic
+  to cover both cases using the base class. Specifically, places that
+  were using the type ``unsigned`` to count the number of lanes of a
+  vector are now using ``llvm::ElementCount``. In places where
+  ``uint64_t`` was used to denote the size in bits of a IR type we
+  have partially migrated the codebase to using ``llvm::TypeSize``.
+
 Changes to building LLVM
 
 
@@ -101,6 +114,55 @@
   default may wish to specify ``-fno-omit-frame-pointer`` to get the old
   behavior. This improves compatibility with GCC.
 
+* Clang supports to the following macros that enable the C-intrinsics
+  from the `Arm C language extensions for SVE
+  `_ (version
+  ``00bet5``, see section 2.1 for the list of intrinsics associated to
+  each macro):
+
+
+  =  =
+  Preprocessor macro Target feature
+  =  =
+  ``__ARM_FEATURE_SVE``  ``+sve``
+  ``__ARM_FEATURE_SVE_BF16`` ``+sve+bf16``
+  ``__ARM_FEATURE_SVE_MATMUL_FP32``  ``+sve+f32mm``
+  ``__ARM_FEATURE_SVE_MATMUL_FP64``  ``+sve+f64mm``
+  ``__ARM_FEATURE_SVE_MATMUL_INT8``  ``+sve+i8mm``
+  ``__ARM_FEATURE_SVE2`` ``+sve2``
+  ``__ARM_FEATURE_SVE2_AES`` ``+sve2-aes``
+  ``__ARM_FEATURE_SVE2_BITPERM`` ``+sve2-bitperm``
+  ``__ARM_FEATURE_SVE2_SHA3````+sve2-sha3``
+  ``__ARM_FEATURE_SVE2_SM4`` ``+sve2-sm4``
+  =  =
+
+  The macros enable users to write C/C++ `Vector Length Agnostic
+  (VLA)` loops, that can be executed on any CPU that implements the
+  underlying instructions supported by the C intrinsics, independently
+  of the hardware vector register size.
+
+  For example, the ``__ARM_FEATURE_SVE`` macro is enabled when
+  targeting AArch64 code generation by setting ``-march=armv8-a+sve``
+  on the command line.
+
+  .. code-block:: c
+ :caption: Example of VLA addition of two arrays with SVE ACLE.
+
+ // Compile with:
+ // `clang++ -march=armv8a+sve ...` (for c++)
+ // `clang -stc=c11 -march=armv8a+sve ...` (for c)
+ #include 
+
+ void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+   for (unsigned i = 0; i < N; i += svcntd()) {
+ svbool_t Pg = svwhilelt_b64(i, N);
+ svfloat64_t vx = svld1(Pg, &x[i]);
+ svfloat64_t vy = svld1(Pg, &y[i]);
+ svfloat64_t vout = svadd_x(Pg, vx, vy);
+ svst1(Pg, &out[i], vout);
+   }
+ }
+
 Changes to the MIPS Target
 --
 
Index: clang/test/CodeGen/aarch64-sve-acle-example.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-example.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -x c++ -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=CPP
+// RUN: %clang -x c -std=c11 -c -target aarch64-linux-gnu -march=armv8-a+sve -o - -S %s -O3 | FileCheck %s --check-prefix=C
+// REQUIRES: aarch64-registered-target
+
+#include 
+
+// CPP-LABEL: _Z14VLA_add_arraysPdS_S_j:
+// C-LABEL: VLA_add_arrays:
+void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
+  for (unsigned i = 0; i < N; i += svcntd()) {
+svbool_t Pg = svwhilelt_b64(i, N);

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

I think the user visible option needs to match the functionality. Internal 
naming just needs some comments to document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85948

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

[PATCH] D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield created this revision.
JonChesterfield added reviewers: erichkeane, aaron.ballman, rjmccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
JonChesterfield requested review of this revision.

[Clang] Fix BZ47169, loader_uninitialized on incomplete types

Reported by @erichkeane. Fix proposed by @erichkeane works, tests included.
Bug introduced in D74361 . Crash was on 
querying a CXXRecordDecl for
hasTrivialDefaultConstructor on an incomplete type. Fixed by calling
RequireCompleteType in the right place.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85990

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/attr-loader-uninitialized.cpp
  clang/test/Sema/attr-loader-uninitialized.c
  clang/test/Sema/attr-loader-uninitialized.cpp


Index: clang/test/Sema/attr-loader-uninitialized.cpp
===
--- clang/test/Sema/attr-loader-uninitialized.cpp
+++ clang/test/Sema/attr-loader-uninitialized.cpp
@@ -9,6 +9,11 @@
 extern int external_rejected __attribute__((loader_uninitialized));
 // expected-error@-1 {{variable 'external_rejected' cannot be declared both 
'extern' and with the 'loader_uninitialized' attribute}}
 
+struct S;
+extern S incomplete_external_rejected __attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'S'}}
+// expected-note@-3 {{forward declaration of 'S'}}
+
 int noargs __attribute__((loader_uninitialized(0)));
 // expected-error@-1 {{'loader_uninitialized' attribute takes no arguments}}
 
@@ -58,3 +63,12 @@
 
 nontrivial needs_trivial_ctor __attribute__((loader_uninitialized));
 // expected-error@-1 {{variable with 'loader_uninitialized' attribute must 
have a trivial default constructor}}
+
+struct Incomplete;
+Incomplete incomplete __attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'Incomplete'}}
+// expected-note@-3 {{forward declaration of 'Incomplete'}}
+
+struct Incomplete s_incomplete __attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'struct Incomplete'}}
+// expected-note@-7 {{forward declaration of 'Incomplete'}}
Index: clang/test/Sema/attr-loader-uninitialized.c
===
--- clang/test/Sema/attr-loader-uninitialized.c
+++ clang/test/Sema/attr-loader-uninitialized.c
@@ -10,6 +10,11 @@
 extern int external_rejected __attribute__((loader_uninitialized));
 // expected-error@-1 {{variable 'external_rejected' cannot be declared both 
'extern' and with the 'loader_uninitialized' attribute}}
 
+struct S;
+extern struct S incomplete_external_rejected 
__attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'struct S'}}
+// expected-note@-3 {{forward declaration of 'struct S'}}
+
 int noargs __attribute__((loader_uninitialized(0)));
 // expected-error@-1 {{'loader_uninitialized' attribute takes no arguments}}
 
@@ -35,3 +40,8 @@
 
 extern __attribute__((visibility("hidden"))) int extern_hidden 
__attribute__((loader_uninitialized));
 // expected-error@-1 {{variable 'extern_hidden' cannot be declared both 
'extern' and with the 'loader_uninitialized' attribute}}
+
+struct Incomplete;
+struct Incomplete incomplete __attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'struct Incomplete'}}
+// expected-note@-3 {{forward declaration of 'struct Incomplete'}}
Index: clang/test/CodeGenCXX/attr-loader-uninitialized.cpp
===
--- clang/test/CodeGenCXX/attr-loader-uninitialized.cpp
+++ clang/test/CodeGenCXX/attr-loader-uninitialized.cpp
@@ -28,3 +28,15 @@
 // Defining as arr2[] [[clang..]] raises the error: attribute cannot be 
applied to types
 // CHECK: @arr2 = global [4 x double] undef
 double arr2 [[clang::loader_uninitialized]] [4];
+
+template struct templ{T * t;};
+
+// CHECK: @templ_int = global %struct.templ undef, align 8
+templ templ_int [[clang::loader_uninitialized]];
+
+// CHECK: @templ_trivial = global %struct.templ.0 undef, align 8
+templ templ_trivial [[clang::loader_uninitialized]];
+
+// CHECK: @templ_incomplete = global %struct.templ.1 undef, align 8
+struct incomplete;
+templ templ_incomplete [[clang::loader_uninitialized]];
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12476,6 +12476,11 @@
 }
 
 if (!Var->isInvalidDecl() && RealDecl->hasAttr()) 
{
+  if (RequireCompleteType(Var->getLocation(), Var->getType(),
+  diag::err_typecheck_decl_incomplete_type)) {
+Var->setInvalidDecl();
+return;
+  }
   if (CXXRecordDecl *RD = Var->getType()->getAsCXXRecordDecl()) {
 if (!RD->hasTrivialDefaultConstructor()) {
   Diag(Va

[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Fix proposed at D85990 . Thanks for the 
detailed bug report!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/Sema/attr-loader-uninitialized.cpp:14
+extern S incomplete_external_rejected __attribute__((loader_uninitialized));
+// expected-error@-1 {{variable has incomplete type 'S'}}
+// expected-note@-3 {{forward declaration of 'S'}}

Should this give the 'cannot be declared 'extern' and with the 
'loader_uninitialized' attribute?' error instead?  it seems to make more sense 
there, and is more specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85990

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

[PATCH] D85559: [MSAN] Reintroduce libatomic load/store instrumentation

[Evgenii Stepanov via Phabricator via cfe-commits]

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

LGTM with 1 comment




Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3573
+  case LibFunc_atomic_load:
+if (!isa(CB)) {
+  llvm::errs() << "MSAN -- cannot instrument invoke of libatomic load."

Probably need the same check for atomic_store.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85559

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante planned changes to this revision.
Mordante marked 14 inline comments as done.
Mordante added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Hmm, I'm on the fence about specifying `201803` for these attributes. Given 
> that this is only the start of supporting the attribute, do we want to claim 
> it already matches the standard's behavior? Or do we just want to return `1` 
> to signify that we understand this attribute but we don't yet fully support 
> it in common cases (such as on labels in switch statements, etc)?
> 
> As another question, should we consider adding a C2x spelling 
> `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality to C?
I was also somewhat on the fence, for now I'll change it to specify `1`.

I'll have a look at the C2x changes. Just curious do you know whether there's a 
proposal to add this to C2x?



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It's not allowed to annotate a statement with both ``likely`` and
+``unlikely``.  It's not recommended to annotate both branches of an ``if``
+statement with an attribute.

Quuxplusone wrote:
> aaron.ballman wrote:
> > Why? I would expect this to be reasonable code:
> > ```
> > if (foo) [[likely]] {
> >   ...
> > } else if (bar) [[unlikely]] {
> >   ...
> > } else if (baz) [[likely]] {
> >   ...
> > } else {
> >   ...
> > }
> > ```
> > Especially because I would expect this to be reasonable code:
> > ```
> > switch (value) {
> > [[likely]] case 0: ... break;
> > [[unlikely]] case 1: ... break;
> > [[likely]] case 2: ... break;
> > [[unlikely]] default: ... break;
> > }
> > ```
> > As motivating examples, consider a code generator that knows whether a 
> > particular branch is likely or not and it writes out the attribute on all 
> > branches. Or, something like this:
> > ```
> > float rnd_value = get_super_random_number_between_zero_and_one();
> > if (rnd_value < .1) [[unlikely]] {
> > } else if (rnd_value > .9) [[unlikely]] {
> > } else [[likely]] {
> > }
> > ```
> Right, annotating both/multiple branches of a control statement should be 
> perfectly fine. Even `if (x) [[likely]] { } else [[likely]] { }` should be 
> perfectly okay as far as the code generator is concerned (and we should have 
> a test for that); it's silly, but there's no reason to warn against it in the 
> compiler docs.
> 
> Aaron, notice that `if (x) [[likely]] { } else if (y) [[likely]] { }` is not 
> actually annotating "both" branches of any single `if`-statement. There 
> you're annotating the true branch of an if (without annotating the else 
> branch), and then annotating the true branch of another if (which doesn't 
> have an else branch).
> 
> Mordante, in these docs, please document the "interesting" behavior of the 
> standard attribute on labels — annotating a label is different from 
> annotating the labeled statement itself. In particular,
> 
> [[likely]] case 1: case 2: foo(); break;
> case 3: [[likely]] case 4: foo(); break;
> case 5: case 6: [[likely]] foo(); break;
> 
> indicates that the likely cases are 1, 4, 5, and 6. (1 and 4 because their 
> labels are annotated; 5 and 6 because their control flow passes through an 
> annotated statement. Case 3's control flow passes through an annotated label, 
> but that doesn't matter to the standard attribute.)
Aaron, the current `CodeGen` code for the `switch` statement allows all 
branches to have a weight, for the `if` there are only two weights allowed. As 
Arthur explained the chained `if`s are different statements, so this will work 
properly.

Arthur, I agree we should add the documentation about the `switch`, but I'd 
rather do that when the attributes are implemented for the `switch`. For now I 
think it would make sense to add some documentation about the fact that the 
attribute can be placed inside the `CompoundStmt`.



Comment at: clang/include/clang/Basic/AttrDocs.td:1702
+
+At the moment the attribute is only implemented for an ``if`` statement.
+

aaron.ballman wrote:
> We should document whether we silently accept the attribute in other 
> locations and do nothing with it (I'm not keen on this approach because it 
> surprises users) or that we loudly diagnose the attribute in situations that 
> we don't support it in (I think this is a more user-friendly approach, but 
> may require a separate warning flag).
I'll update the documentation. I intend to continue to work on implementing 
these attributes so I feel adding a temporary diagnostic is not worth the 
effort.



Comment at: clang/include/clang/Basic/AttrDocs.td:1708
+
+  // compile with -Wimplicit-fallthrough
+  if (b) [[likely]] {

aaron.ballman wrote:
> Why does `-Wimplicit-fallthrough` matter?
I

[PATCH] D85559: [MSAN] Reintroduce libatomic load/store instrumentation

[Gui Andrade via Phabricator via cfe-commits]

guiand added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3573
+  case LibFunc_atomic_load:
+if (!isa(CB)) {
+  llvm::errs() << "MSAN -- cannot instrument invoke of libatomic load."

eugenis wrote:
> Probably need the same check for atomic_store.
Although it would be good for consistency I don't think it's strictly needed 
since with `atomic_store` we don't need to use `getNextNode()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85559

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

[PATCH] D85559: [MSAN] Reintroduce libatomic load/store instrumentation

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3573
+  case LibFunc_atomic_load:
+if (!isa(CB)) {
+  llvm::errs() << "MSAN -- cannot instrument invoke of libatomic load."

guiand wrote:
> eugenis wrote:
> > Probably need the same check for atomic_store.
> Although it would be good for consistency I don't think it's strictly needed 
> since with `atomic_store` we don't need to use `getNextNode()`
ah, good point. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85559

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

[PATCH] D85924: [clang][feature] Add cxx_abi_relative_vtable feature

[Roland McGrath via Phabricator via cfe-commits]

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

LGTM but I'm not sure who should sign off on new `__has_feature` symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

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

[PATCH] D85798: Split Preprocessor/init.c test. NFC.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 285745.
tra edited the summary of this revision.
tra added a comment.

Separated blocks of RUN commands with an empty line to make navigation easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85798

Files:
  clang/test/Preprocessor/init-arm.c
  clang/test/Preprocessor/init-mips.c
  clang/test/Preprocessor/init-ppc.c
  clang/test/Preprocessor/init-x86.c
  clang/test/Preprocessor/init.c

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

[PATCH] D85798: Split Preprocessor/init.c test. NFC.

[Artem Belevich 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 rG1689c36b1aeb: Split Preprocessor/init.c test (authored by 
tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85798

Files:
  clang/test/Preprocessor/init-arm.c
  clang/test/Preprocessor/init-mips.c
  clang/test/Preprocessor/init-ppc.c
  clang/test/Preprocessor/init-x86.c
  clang/test/Preprocessor/init.c

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

[PATCH] D85144: [clang] Improve Dumping of APValues

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 285747.
Tyker added a comment.

In D85144#2205461 , @riccibruno wrote:

> I agree with you that it's fine to use `printPretty`  for leaves (and 
> additionally it would be annoying to duplicate the `LValue` case); that's 
> what I was planning to do anyway.
>
> What I am not sure I agree with is the additional complexity to handle the 
> (debugger-only and easy to avoid) case where no context is given.

i don't have a strong opinion on this.
it adds complexity, isn't testable and in many situation where the ASTContext 
is missing the QualType is missing too anyway.
soo i removed that code path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85144

Files:
  clang/include/clang/AST/APValue.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-APValue-LValue.cpp
  clang/test/AST/ast-dump-APValue-MemPtr.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp

Index: clang/test/AST/ast-dump-APValue-todo.cpp
===
--- clang/test/AST/ast-dump-APValue-todo.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// Test without serialization:
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \
-// RUN:-ast-dump %s -ast-dump-filter Test \
-// RUN: | FileCheck --strict-whitespace --match-full-lines %s
-//
-// Test with serialization:
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 -emit-pch -o %t %s
-// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \
-// RUN:   -include-pch %t -ast-dump-all -ast-dump-filter Test /dev/null \
-// RUN: | sed -e "s/ //" -e "s/ imported//" \
-// RUN: | FileCheck --strict-whitespace --match-full-lines %s
-
-int i;
-struct S {
-  int i;
-};
-
-void Test() {
-  constexpr int *pi = &i;
-  // CHECK:  | `-VarDecl {{.*}}  col:{{.*}} pi 'int *const' constexpr cinit
-  // CHECK-NEXT:  |   |-value: LValue 
-
-  constexpr int(S::*pmi) = &S::i;
-  // CHECK:`-VarDecl {{.*}}  col:{{.*}} pmi 'int (S::*const)' constexpr cinit
-  // CHECK-NEXT:  |-value: MemberPointer 
-}
Index: clang/test/AST/ast-dump-APValue-MemPtr.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-APValue-MemPtr.cpp
@@ -0,0 +1,15 @@
+// Test without serialization:
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \
+// RUN:-ast-dump %s -ast-dump-filter Test \
+// RUN: | FileCheck --strict-whitespace %s
+
+int i;
+struct S {
+  int i;
+};
+
+void Test() {
+  constexpr int(S::*pmi) = &S::i;
+  // CHECK:VarDecl {{.*}}  col:{{.*}} pmi 'int (S::*const)' constexpr cinit
+  // CHECK-NEXT:  value: MemberPointer &S::i
+}
Index: clang/test/AST/ast-dump-APValue-LValue.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-APValue-LValue.cpp
@@ -0,0 +1,12 @@
+// Test without serialization:
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \
+// RUN:-ast-dump %s -ast-dump-filter Test \
+// RUN: | FileCheck --strict-whitespace %s
+
+int i;
+
+void Test() {
+  constexpr int *pi = &i;
+  // CHECK:  VarDecl {{.*}}  col:{{.*}} pi 'int *const' constexpr cinit
+  // CHECK-NEXT:  |-value: LValue &i
+}
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -493,7 +493,11 @@
 return;
   case APValue::LValue:
 (void)Context;
-OS << "LValue ";
+OS << "LValue ";
+if (Context)
+  Value.printPretty(OS, Context, Ty);
+else
+  OS << "";
 return;
   case APValue::Array: {
 unsigned ArraySize = Value.getArraySize();
@@ -558,10 +562,12 @@
 return;
   }
   case APValue::MemberPointer:
-OS << "MemberPointer ";
+OS << "MemberPointer ";
+Value.printPretty(OS, Context, Ty);
 return;
   case APValue::AddrLabelDiff:
-OS << "AddrLabelDiff ";
+OS << "AddrLabelDiff ";
+Value.printPretty(OS, Context, Ty);
 return;
   }
   llvm_unreachable("Unknown APValue kind!");
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -388,6 +388,11 @@
 
 void APValue::printPretty(raw_ostream &Out, const ASTContext &Ctx,
   QualType Ty) const {
+  printPretty(Out, &Ctx, Ty);
+}
+
+void APValue::printPretty(raw_ostream &Out, const ASTContext *Ctx,
+  QualType Ty) const {
   switch (getKind()) {
   case APValue::None:
 Out << "";
@@ -426,6 +431,7 @@
 << GetApproxValue(getComplexFloatImag()) << "i";
 return;
   case APValue::LValue: {
+assert(Ctx);
 

[PATCH] D85933: Don't track consteval references for dependent members

[Wyatt Childers via Phabricator via cfe-commits]

wchilders added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:1761
+  llvm::SaveAndRestore DisableIITracking(
+  S.RebuildingImmediateInvocation, true);
+

Tyker wrote:
> I don't think RebuildingImmediateInvocation should be used here since we are 
> not rebuilding immediate invocations
> 
> setSuppressAllDiagnostics or TentativeAnalysisScope seems more adapted.
These don't actually work for this use case due to the queuing nature of the 
diagnostic. Given the hacky nature of this assertion as it stands, I didn't 
want to further pollute the code base.

I'm open to suggestions. If you're proposing making `MarkDeclRefReferenced` not 
queue in response to diagnostic suppression, that doesn't seem totally 
unreasonable; we'll just have to add the additional condition to check the 
diagnostic state in `MarkDeclRefReferenced`.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:131
+  consteval A() = default;
+  consteval ~A() = default; // expected-error {{destructor cannot be declared 
consteval}}
+};

Tyker wrote:
> could you check that this diagnostic and those like it doesn't appear when 
> the struct A isn't instantiated.
I duplicated this namespace without any of the instantiation, and it does seem 
to trigger the diagnostic "destructor cannot be declared consteval" -- all 
other diagnostics are silent. Is that particularly undesirable? 

Also, WRT testing. would that the best option here (having a duplicated 
namespace, "dependent_addressing_uninstantiated" -- without the line triggering 
instantiation)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85933

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