[PATCH] D97138: [Driver] replace argc_ with argc

[Shao-Ce Sun via Phabricator via cfe-commits]

achieveartificialintelligence created this revision.
achieveartificialintelligence added reviewers: MaskRay, jhenderson, aganea, 
richard.barton.arm, sameeranjoshi.
Herald added a reviewer: awarzynski.
achieveartificialintelligence requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Using `int main(int argc, const char **argv)` instead of `int main(int argc_, 
const char **argv_)` to keep pace with other `main()`s.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97138

Files:
  clang/tools/driver/driver.cpp
  flang/tools/flang-driver/driver.cpp

Index: flang/tools/flang-driver/driver.cpp
===
--- flang/tools/flang-driver/driver.cpp
+++ flang/tools/flang-driver/driver.cpp
@@ -66,27 +66,27 @@
   return 1;
 }
 
-int main(int argc_, const char **argv_) {
+int main(int argc, const char **argv) {
 
   // Initialize variables to call the driver
-  llvm::InitLLVM x(argc_, argv_);
-  llvm::SmallVector argv(argv_, argv_ + argc_);
+  llvm::InitLLVM x(argc, argv);
+  llvm::SmallVector Argv(argv, argv + argc);
 
   clang::driver::ParsedClangName targetandMode("flang", "--driver-mode=flang");
-  std::string driverPath = GetExecutablePath(argv[0]);
+  std::string driverPath = GetExecutablePath(Argv[0]);
 
   // Check if flang-new is in the frontend mode
   auto firstArg = std::find_if(
-  argv.begin() + 1, argv.end(), [](const char *a) { return a != nullptr; });
-  if (firstArg != argv.end()) {
-if (llvm::StringRef(argv[1]).startswith("-cc1")) {
-  llvm::errs() << "error: unknown integrated tool '" << argv[1] << "'. "
+  Argv.begin() + 1, Argv.end(), [](const char *a) { return a != nullptr; });
+  if (firstArg != Argv.end()) {
+if (llvm::StringRef(Argv[1]).startswith("-cc1")) {
+  llvm::errs() << "error: unknown integrated tool '" << Argv[1] << "'. "
<< "Valid tools include '-fc1'.\n";
   return 1;
 }
 // Call flang-new frontend
-if (llvm::StringRef(argv[1]).startswith("-fc1")) {
-  return ExecuteFC1Tool(argv);
+if (llvm::StringRef(Argv[1]).startswith("-fc1")) {
+  return ExecuteFC1Tool(Argv);
 }
   }
 
@@ -94,14 +94,14 @@
 
   // Create DiagnosticsEngine for the compiler driver
   llvm::IntrusiveRefCntPtr diagOpts =
-  CreateAndPopulateDiagOpts(argv);
+  CreateAndPopulateDiagOpts(Argv);
   llvm::IntrusiveRefCntPtr diagID(
   new clang::DiagnosticIDs());
   Fortran::frontend::TextDiagnosticPrinter *diagClient =
   new Fortran::frontend::TextDiagnosticPrinter(llvm::errs(), &*diagOpts);
 
   diagClient->set_prefix(
-  std::string(llvm::sys::path::stem(GetExecutablePath(argv[0];
+  std::string(llvm::sys::path::stem(GetExecutablePath(Argv[0];
 
   clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagClient);
 
@@ -110,7 +110,7 @@
   llvm::sys::getDefaultTargetTriple(), diags, "flang LLVM compiler");
   theDriver.setTargetAndMode(targetandMode);
   std::unique_ptr c(
-  theDriver.BuildCompilation(argv));
+  theDriver.BuildCompilation(Argv));
   llvm::SmallVector, 4>
   failingCommands;
 
Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -340,25 +340,25 @@
   return 1;
 }
 
-int main(int argc_, const char **argv_) {
+int main(int argc, const char **argv) {
   noteBottomOfStack();
-  llvm::InitLLVM X(argc_, argv_);
+  llvm::InitLLVM X(argc, argv);
   llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
 " and include the crash backtrace, preprocessed "
 "source, and associated run script.\n");
-  SmallVector argv(argv_, argv_ + argc_);
+  SmallVector Argv(argv, argv + argc);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
 return 1;
 
   llvm::InitializeAllTargets();
-  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
+  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(Argv[0]);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
   // Parse response files using the GNU syntax, unless we're in CL mode. There
-  // are two ways to put clang in CL compatibility mode: argv[0] is either
+  // are two ways to put clang in CL compatibility mode: Argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
   // command line parsing can't happen until after response file parsing, so we
   // have to manually search for a --driver-mode=cl argument the hard way.
@@ -366,20 +366,20 @@
   // response files written by clang will tokenize the same way in either mode.
   bool ClangCLMode = false;
   if (StringRef(TargetAndMode.DriverMode).equals("--driver-mode=cl") ||
-  llvm::find_if(argv, [](const char *F) {
+  llvm::find_if(Argv, [](const char *F) {
 return F && strcmp(F, "--d

[PATCH] D96222: [clang-tidy] Simplify redundant smartptr get check

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96222

Files:
  clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-smartptr-get.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-smartptr-get.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-smartptr-get.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-smartptr-get.cpp
@@ -168,6 +168,42 @@
   // CHECK-FIXES: if (NULL == x);
 }
 
+template 
+void testTemplate() {
+  T().get()->Do();
+}
+
+template 
+void testTemplate2() {
+  std::unique_ptr up;
+  up.get()->Do();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant get() call
+  // CHECK-FIXES: up->Do();
+}
+
+void instantiate() {
+  testTemplate();
+  testTemplate>();
+  testTemplate();
+
+  testTemplate2();
+}
+
+struct S {
+
+  void foo() {
+m_up.get()->Do();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+// CHECK-FIXES: m_up->Do();
+m_bp.get()->Do();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+// CHECK-FIXES: m_bp->Do();
+  }
+
+  std::unique_ptr m_up;
+  BarPtr m_bp;
+};
+
 #define MACRO(p) p.get()
 
 void Negative() {
Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -35,6 +35,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   const bool IgnoreMacros;
Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -18,15 +18,30 @@
 
 namespace {
 internal::Matcher callToGet(const internal::Matcher &OnClass) {
-  return cxxMemberCallExpr(
- on(expr(anyOf(hasType(OnClass),
-   hasType(qualType(
-   pointsTo(decl(OnClass).bind("ptr_to_ptr"))
-.bind("smart_pointer")),
- unless(callee(memberExpr(hasObjectExpression(cxxThisExpr(),
- callee(cxxMethodDecl(
- hasName("get"),
- returns(qualType(pointsTo(type().bind("getType")))
+  return expr(
+ anyOf(cxxMemberCallExpr(
+   on(expr(anyOf(hasType(OnClass),
+ hasType(qualType(pointsTo(
+ decl(OnClass).bind("ptr_to_ptr"))
+  .bind("smart_pointer")),
+   unless(callee(
+   memberExpr(hasObjectExpression(cxxThisExpr(),
+   callee(cxxMethodDecl(hasName("get"),
+returns(qualType(pointsTo(
+type().bind("getType"))),
+   cxxDependentScopeMemberExpr(
+   hasMemberName("get"),
+   hasObjectExpression(
+   expr(hasType(qualType(hasCanonicalType(
+templateSpecializationType(hasDeclaration(
+classTemplateDecl(has(cxxRecordDecl(
+OnClass,
+hasMethod(cxxMethodDecl(
+hasName("get"),
+returns(qualType(
+pointsTo(type().bind(
+"getType")))
+   .bind("smart_pointer")
   .bind("redundant_get");
 }
 
@@ -47,10 +62,9 @@
   const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr);
 
   // Catch 'ptr.get()->Foo()'
-  Finder->addMatcher(
-  memberExpr(expr().bind("memberExpr"), isArrow(),
- hasObjectExpression(ignoringImpCasts(callToGet(Smartptr,
-  Callback);
+  Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
+ 

[PATCH] D96222: Handle uninstantiated templates in redundant get check

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp:21-45
+  return expr(
+ anyOf(cxxMemberCallExpr(
+   on(expr(anyOf(hasType(OnClass),
+ hasType(qualType(pointsTo(
+ decl(OnClass).bind("ptr_to_ptr"))
+  .bind("smart_pointer")),
+   unless(callee(

njames93 wrote:
> The patch says its simplifying the check, but this doesn't look like its 
> simplifying it. Seems to be extending it to support dependent members.
Changed the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96222

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

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874
 /// [C++0x] static_assert-declaration:
 ///   static_assert ( constant-expression  ,  string-literal  ) ;
 ///

aaron.ballman wrote:
> xbolva00 wrote:
> > Do we warn for:
> > 
> > static_assert(“my msg”)? 
> > 
> > I remember some long time ago bug in llvm itself -  assert(“my msg”) - 
> > condition expr was missing.
> We don't, but that would be a reasonable warning to add (not necessarily as 
> part of this patch).
There is a clang-tidy check, misc-static-assert, that will flag asserts like 
that, although it won't flag static asserts of that kind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89065

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

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Seems pretty ok at a first glance. I'll have a deeper look this week.
Please update the revision title to something more comprehensible and move the 
bug link to the summary.




Comment at: clang/unittests/Format/FormatTest.cpp:14257
+
+  Alignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned int*   a;\n"

Could you please test as well mixed pointers and references?



Comment at: clang/unittests/Format/FormatTest.cpp:14260
+   "int*b;\n"
+   "unsigned int Const* c;",
+   Alignment);

Could you please test `const` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

[Nathan James via Phabricator via cfe-commits]

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

Add tests for fixits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++14 %s -fdiagnostics-parseable-fixits %s 2>&1 | 
FileCheck %s
+// Ensure no warnings are emitted in c++17.
+// RUN: %clang_cc1 -std=c++17 %s -Werror -fsyntax-only
+
+static_assert(true && "String");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+// String literal prefixes are good.
+static_assert(true && R"(RawString)");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+static_assert(true && L"RawString");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+
+static_assert(true);
+// CHECK-DAG: {[[@LINE-1]]:19-[[@LINE-1]]:19}:", \"\""
+
+// While its technically possible to transform this to
+// static_assert(true, "String") we don't attempt this fix.
+static_assert("String" && true);
+// CHECK-DAG: {[[@LINE-1]]:31-[[@LINE-1]]:31}:", \"\""
+
+// Don't be smart and look in parentheses.
+static_assert((true && "String"));
+// CHECK-DAG: {[[@LINE-1]]:33-[[@LINE-1]]:33}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 
static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -892,12 +902,11 @@
 
   ExprResult AssertMessage;
   if (Tok.is(tok::r_paren)) {
-Diag(Tok, getLangOpts().CPlusPlus17
-  ? diag::warn_cxx14_compat_static_assert_no_message
-  : diag::ext_static_assert_no_message)
-  << (getLangOpts().CPlusPlus17
-  ? FixItHint()
-  : FixItHint::CreateInsertion(Tok.getLocation(), ", \"\""));
+if (getLangOpts().CPlusPlus17)
+  Diag(Tok, diag::warn_cxx14_compat_static_assert_no_message);
+else
+  Diag(Tok, diag::ext_static_assert_no_message)
+  << getStaticAssertNoMessageFixIt(AssertExpr.get(), 
Tok.getLocation());
   } else {
 if (ExpectAndConsume(tok::comma)) {
   SkipUntil(tok::semi);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++14 %s -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// Ensure no warnings are emitted in c++17.
+// RUN: %clang_cc1 -std=c++17 %s -Werror -fsyntax-only
+
+static_assert(true && "String");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+// String literal prefixes are good.
+static_assert(true && R"(RawString)");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+static_assert(true && L"RawString");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+
+static_assert(true);
+// CHECK-DAG: {[[@LINE-1]]:19-[[@LINE-1]]:19}:", \"\""
+
+// While its technically possible to transform this to
+// static_assert(true, "String") we don't attempt this fix.
+static_assert("String" && true);
+// CHECK-DAG: {[[@LINE-1]]:31-[[@LINE-1]]:31}:", \"\""
+
+// Don't be smart and look in parentheses.
+static_assert((true && "String"));
+// CHECK-DAG: {[[@LINE-1]]:33-[[@LINE-1]]:33}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -892,12 +902,11 @@
 
   ExprResu

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

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, steveire.
Herald added subscribers: usaxena95, kadircet, arphaman, kbarton, xazax.hun, 
nemanjai.
njames93 updated this revision to Diff 325234.
njames93 added a comment.
njames93 updated this revision to Diff 325238.
njames93 updated this revision to Diff 325239.
njames93 published this revision for review.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

Fix test


njames93 added a comment.

Disable formatting causing test to fail and document it.


njames93 added a comment.

Remove unintended change.


Adds a flag to ClangTidyContext that is used to indicate to checks that fixes 
will only be applied one at a time.
This is to indicate to checks that each fix emitted should not depend on any 
other fixes emitted across the translation unit.
I've currently implemented the IncludeInserter and PreferMemberInitializerCheck 
to use these support these modes.

Reasoning behind this is use cases like clangd its only possible to apply one 
fix at a time.
For include inserter checks, the include is only added once for the first 
diagnostic that requires it, this will result in subsequent fixes not having 
the included needed.

A similar issue is seen in the PreferMemberInitializerCheck where the `:` will 
only be added for the first member that needs fixing.

Fixes emitted in SingleFixMode will likely result in malformed code if they are 
applied all together, conversely fixes currently emitted may result in 
malformed code if they are applied one at a time.
For this reason invoking clang-tidy from the binary will always with 
SingleFixMode disabled, However using it as a library its possible to select 
the mode you wish to use, clangd always selects SingleFixMode.

This is an example of the current behaviour failing

  struct Foo {
int A, B;
Foo(int D, int E) {
  A = D;
  B = E; // Fix Here
}
  };

Incorrectly transformed to:

  struct Foo {
int A, B;
Foo(int D, int E), B(E) {
  A = D;
   // Fix Here
}
  };

In SingleFixMode, it gets transformed to:

  struct Foo {
int A, B;
Foo(int D, int E) : B(E) {
  A = D;
   // Fix Here
}
  };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97121

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeInserter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -30,8 +30,9 @@
 public:
   IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context,
utils::IncludeSorter::IncludeStyle Style =
-   utils::IncludeSorter::IS_Google)
-  : ClangTidyCheck(CheckName, Context), Inserter(Style) {}
+   utils::IncludeSorter::IS_Google,
+   bool SingleFixMode = false)
+  : ClangTidyCheck(CheckName, Context), Inserter(Style, SingleFixMode) {}
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override {
@@ -85,6 +86,18 @@
   }
 };
 
+class MultipleHeaderSingleInserterCheck : public IncludeInserterCheckBase {
+public:
+  MultipleHeaderSingleInserterCheck(StringRef CheckName,
+ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google, true) {}
+
+  std::vector headersToInclude() const override {
+return {"path/to/header.h", "p

[PATCH] D96853: [clang][AVR] Support variable decorator '__flash'

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 325285.

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

https://reviews.llvm.org/D96853

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/address-space-avr.c
  clang/test/CodeGen/avr-flash.c


Index: clang/test/CodeGen/avr-flash.c
===
--- /dev/null
+++ clang/test/CodeGen/avr-flash.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s
+
+static __flash int arr[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 
'const' needed for variables in address space '__flash'}}
+
+int foo(int i, int j) {
+  static __flash int b[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 
'const' needed for variables in address space '__flash'}}
+  return arr[i] + b[j];
+}
Index: clang/test/CodeGen/address-space-avr.c
===
--- clang/test/CodeGen/address-space-avr.c
+++ clang/test/CodeGen/address-space-avr.c
@@ -3,10 +3,16 @@
 // Test that function declarations in nonzero address spaces without prototype
 // are called correctly.
 
+// CHECK: @var0 {{.*}} addrspace(1) constant i16 333
+// CHECK: @bar.var1 {{.*}} addrspace(1) constant i16 555
 // CHECK: define{{.*}} void @bar() addrspace(1)
 // CHECK: call addrspace(1) void bitcast (void (...) addrspace(1)* @foo to 
void (i16) addrspace(1)*)(i16 3)
 // CHECK: declare void @foo(...) addrspace(1)
+
+__flash const int var0 = 333;
+
 void foo();
 void bar(void) {
+   static __flash const int var1 = 555;
foo(3);
 }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/DiagnosticFrontend.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/SmallBitVector.h"
@@ -8032,6 +8033,19 @@
   AVRTargetCodeGenInfo(CodeGenTypes &CGT)
   : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
+  LangAS getGlobalVarAddressSpace(CodeGenModule &CGM,
+  const VarDecl *D) const override {
+// Check if a global/static variable is defined within address space 1
+// but not constant.
+LangAS AS = D->getType().getAddressSpace();
+if (isTargetAddressSpace(AS) && toTargetAddressSpace(AS) == 1 &&
+!D->getType().isConstQualified())
+  CGM.getDiags().Report(D->getLocation(),
+diag::err_qualifier_with_address_space)
+  << 0 << 0 << "__flash";
+return TargetCodeGenInfo::getGlobalVarAddressSpace(CGM, D);
+  }
+
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
 if (GV->isDeclaration())
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -308,6 +308,7 @@
   Builder.defineMacro("__AVR");
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
+  Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
 
   if (!this->CPU.empty()) {
 auto It = llvm::find_if(
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2991,6 +2991,9 @@
   "parameter may not be qualified with an address space">;
 def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;
+def err_qualifier_with_address_space : Error<
+  "qualifier '%select{const|volatile}0' %select{needed|invalid}1 "
+  "for variables in address space '%2'">;
 def err_compound_literal_with_address_space : Error<
   "compound literal in function scope may not be qualified with an address 
space">;
 def err_address_space_mismatch_templ_inst : Error<


Index: clang/test/CodeGen/avr-flash.c
===
--- /dev/null
+++ clang/test/CodeGen/avr-flash.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s
+
+static __flash int arr[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 'const' needed for variables in address space '__flash'}}
+
+int foo(int i, int j) {
+  static __flash int b[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 'const' needed for variables in address space '__flash'}}
+  return arr[i] + b[j];
+}
Index: clang/test/CodeGen/address-space-avr.c
===
--- clang/test/CodeGen/address-space-avr.c
+++ clang/test/Cod

[PATCH] D96853: [clang][AVR] Support variable decorator '__flash'

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 325286.

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

https://reviews.llvm.org/D96853

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/address-space-avr.c
  clang/test/CodeGen/avr-flash.c


Index: clang/test/CodeGen/avr-flash.c
===
--- /dev/null
+++ clang/test/CodeGen/avr-flash.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s
+
+static __flash int arr[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 
'const' needed for variables in address space '__flash'}}
+
+int foo(int i, int j) {
+  static __flash int b[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 
'const' needed for variables in address space '__flash'}}
+  return arr[i] + b[j];
+}
Index: clang/test/CodeGen/address-space-avr.c
===
--- clang/test/CodeGen/address-space-avr.c
+++ clang/test/CodeGen/address-space-avr.c
@@ -1,12 +1,15 @@
 // RUN: %clang_cc1 -triple avr -emit-llvm < %s | FileCheck %s
 
-// Test that function declarations in nonzero address spaces without prototype
-// are called correctly.
-
+// CHECK: @var0 {{.*}} addrspace(1) constant i16 333
+// CHECK: @bar.var1 {{.*}} addrspace(1) constant i16 555
 // CHECK: define{{.*}} void @bar() addrspace(1)
 // CHECK: call addrspace(1) void bitcast (void (...) addrspace(1)* @foo to 
void (i16) addrspace(1)*)(i16 3)
 // CHECK: declare void @foo(...) addrspace(1)
+
+__flash const int var0 = 333;
+
 void foo();
 void bar(void) {
+   static __flash const int var1 = 555;
foo(3);
 }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/DiagnosticFrontend.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/SmallBitVector.h"
@@ -8032,6 +8033,19 @@
   AVRTargetCodeGenInfo(CodeGenTypes &CGT)
   : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
+  LangAS getGlobalVarAddressSpace(CodeGenModule &CGM,
+  const VarDecl *D) const override {
+// Check if a global/static variable is defined within address space 1
+// but not constant.
+LangAS AS = D->getType().getAddressSpace();
+if (isTargetAddressSpace(AS) && toTargetAddressSpace(AS) == 1 &&
+!D->getType().isConstQualified())
+  CGM.getDiags().Report(D->getLocation(),
+diag::err_qualifier_with_address_space)
+  << 0 << 0 << "__flash";
+return TargetCodeGenInfo::getGlobalVarAddressSpace(CGM, D);
+  }
+
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
 if (GV->isDeclaration())
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -308,6 +308,7 @@
   Builder.defineMacro("__AVR");
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
+  Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
 
   if (!this->CPU.empty()) {
 auto It = llvm::find_if(
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2991,6 +2991,9 @@
   "parameter may not be qualified with an address space">;
 def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;
+def err_qualifier_with_address_space : Error<
+  "qualifier '%select{const|volatile}0' %select{needed|invalid}1 "
+  "for variables in address space '%2'">;
 def err_compound_literal_with_address_space : Error<
   "compound literal in function scope may not be qualified with an address 
space">;
 def err_address_space_mismatch_templ_inst : Error<


Index: clang/test/CodeGen/avr-flash.c
===
--- /dev/null
+++ clang/test/CodeGen/avr-flash.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s
+
+static __flash int arr[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 'const' needed for variables in address space '__flash'}}
+
+int foo(int i, int j) {
+  static __flash int b[] = {4, 6, 1, 2, 5}; // expected-error {{qualifier 'const' needed for variables in address space '__flash'}}
+  return arr[i] + b[j];
+}
Index: clang/test/CodeGen/address-space-avr.c

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

[Nathan James via Phabricator via cfe-commits]

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

Add recompile test to ensure fix-its are correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++14 %s -fdiagnostics-parseable-fixits %s 2>&1 | 
FileCheck %s
+// Ensure no warnings are emitted in c++17.
+// RUN: %clang_cc1 -std=c++17 %s -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror
+
+static_assert(true && "String");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+// String literal prefixes are good.
+static_assert(true && R"(RawString)");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+static_assert(true && L"RawString");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+
+static_assert(true);
+// CHECK-DAG: {[[@LINE-1]]:19-[[@LINE-1]]:19}:", \"\""
+
+// While its technically possible to transform this to
+// static_assert(true, "String") we don't attempt this fix.
+static_assert("String" && true);
+// CHECK-DAG: {[[@LINE-1]]:31-[[@LINE-1]]:31}:", \"\""
+
+// Don't be smart and look in parentheses.
+static_assert((true && "String"));
+// CHECK-DAG: {[[@LINE-1]]:33-[[@LINE-1]]:33}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 
static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -892,12 +902,11 @@
 
   ExprResult AssertMessage;
   if (Tok.is(tok::r_paren)) {
-Diag(Tok, getLangOpts().CPlusPlus17
-  ? diag::warn_cxx14_compat_static_assert_no_message
-  : diag::ext_static_assert_no_message)
-  << (getLangOpts().CPlusPlus17
-  ? FixItHint()
-  : FixItHint::CreateInsertion(Tok.getLocation(), ", \"\""));
+if (getLangOpts().CPlusPlus17)
+  Diag(Tok, diag::warn_cxx14_compat_static_assert_no_message);
+else
+  Diag(Tok, diag::ext_static_assert_no_message)
+  << getStaticAssertNoMessageFixIt(AssertExpr.get(), 
Tok.getLocation());
   } else {
 if (ExpectAndConsume(tok::comma)) {
   SkipUntil(tok::semi);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++14 %s -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// Ensure no warnings are emitted in c++17.
+// RUN: %clang_cc1 -std=c++17 %s -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror
+
+static_assert(true && "String");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+// String literal prefixes are good.
+static_assert(true && R"(RawString)");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+static_assert(true && L"RawString");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+
+static_assert(true);
+// CHECK-DAG: {[[@LINE-1]]:19-[[@LINE-1]]:19}:", \"\""
+
+// While its technically possible to transform this to
+// static_assert(true, "String") we don't attempt this fix.
+static_assert("String" && true);
+// CHECK-DAG: {[[@LINE-1]]:31-[[@LINE-1]]:31}:", \"\""
+
+// Don't be smart and look in parentheses.
+static_assert((true && "String"));
+// CHECK-DAG: {[[@LINE-1]]:33-[[@LINE-1]]:33}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndEx

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

[Stephen Kelly via Phabricator via cfe-commits]

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

Fix handling of default construction where the constructor has a default arg.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97142

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -46,6 +46,11 @@
   T();
 }
 
+struct CtorDefaultArg {
+  CtorDefaultArg(int i = 0);
+  ~CtorDefaultArg();
+};
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
@@ -64,6 +69,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
 // CHECK-FIXES: FooBar give_me_a_name;
 
+  CtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: CtorDefaultArg give_me_a_name;
+
   templ();
   templ();
 
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
@@ -28,6 +28,9 @@
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,27 +25,16 @@
 
 void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
   // Look for temporaries that are constructed in-place and immediately
-  // destroyed. Look for temporaries created by a functional cast but not for
-  // those returned from a call.
-  auto BindTemp = cxxBindTemporaryExpr(
-  unless(has(ignoringParenImpCasts(callExpr(,
-  unless(has(ignoringParenImpCasts(objcMessageExpr()
-  .bind("temp");
+  // destroyed.
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   exprWithCleanups(
-   unless(isInTemplateInstantiation()),
-   hasParent(compoundStmt().bind("compound")),
-   hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-   anyOf(has(ignoringParenImpCasts(BindTemp)),
- has(ignoringParenImpCasts(cxxFunctionalCastExpr(
- has(ignoringParenImpCasts(BindTemp)))
-   .bind("expr")),
+  cxxConstructExpr(hasParent(compoundStmt().bind("compound")),
+   hasType(cxxRecordDecl(hasNonTrivialDestructor(
+  .bind("expr"),
   this);
 }
 
 void UnusedRaiiCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *E = Result.Nodes.getNodeAs("expr");
+  const auto *E = Result.Nodes.getNodeAs("expr");
 
   // We ignore code expanded from macros to reduce the number of false
   // positives.
@@ -55,7 +44,8 @@
   // Don't emit a warning for the last statement in the surrounding compound
   // statement.
   const auto *CS = Result.Nodes.getNodeAs("compound");
-  if (E == CS->body_back())
+  const auto *LastExpr = dyn_cast(CS->body_back());
+  if (LastExpr && E == LastExpr->IgnoreUnlessSpelledInSource())
 return;
 
   // Emit a warning.
@@ -65,25 +55,17 @@
 
   // If this is a default ctor we have to remove the parens or we'll introduce a
   // most vexing parse.
-  const auto *BTE = Result.Nodes.getNodeAs("temp");
-  if (const auto *TOE = dyn_cast(BTE->getSubExpr()))
-if (TOE->getNumArgs() == 0) {
-  D << FixItHint::CreateReplacement(
-  CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()),
-  Replacement);
-  return;
-}
+  if (E->getNumArgs() == 0 || isa(E->getArg(0))) {
+D << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(E->getParenOrBraceRange()), Replacement);
+return;
+  }
 
   // Otherwise just suggest adding a name. To find the place to insert the name
   // find the first TypeLoc in the children of E, which always points to the
   // wri

[PATCH] D97144: [clang-tidy] Simplify shrink to fit check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97144

Files:
  clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h


Index: clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
+++ clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
@@ -30,6 +30,9 @@
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
@@ -21,31 +21,24 @@
 void ShrinkToFitCheck::registerMatchers(MatchFinder *Finder) {
   // Swap as a function need not to be considered, because rvalue can not
   // be bound to a non-const reference.
-  const auto ShrinkableAsMember =
-  memberExpr(member(valueDecl().bind("ContainerDecl")));
-  const auto ShrinkableAsDecl =
-  declRefExpr(hasDeclaration(valueDecl().bind("ContainerDecl")));
-  const auto CopyCtorCall = cxxConstructExpr(hasArgument(
-  0, anyOf(ShrinkableAsMember, ShrinkableAsDecl,
-   unaryOperator(has(ignoringParenImpCasts(ShrinkableAsMember))),
-   unaryOperator(has(ignoringParenImpCasts(ShrinkableAsDecl));
-  const auto SwapParam =
-  expr(anyOf(memberExpr(member(equalsBoundNode("ContainerDecl"))),
- declRefExpr(hasDeclaration(equalsBoundNode("ContainerDecl"))),
- unaryOperator(has(ignoringParenImpCasts(
- memberExpr(member(equalsBoundNode("ContainerDecl")),
- unaryOperator(has(ignoringParenImpCasts(declRefExpr(
- hasDeclaration(equalsBoundNode("ContainerDecl";
+  const auto ShrinkableExpr = mapAnyOf(memberExpr, declRefExpr);
+  const auto Shrinkable =
+  ShrinkableExpr.with(hasDeclaration(valueDecl().bind("ContainerDecl")));
+  const auto BoundShrinkable = ShrinkableExpr.with(
+  hasDeclaration(valueDecl(equalsBoundNode("ContainerDecl";
 
   Finder->addMatcher(
   cxxMemberCallExpr(
-  on(hasType(hasCanonicalType(hasDeclaration(namedDecl(
-  hasAnyName("std::basic_string", "std::deque", 
"std::vector")),
   callee(cxxMethodDecl(hasName("swap"))),
-  has(ignoringParenImpCasts(
-  memberExpr(traverse(TK_AsIs, hasDescendant(CopyCtorCall),
-  hasArgument(0, SwapParam.bind("ContainerToShrink")),
-  unless(isInTemplateInstantiation()))
+  hasArgument(
+  0, anyOf(Shrinkable, 
unaryOperator(hasUnaryOperand(Shrinkable,
+  on(cxxConstructExpr(hasArgument(
+  0,
+  expr(anyOf(BoundShrinkable,
+ unaryOperator(hasUnaryOperand(BoundShrinkable))),
+   
hasType(hasCanonicalType(hasDeclaration(namedDecl(hasAnyName(
+   "std::basic_string", "std::deque", "std::vector"))
+  .bind("ContainerToShrink")
   .bind("CopyAndSwapTrick"),
   this);
 }


Index: clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
+++ clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.h
@@ -30,6 +30,9 @@
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ShrinkToFitCheck.cpp
@@ -21,31 +21,24 @@
 void ShrinkToFitCheck::registerMatchers(MatchFinder *Finder) {
   // Swap as a function need not to be considered, because rvalue can not
   // be bound to a non-const reference.
-  const auto ShrinkableAsMember =
-  memberExpr(member(valueDecl().bind("ContainerDecl")));
-  const auto ShrinkableAsDecl =
-  declRefExpr(hasDeclaration(valueDecl().bind("ContainerDecl")));
-  const auto CopyCtorCall = 

[PATCH] D97145: [clang-tidy] Simplify default member init check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97145

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h


Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -30,6 +30,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   void checkDefaultInit(const ast_matchers::MatchFinder::MatchResult &Result,
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -207,14 +207,13 @@
 declRefExpr(to(enumConstantDecl(;
 
   auto Init =
-  anyOf(initListExpr(anyOf(
-allOf(initCountIs(1), hasInit(0, ignoringImplicit(InitBase))),
-initCountIs(0))),
+  anyOf(initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, InitBase)),
+   initCountIs(0))),
 InitBase);
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(), unless(isInstantiated()),
+  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20
@@ -222,18 +221,15 @@
 : isBitField(),
 hasInClassInitializer(anything()),
 hasParent(recordDecl(isUnion()),
-  isWritten(), withInitializer(ignoringImplicit(Init)))
+  withInitializer(Init))
   .bind("default"))),
   this);
 
   Finder->addMatcher(
-  cxxConstructorDecl(
-  unless(ast_matchers::isTemplateInstantiation()),
-  forEachConstructorInitializer(
-  cxxCtorInitializer(forField(hasInClassInitializer(anything())),
- isWritten(),
- withInitializer(ignoringImplicit(Init)))
-  .bind("existing"))),
+  cxxConstructorDecl(forEachConstructorInitializer(
+  cxxCtorInitializer(forField(hasInClassInitializer(anything())),
+ withInitializer(Init))
+  .bind("existing"))),
   this);
 }
 


Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -30,6 +30,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   void checkDefaultInit(const ast_matchers::MatchFinder::MatchResult &Result,
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -207,14 +207,13 @@
 declRefExpr(to(enumConstantDecl(;
 
   auto Init =
-  anyOf(initListExpr(anyOf(
-allOf(initCountIs(1), hasInit(0, ignoringImplicit(InitBase))),
-initCountIs(0))),
+  anyOf(initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, InitBase)),
+   initCountIs(0))),
 InitBase);
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(), unless(isInstantiated()),
+  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20
@@ -222,18 +221,15 @@
 : isBitField(),
 has

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

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp:49-53
+struct CtorDefaultArg {
+  CtorDefaultArg(int i = 0);
+  ~CtorDefaultArg();
+};
+

Its not obvious from the code, but this should work if there are multiple 
default args. Can that behaviour be demonstrated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97142

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

[PATCH] D97147: [clang-tidy] Simplify redundant member init check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97147

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h


Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -32,6 +32,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool IgnoreBaseInCopyConstructors;
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -26,26 +26,21 @@
 }
 
 void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
-  auto Construct =
-  cxxConstructExpr(
-  hasDeclaration(cxxConstructorDecl(hasParent(
-  cxxRecordDecl(unless(isTriviallyDefaultConstructible()))
-  .bind("construct");
-
   Finder->addMatcher(
-  traverse(
-  TK_AsIs,
-  cxxConstructorDecl(
-  unless(isDelegatingConstructor()),
-  ofClass(unless(
-  anyOf(isUnion(), ast_matchers::isTemplateInstantiation(,
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  isWritten(), 
withInitializer(ignoringImplicit(Construct)),
-  unless(forField(hasType(isConstQualified(,
-  unless(forField(hasParent(recordDecl(isUnion())
-  .bind("init")))
-  .bind("constructor")),
+  cxxConstructorDecl(
+  unless(isDelegatingConstructor()), ofClass(unless(isUnion())),
+  forEachConstructorInitializer(
+  cxxCtorInitializer(
+  withInitializer(
+  cxxConstructExpr(
+  hasDeclaration(
+  cxxConstructorDecl(ofClass(cxxRecordDecl(
+  
unless(isTriviallyDefaultConstructible()))
+  .bind("construct")),
+  unless(forField(hasType(isConstQualified(,
+  unless(forField(hasParent(recordDecl(isUnion())
+  .bind("init")))
+  .bind("constructor"),
   this);
 }
 


Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -32,6 +32,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool IgnoreBaseInCopyConstructors;
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -26,26 +26,21 @@
 }
 
 void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
-  auto Construct =
-  cxxConstructExpr(
-  hasDeclaration(cxxConstructorDecl(hasParent(
-  cxxRecordDecl(unless(isTriviallyDefaultConstructible()))
-  .bind("construct");
-
   Finder->addMatcher(
-  traverse(
-  TK_AsIs,
-  cxxConstructorDecl(
-  unless(isDelegatingConstructor()),
-  ofClass(unless(
-  anyOf(isUnion(), ast_matchers::isTemplateInstantiation(,
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  isWritten(), withInitializer(ignoringImplicit(Construct)),
-  unless(forField(hasType(isConstQualified(,
-  unless(forField(hasParent(recordDecl(isUnion())
-  .bind("init")))
-   

[PATCH] D97149: [clang-tidy] Simplify suspicious enum usage check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97149

Files:
  clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h


Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
@@ -25,6 +25,9 @@
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   void checkSuspiciousBitmaskUsage(const Expr*, const EnumDecl*);
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
@@ -118,30 +118,28 @@
 
 void SuspiciousEnumUsageCheck::registerMatchers(MatchFinder *Finder) {
   const auto EnumExpr = [](StringRef RefName, StringRef DeclName) {
-return expr(ignoringImpCasts(expr().bind(RefName)),
-ignoringImpCasts(hasType(enumDecl().bind(DeclName;
+return expr(hasType(enumDecl().bind(DeclName))).bind(RefName);
   };
 
   Finder->addMatcher(
-  binaryOperator(hasOperatorName("|"), hasLHS(EnumExpr("", "enumDecl")),
- hasRHS(expr(EnumExpr("", "otherEnumDecl"),
- ignoringImpCasts(hasType(enumDecl(
- unless(equalsBoundNode("enumDecl"
+  binaryOperator(
+  hasOperatorName("|"), hasLHS(hasType(enumDecl().bind("enumDecl"))),
+  hasRHS(hasType(enumDecl(unless(equalsBoundNode("enumDecl")))
+ .bind("otherEnumDecl"
   .bind("diffEnumOp"),
   this);
 
   Finder->addMatcher(
   binaryOperator(hasAnyOperatorName("+", "|"),
  hasLHS(EnumExpr("lhsExpr", "enumDecl")),
- hasRHS(expr(EnumExpr("rhsExpr", ""),
- ignoringImpCasts(hasType(
- 
enumDecl(equalsBoundNode("enumDecl"))),
+ 
hasRHS(expr(hasType(enumDecl(equalsBoundNode("enumDecl"
+.bind("rhsExpr"))),
   this);
 
   Finder->addMatcher(
   binaryOperator(
   hasAnyOperatorName("+", "|"),
-  hasOperands(expr(hasType(isInteger()), unless(EnumExpr("", ""))),
+  hasOperands(expr(hasType(isInteger()), unless(hasType(enumDecl(,
   EnumExpr("enumExpr", "enumDecl"))),
   this);
 


Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.h
@@ -25,6 +25,9 @@
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   void checkSuspiciousBitmaskUsage(const Expr*, const EnumDecl*);
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
@@ -118,30 +118,28 @@
 
 void SuspiciousEnumUsageCheck::registerMatchers(MatchFinder *Finder) {
   const auto EnumExpr = [](StringRef RefName, StringRef DeclName) {
-return expr(ignoringImpCasts(expr().bind(RefName)),
-ignoringImpCasts(hasType(enumDecl().bind(DeclName;
+return expr(hasType(enumDecl().bind(DeclName))).bind(RefName);
   };
 
   Finder->addMatcher(
-  binaryOperator(hasOperatorName("|"), hasLHS(EnumExpr("", "enumDecl")),
- hasRHS(expr(EnumExpr("", "otherEnumDecl"),
- ignoringImpCasts(hasType(enumDecl(
- unless(equalsBoundNode("enumDecl"
+  binaryOperator(
+  hasOperatorName("|"), hasLHS(hasType(enumDecl().bind("enumDecl"))),
+  hasRHS(ha

[PATCH] D97150: [clang-tidy] Simplify suspicious memset usage check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97150

Files:
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h


Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
@@ -25,6 +25,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
@@ -35,27 +35,24 @@
   callee(MemsetDecl),
   hasArgument(1, characterLiteral(equals(static_cast('0')))
  .bind("char-zero-fill")),
-  unless(
-  eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())),
-  hasType(arrayType(hasElementType(
-  isAnyCharacter()),
- isInTemplateInstantiation(,
+  unless(hasArgument(
+  0, anyOf(hasType(pointsTo(isAnyCharacter())),
+   
hasType(arrayType(hasElementType(isAnyCharacter(,
   this);
 
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
-  Finder->addMatcher(callExpr(callee(MemsetDecl),
-  hasArgument(1, 
integerLiteral().bind("num-fill")),
-  unless(isInTemplateInstantiation())),
- this);
+  Finder->addMatcher(
+  callExpr(callee(MemsetDecl),
+   hasArgument(1, integerLiteral().bind("num-fill"))),
+  this);
 
   // Look for memset(x, y, 0) as that is most likely an argument swap.
   Finder->addMatcher(
   callExpr(callee(MemsetDecl),
unless(hasArgument(1, anyOf(characterLiteral(equals(
static_cast('0'))),
-   integerLiteral(,
-   unless(isInTemplateInstantiation()))
+   integerLiteral()
   .bind("call"),
   this);
 }


Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h
@@ -25,6 +25,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
@@ -35,27 +35,24 @@
   callee(MemsetDecl),
   hasArgument(1, characterLiteral(equals(static_cast('0')))
  .bind("char-zero-fill")),
-  unless(
-  eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())),
-  hasType(arrayType(hasElementType(
-  isAnyCharacter()),
- isInTemplateInstantiation(,
+  unless(hasArgument(
+  0, anyOf(hasType(pointsTo(isAnyCharacter())),
+   hasType(arrayType(hasElementType(isAnyCharacter(,
   this);
 
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
-  Finder->addMatcher(callExpr(callee(MemsetDecl),
-  hasArgument(1, integerLiteral().bind("num-fill")),
-  unless(isInTemplateInstantiation())),
- this);
+  Finder->addMatcher(
+  callExpr(c

[PATCH] D97151: [clang-tidy] Simplify redundant branch condition check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97151

Files:
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h


Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
@@ -26,6 +26,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -48,23 +48,23 @@
   .bind(CondVarStr);
   Finder->addMatcher(
   ifStmt(
-  hasCondition(ignoringParenImpCasts(anyOf(
+  hasCondition(anyOf(
   declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
-  binaryOperator(hasOperatorName("&&"),
- hasEitherOperand(ignoringParenImpCasts(
- declRefExpr(hasDeclaration(ImmutableVar))
- .bind(OuterIfVar2Str))),
+  binaryOperator(
+  hasOperatorName("&&"),
+  hasEitherOperand(declRefExpr(hasDeclaration(ImmutableVar))
+   .bind(OuterIfVar2Str),
   hasThen(hasDescendant(
-  ifStmt(hasCondition(ignoringParenImpCasts(
- anyOf(declRefExpr(hasDeclaration(varDecl(
-equalsBoundNode(CondVarStr
-.bind(InnerIfVar1Str),
-   binaryOperator(
-   hasAnyOperatorName("&&", "||"),
-   hasEitherOperand(ignoringParenImpCasts(
-   declRefExpr(hasDeclaration(varDecl(
+  ifStmt(hasCondition(anyOf(
+ declRefExpr(hasDeclaration(
+ varDecl(equalsBoundNode(CondVarStr
+ .bind(InnerIfVar1Str),
+ binaryOperator(
+ hasAnyOperatorName("&&", "||"),
+ hasEitherOperand(
+ declRefExpr(hasDeclaration(varDecl(
  equalsBoundNode(CondVarStr
- .bind(InnerIfVar2Str
+ .bind(InnerIfVar2Str))
   .bind(InnerIfStr))),
   forFunction(functionDecl().bind(FuncStr)))
   .bind(OuterIfStr),


Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.h
@@ -26,6 +26,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -48,23 +48,23 @@
   .bind(CondVarStr);
   Finder->addMatcher(
   ifStmt(
-  hasCondition(ignoringParenImpCasts(anyOf(
+  hasCondition(anyOf(
   declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
-  binaryOperator(hasOperatorName("&&"),
- hasEitherOperand(ignoringParenImpCasts(
- declRefExpr(hasDeclaration(ImmutableVar))
- .bind(OuterIfVar2Str))),
+  binaryOperator(
+  hasO

[PATCH] D97152: [clang-tidy] Simplify special member functions check

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97152

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h


Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -32,7 +32,9 @@
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void onEndOfTranslationUnit() override;
-
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
   enum class SpecialMemberFunctionKind : uint8_t {
 Destructor,
 DefaultDestructor,
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -40,18 +40,13 @@
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   cxxRecordDecl(
-  eachOf(
-  has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
-  has(cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))
-  .bind("copy-ctor")),
-  has(cxxMethodDecl(isCopyAssignmentOperator(),
-unless(isImplicit()))
-  .bind("copy-assign")),
-  has(cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))
-  .bind("move-ctor")),
-  has(cxxMethodDecl(isMoveAssignmentOperator(),
-unless(isImplicit()))
-  .bind("move-assign"
+  eachOf(has(cxxDestructorDecl().bind("dtor")),
+ 
has(cxxConstructorDecl(isCopyConstructor()).bind("copy-ctor")),
+ has(cxxMethodDecl(isCopyAssignmentOperator())
+ .bind("copy-assign")),
+ 
has(cxxConstructorDecl(isMoveConstructor()).bind("move-ctor")),
+ has(cxxMethodDecl(isMoveAssignmentOperator())
+ .bind("move-assign"
   .bind("class-def"),
   this);
 }


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -32,7 +32,9 @@
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void onEndOfTranslationUnit() override;
-
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
   enum class SpecialMemberFunctionKind : uint8_t {
 Destructor,
 DefaultDestructor,
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -40,18 +40,13 @@
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   cxxRecordDecl(
-  eachOf(
-  has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
-  has(cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))
-  .bind("copy-ctor")),
-  has(cxxMethodDecl(isCopyAssignmentOperator(),
-unless(isImplicit()))
-  .bind("copy-assign")),
-  has(cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))
-  .bind("move-ctor")),
-  has(cxxMethodDecl(isMoveAssignmentOperator(),
-unless(isImplicit()))
-  .bind("move-assign"
+  eachOf(has(cxxDestructorDecl().bind("dtor")),
+ has(cxxConstructorDecl(isCopyConstructor()).bind("copy-ctor")),
+ has(cxxMethodDecl(isCopyAssignmentOperator())
+ .bind("copy-assign")),
+ has(cxxConstructorDec

[PATCH] D97153: [clang-tidy] Simplify boolean expr scheck

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97153

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -27,6 +27,9 @@
   void storeOptions(ClangTidyOptions::OptionMap &Options) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   class Visitor;
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -71,10 +71,10 @@
 }
 
 internal::BindableMatcher literalOrNegatedBool(bool Value) {
-  return expr(anyOf(cxxBoolLiteral(equals(Value)),
-unaryOperator(hasUnaryOperand(ignoringParenImpCasts(
-  cxxBoolLiteral(equals(!Value,
-  hasOperatorName("!";
+  return expr(
+  anyOf(cxxBoolLiteral(equals(Value)),
+unaryOperator(hasUnaryOperand(cxxBoolLiteral(equals(!Value))),
+  hasOperatorName("!";
 }
 
 internal::Matcher returnsBool(bool Value, StringRef Id = "ignored") {
@@ -443,8 +443,7 @@
   bool Value,
   StringRef BooleanId) {
   Finder->addMatcher(
-  ifStmt(unless(isInTemplateInstantiation()),
- hasCondition(literalOrNegatedBool(Value).bind(BooleanId)))
+  ifStmt(hasCondition(literalOrNegatedBool(Value).bind(BooleanId)))
   .bind(IfStmtId),
   this);
 }
@@ -453,8 +452,7 @@
   bool Value,
   StringRef TernaryId) {
   Finder->addMatcher(
-  conditionalOperator(unless(isInTemplateInstantiation()),
-  hasTrueExpression(literalOrNegatedBool(Value)),
+  conditionalOperator(hasTrueExpression(literalOrNegatedBool(Value)),
   hasFalseExpression(literalOrNegatedBool(!Value)))
   .bind(TernaryId),
   this);
@@ -463,14 +461,12 @@
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
   bool Value, StringRef Id) {
   if (ChainedConditionalReturn)
-Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
-  hasThen(returnsBool(Value, ThenLiteralId)),
+Finder->addMatcher(ifStmt(hasThen(returnsBool(Value, ThenLiteralId)),
   hasElse(returnsBool(!Value)))
.bind(Id),
this);
   else
-Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
-  unless(hasParent(ifStmt())),
+Finder->addMatcher(ifStmt(unless(hasParent(ifStmt())),
   hasThen(returnsBool(Value, ThenLiteralId)),
   hasElse(returnsBool(!Value)))
.bind(Id),
@@ -495,16 +491,12 @@
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
  hasAnySubstatement(SimpleElse)));
   if (ChainedConditionalAssignment)
-Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
-  hasThen(Then), hasElse(Else))
-   .bind(Id),
-   this);
+Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
   else
-Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
-  unless(hasParent(ifStmt())), hasThen(Then),
-  hasElse(Else))
-   .bind(Id),
-   this);
+Finder->addMatcher(
+ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
+.bind(Id),
+this);
 }
 
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
@@ -512,11 +504,9 @@
   StringRef Id) {
   Finder->addMatcher(
   compoundStmt(

[PATCH] D96838: Add GNU attribute 'retain'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2655
+
+def Retain : InheritableAttr {
+  let Spellings = [GCC<"retain">];

Should this be a target-specific attribute as it only has effects on ELF 
targets?



Comment at: clang/lib/Sema/SemaDecl.cpp:2834
   }
+  if (RetainAttr *OldAttr = Old->getMostRecentDecl()->getAttr()) {
+RetainAttr *NewAttr = OldAttr->clone(Context);

I realize you're following the same pattern as the used attribute, but.. why is 
this code even necessary? The attributes are already marked as being inherited 
in Attr.td, so I'd not expect to need either of these. (If you don't know the 
answer off the top of your head, that's fine -- I can investigate on my own.)



Comment at: clang/test/Sema/attr-retain.c:3
+
+extern char test1[] __attribute__((retain)); // expected-warning {{'retain' 
attribute ignored}}
+extern const char test2[] __attribute__((retain)); // expected-warning 
{{'retain' attribute ignored}}

Oof, this is not a particularly friendly diagnostic (there's no mention of 
*why* the attribute is ignored, so it's hard to know how to respond to the 
diagnostic as a user).



Comment at: clang/test/Sema/attr-retain.c:8
+void foo() {
+  int l __attribute__((retain)); // expected-warning {{'retain' attribute only 
applies to variables with non-local storage, functions, and Objective-C 
methods}}
+}

Can you also add a test case showing the attribute does not accept any 
arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14261
+   "unsigned int Const* c;",
+   Alignment);
 }

can you test west const too?

```
Const unsigned int* c;
const unsigned int* d;
Const unsigned int& e;
const unsigned int& f;
const unsigned g;
Const unsigned h;
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

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

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97156

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -239,10 +239,10 @@
  *Builder) const override; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
   internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>\
   DefineMatcher() {\
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<  \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<  \
 internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>();   \
   }\
   template  \
@@ -284,18 +284,17 @@
 ParamType const Param; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType>   \
   DefineMatcher(ParamType const &Param) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType, \
-ReturnTypesF>(Param);  \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<  \
+internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,  \
+ParamType>(Param); \
   }\
-  typedef ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<   \
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>(&DefineMatcher##_Type##OverloadId)(\
-  ParamType const &Param); \
+  typedef ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<   \
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType> (&DefineMatcher##_Type##OverloadId)(ParamType const &Param);  \
   template \
   bool internal::  \
   matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -338,17 +337,17 @@
 ParamType2 const Param2;   \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,  \
-  ParamType2, ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType1, ParamType2>  \
   DefineMatcher(ParamType1 const &Param1, ParamType2 const &Param2) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,\
-ParamType2, ReturnTypesF>(Param1, Param2); \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWit

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

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

This was part of an early attempt to implement `mapAnyOf`. It's not needed, but 
it seems like a better idea to have one variadic class than multiple Param1/2 
classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

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

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

[Stephen Kelly via Phabricator via cfe-commits]

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

Update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -239,10 +239,10 @@
  *Builder) const override; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
   internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>\
   DefineMatcher() {\
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<  \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<  \
 internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>();   \
   }\
   template  \
@@ -284,18 +284,17 @@
 ParamType const Param; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType>   \
   DefineMatcher(ParamType const &Param) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType, \
-ReturnTypesF>(Param);  \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<  \
+internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,  \
+ParamType>(Param); \
   }\
-  typedef ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<   \
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>(&DefineMatcher##_Type##OverloadId)(\
-  ParamType const &Param); \
+  typedef ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<   \
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType> (&DefineMatcher##_Type##OverloadId)(ParamType const &Param);  \
   template \
   bool internal::  \
   matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -338,17 +337,17 @@
 ParamType2 const Param2;   \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,  \
-  ParamType2, ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<\
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType1, ParamType2>  \
   DefineMatcher(ParamType1 const &Param1, ParamType2 const &Param2) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,\
-ParamType2, ReturnTypesF>(Param1, Param2); \
+return ::clang::ast_matchers::internal::PolymorphicMatcherWithParamN<  \
+internal::matche

[PATCH] D97158: [ASTMatchers] Make nullPointerConstant usable at top-level

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97158

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -3660,10 +3660,10 @@
 TEST_P(ASTMatchersTest, NullPointerConstant) {
   EXPECT_TRUE(matches("#define NULL ((void *)0)\n"
   "void *v1 = NULL;",
-  expr(nullPointerConstant(;
-  EXPECT_TRUE(matches("char *cp = (char *)0;", expr(nullPointerConstant(;
-  EXPECT_TRUE(matches("int *ip = 0;", expr(nullPointerConstant(;
-  EXPECT_FALSE(matches("int i = 0;", expr(nullPointerConstant(;
+  nullPointerConstant()));
+  EXPECT_TRUE(matches("char *cp = (char *)0;", nullPointerConstant()));
+  EXPECT_TRUE(matches("int *ip = 0;", nullPointerConstant()));
+  EXPECT_FALSE(matches("int i = 0;", nullPointerConstant()));
 }
 
 TEST_P(ASTMatchersTest, NullPointerConstant_GNUNull) {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7379,10 +7379,10 @@
 /// expr(nullPointerConstant())
 ///   matches the initializer for v1, v2, v3, cp, and ip. Does not match the
 ///   initializer for i.
-AST_MATCHER_FUNCTION(internal::Matcher, nullPointerConstant) {
-  return anyOf(
+AST_MATCHER_FUNCTION(internal::BindableMatcher, nullPointerConstant) {
+  return stmt(anyOf(
   gnuNullExpr(), cxxNullPtrLiteralExpr(),
-  integerLiteral(equals(0), hasParent(expr(hasType(pointerType());
+  integerLiteral(equals(0), hasParent(expr(hasType(pointerType()));
 }
 
 /// Matches the DecompositionDecl the binding belongs to.


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -3660,10 +3660,10 @@
 TEST_P(ASTMatchersTest, NullPointerConstant) {
   EXPECT_TRUE(matches("#define NULL ((void *)0)\n"
   "void *v1 = NULL;",
-  expr(nullPointerConstant(;
-  EXPECT_TRUE(matches("char *cp = (char *)0;", expr(nullPointerConstant(;
-  EXPECT_TRUE(matches("int *ip = 0;", expr(nullPointerConstant(;
-  EXPECT_FALSE(matches("int i = 0;", expr(nullPointerConstant(;
+  nullPointerConstant()));
+  EXPECT_TRUE(matches("char *cp = (char *)0;", nullPointerConstant()));
+  EXPECT_TRUE(matches("int *ip = 0;", nullPointerConstant()));
+  EXPECT_FALSE(matches("int i = 0;", nullPointerConstant()));
 }
 
 TEST_P(ASTMatchersTest, NullPointerConstant_GNUNull) {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7379,10 +7379,10 @@
 /// expr(nullPointerConstant())
 ///   matches the initializer for v1, v2, v3, cp, and ip. Does not match the
 ///   initializer for i.
-AST_MATCHER_FUNCTION(internal::Matcher, nullPointerConstant) {
-  return anyOf(
+AST_MATCHER_FUNCTION(internal::BindableMatcher, nullPointerConstant) {
+  return stmt(anyOf(
   gnuNullExpr(), cxxNullPtrLiteralExpr(),
-  integerLiteral(equals(0), hasParent(expr(hasType(pointerType());
+  integerLiteral(equals(0), hasParent(expr(hasType(pointerType()));
 }
 
 /// Matches the DecompositionDecl the binding belongs to.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95745: Support unwinding from inline assembly

[Paul via Phabricator via cfe-commits]

cynecx updated this revision to Diff 325323.
cynecx edited the summary of this revision.
cynecx added a comment.
Herald added a subscriber: pengfei.

Added tests and updated splitted patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95745

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGenCXX/unwind-inline-asm.cpp
  llvm/bindings/go/llvm/ir.go
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/InlineAsm.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantsContext.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-no-unwind-inline-asm.ll
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll
  llvm/test/CodeGen/AArch64/GlobalISel/unwind-inline-asm.ll
  llvm/test/CodeGen/X86/no-unwind-inline-asm-codegen.ll
  llvm/test/CodeGen/X86/unwind-inline-asm-codegen.ll
  llvm/test/Transforms/Inline/no-unwind-inline-asm.ll
  llvm/test/Transforms/Inline/unwind-inline-asm.ll
  llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
  llvm/test/Transforms/InstCombine/unwind-inline-asm.ll

Index: llvm/test/Transforms/InstCombine/unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/unwind-inline-asm.ll
@@ -0,0 +1,38 @@
+; RUN: opt < %s -O2 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+; CHECK: define dso_local void @test()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: invoke void asm sideeffect unwind
+
+  invoke void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+; CHECK: %0 = landingpad { i8*, i32 }
+; CHECK: resume { i8*, i32 } %0
+
+  %0 = landingpad { i8*, i32 }
+  cleanup
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.2, i64 0, i64 0))
+  resume { i8*, i32 } %0
+
+}
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+declare dso_local void @printf(i8*, ...)
Index: llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
@@ -0,0 +1,36 @@
+; RUN: opt < %s -O2 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+; CHECK: define dso_local void @test()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: tail call void asm sideeffect
+; CHECK-NEXT: ret void
+
+  invoke void asm sideeffect "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+  cleanup
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.2, i64 0, i64 0))
+  resume { i8*, i32 } %0
+
+}
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+declare dso_local void @printf(i8*, ...)
Index: llvm/test/Transforms/Inline/unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/unwind-inline-asm.ll
@@ -0,0 +1,46 @@
+; RUN: opt < %s -inline -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @proxy() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  call void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  call void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  ret void

[PATCH] D95745: Support unwinding from inline assembly

[Paul via Phabricator via cfe-commits]

cynecx added a comment.

@rnk This is ready for a proper review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95745

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

[PATCH] D95745: Support unwinding from inline assembly

[Paul via Phabricator via cfe-commits]

cynecx updated this revision to Diff 325328.
cynecx added a comment.

clang-format


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

https://reviews.llvm.org/D95745

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGenCXX/unwind-inline-asm.cpp
  llvm/bindings/go/llvm/ir.go
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/InlineAsm.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantsContext.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-no-unwind-inline-asm.ll
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll
  llvm/test/CodeGen/AArch64/GlobalISel/unwind-inline-asm.ll
  llvm/test/CodeGen/X86/no-unwind-inline-asm-codegen.ll
  llvm/test/CodeGen/X86/unwind-inline-asm-codegen.ll
  llvm/test/Transforms/Inline/no-unwind-inline-asm.ll
  llvm/test/Transforms/Inline/unwind-inline-asm.ll
  llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
  llvm/test/Transforms/InstCombine/unwind-inline-asm.ll

Index: llvm/test/Transforms/InstCombine/unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/unwind-inline-asm.ll
@@ -0,0 +1,38 @@
+; RUN: opt < %s -O2 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+; CHECK: define dso_local void @test()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: invoke void asm sideeffect unwind
+
+  invoke void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+; CHECK: %0 = landingpad { i8*, i32 }
+; CHECK: resume { i8*, i32 } %0
+
+  %0 = landingpad { i8*, i32 }
+  cleanup
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.2, i64 0, i64 0))
+  resume { i8*, i32 } %0
+
+}
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+declare dso_local void @printf(i8*, ...)
Index: llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/no-unwind-inline-asm.ll
@@ -0,0 +1,36 @@
+; RUN: opt < %s -O2 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+; CHECK: define dso_local void @test()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: tail call void asm sideeffect
+; CHECK-NEXT: ret void
+
+  invoke void asm sideeffect "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  ret void
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+  cleanup
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.2, i64 0, i64 0))
+  resume { i8*, i32 } %0
+
+}
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+declare dso_local void @printf(i8*, ...)
Index: llvm/test/Transforms/Inline/unwind-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/unwind-inline-asm.ll
@@ -0,0 +1,46 @@
+; RUN: opt < %s -inline -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
+
+define dso_local void @trap() {
+entry:
+  unreachable
+}
+
+define dso_local void @proxy() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  call void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  call void asm sideeffect unwind "call trap", "~{dirflag},~{fpsr},~{flags}"()
+  ret void
+}
+
+define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+; CHECK: define dso_local void @tes

[PATCH] D97161: Improve diagnostic for ignored GNU 'used' attribute

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added a reviewer: aaron.ballman.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97161

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/attr-used.c
  clang/test/SemaCXX/attr-used.cpp


Index: clang/test/SemaCXX/attr-used.cpp
===
--- clang/test/SemaCXX/attr-used.cpp
+++ clang/test/SemaCXX/attr-used.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-extern char test1[] __attribute__((used)); // expected-warning {{'used' 
attribute ignored}}
-extern const char test2[] __attribute__((used)); // expected-warning {{'used' 
attribute ignored}}
+extern char test1[] __attribute__((used)); // expected-warning {{'used' 
attribute ignored on a non-definition declaration}}
+extern const char test2[] __attribute__((used)); // expected-warning {{'used' 
attribute ignored on a non-definition declaration}}
 extern const char test3[] __attribute__((used)) = "";
Index: clang/test/Sema/attr-used.c
===
--- clang/test/Sema/attr-used.c
+++ clang/test/Sema/attr-used.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -verify -fsyntax-only -Wno-private-extern %s
 
-extern int l0 __attribute__((used)); // expected-warning {{'used' attribute 
ignored}}
-__private_extern__ int l1 __attribute__((used)); // expected-warning {{'used' 
attribute ignored}}
+extern int l0 __attribute__((used)); // expected-warning {{'used' attribute 
ignored on a non-definition declaration}}
+__private_extern__ int l1 __attribute__((used)); // expected-warning {{'used' 
attribute ignored on a non-definition declaration}}
 
 struct __attribute__((used)) s { // expected-warning {{'used' attribute only 
applies to variables with non-local storage, functions, and Objective-C 
methods}}
   int x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13295,7 +13295,8 @@
 
   if (UsedAttr *Attr = VD->getAttr()) {
 if (!Attr->isInherited() && !VD->isThisDeclarationADefinition()) {
-  Diag(Attr->getLocation(), diag::warn_attribute_ignored) << Attr;
+  Diag(Attr->getLocation(), diag::warn_attribute_ignored_on_non_definition)
+  << Attr;
   VD->dropAttr();
 }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3159,6 +3159,9 @@
 def warn_nothrow_attribute_ignored : Warning<"'nothrow' attribute conflicts 
with"
   " exception specification; attribute ignored">,
   InGroup;
+def warn_attribute_ignored_on_non_definition :
+  Warning<"%0 attribute ignored on a non-definition declaration">,
+  InGroup;
 def warn_attribute_ignored_on_inline :
   Warning<"%0 attribute ignored on inline function">,
   InGroup;


Index: clang/test/SemaCXX/attr-used.cpp
===
--- clang/test/SemaCXX/attr-used.cpp
+++ clang/test/SemaCXX/attr-used.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-extern char test1[] __attribute__((used)); // expected-warning {{'used' attribute ignored}}
-extern const char test2[] __attribute__((used)); // expected-warning {{'used' attribute ignored}}
+extern char test1[] __attribute__((used)); // expected-warning {{'used' attribute ignored on a non-definition declaration}}
+extern const char test2[] __attribute__((used)); // expected-warning {{'used' attribute ignored on a non-definition declaration}}
 extern const char test3[] __attribute__((used)) = "";
Index: clang/test/Sema/attr-used.c
===
--- clang/test/Sema/attr-used.c
+++ clang/test/Sema/attr-used.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -verify -fsyntax-only -Wno-private-extern %s
 
-extern int l0 __attribute__((used)); // expected-warning {{'used' attribute ignored}}
-__private_extern__ int l1 __attribute__((used)); // expected-warning {{'used' attribute ignored}}
+extern int l0 __attribute__((used)); // expected-warning {{'used' attribute ignored on a non-definition declaration}}
+__private_extern__ int l1 __attribute__((used)); // expected-warning {{'used' attribute ignored on a non-definition declaration}}
 
 struct __attribute__((used)) s { // expected-warning {{'used' attribute only applies to variables with non-local storage, functions, and Objective-C methods}}
   int x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13295,7 +13295,8 @@
 
   if (UsedAttr *Att

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Just out of curiosity, in which language is `Const` used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2655
+
+def Retain : InheritableAttr {
+  let Spellings = [GCC<"retain">];

aaron.ballman wrote:
> Should this be a target-specific attribute as it only has effects on ELF 
> targets?
As I understand it, GCC `retain` is not warned on unsupported targets.

Regardless of GCC's choice, I think not having a `warning: unknown attribute 
'retain' ignored [-Wunknown-attributes]` diagnostic makes sense. `retain` will 
be usually used together with `used`. In Clang, `used` already has "retain" 
semantics on macOS and Windows (I don't know what they do on GCC; GCC folks 
want orthogonality for ELF and I agree). If `retain` is silently ignored on 
macOS and Windows, then users don't need to condition compile for different 
targets.



Comment at: clang/lib/Sema/SemaDecl.cpp:2834
   }
+  if (RetainAttr *OldAttr = Old->getMostRecentDecl()->getAttr()) {
+RetainAttr *NewAttr = OldAttr->clone(Context);

aaron.ballman wrote:
> I realize you're following the same pattern as the used attribute, but.. why 
> is this code even necessary? The attributes are already marked as being 
> inherited in Attr.td, so I'd not expect to need either of these. (If you 
> don't know the answer off the top of your head, that's fine -- I can 
> investigate on my own.)
Without `setInherited`, `attr-decl-after-definition.c` will fail. (I don't know 
why..) So I will keep the code but add a test case using the variable 
`tentative`.



Comment at: clang/test/CodeGen/attr-retain.c:21
+int g1 __attribute__((retain));
+__attribute__((retain)) static int g2;
+__attribute__((used,retain)) static int g3;

phosek wrote:
> Would it be possible to also include negative check for `g2`?
```
// CHECK-NEXT: @g1 ={{.*}} global i32 {{.*}} !retain ![[#EMPTY]]
// CHECK-NEXT: @g3 = internal global i32 {{.*}} !retain ![[#EMPTY]]
```

excludes accidental g2.



Comment at: clang/test/Sema/attr-retain.c:3
+
+extern char test1[] __attribute__((retain)); // expected-warning {{'retain' 
attribute ignored}}
+extern const char test2[] __attribute__((retain)); // expected-warning 
{{'retain' attribute ignored}}

aaron.ballman wrote:
> Oof, this is not a particularly friendly diagnostic (there's no mention of 
> *why* the attribute is ignored, so it's hard to know how to respond to the 
> diagnostic as a user).
I created the test based on the existing attr-used.c. Let me figure out how to 
make the diagnostic friendlier...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 325336.
MaskRay marked 5 inline comments as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/attr-retain.c
  clang/test/CodeGenCXX/attr-retain.cpp
  clang/test/CodeGenCXX/extern-c.cpp
  clang/test/Sema/attr-retain.c

Index: clang/test/Sema/attr-retain.c
===
--- /dev/null
+++ clang/test/Sema/attr-retain.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+/// We allow 'retain' on non-ELF targets because 'retain' is often used together
+/// with 'used'. 'used' has GC root semantics on macOS and Windows. We want
+/// users to just write retain,used and don't need to dispatch on binary formats.
+
+extern char test1[] __attribute__((retain));   // expected-warning {{'retain' attribute ignored on a non-definition declaration}}
+extern const char test2[] __attribute__((retain)); // expected-warning {{'retain' attribute ignored on a non-definition declaration}}
+const char test3[] __attribute__((retain)) = "";
+
+struct __attribute__((retain)) s { // expected-warning {{'retain' attribute only applies to variables with non-local storage, functions, and Objective-C methods}}
+};
+
+int g0 __attribute__((retain(0))); // expected-error {{'retain' attribute takes no arguments}}
+
+void foo() {
+  static int a __attribute__((retain));
+  int b __attribute__((retain)); // expected-warning {{'retain' attribute only applies to variables with non-local storage, functions, and Objective-C methods}}
+}
+
+__attribute__((used)) static void f0();
+
+/// Test attribute merging.
+int tentative;
+int tentative __attribute__((retain));
+extern int tentative;
+int tentative = 0;
Index: clang/test/CodeGenCXX/extern-c.cpp
===
--- clang/test/CodeGenCXX/extern-c.cpp
+++ clang/test/CodeGenCXX/extern-c.cpp
@@ -70,16 +70,19 @@
 __attribute__((used)) static int duplicate_internal_fn() { return 0; }
   }
 
-  // CHECK: @llvm.used = appending global {{.*}} @internal_var {{.*}} @internal_fn 
+  // CHECK: @llvm.used = appending global {{.*}} @internal_var {{.*}} @internal_fn
 
   // CHECK-NOT: @unused
   // CHECK-NOT: @duplicate_internal
   // CHECK: @internal_var = internal alias i32, i32* @_ZL12internal_var
+  // CHECK-NOT: !retain
   // CHECK-NOT: @unused
   // CHECK-NOT: @duplicate_internal
   // CHECK: @internal_fn = internal alias i32 (), i32 ()* @_ZL11internal_fnv
+  // CHECK-NOT: !retain
   // CHECK-NOT: @unused
   // CHECK-NOT: @duplicate_internal
+  // CHECK: define internal i32 @_ZL11internal_fnv()
 }
 
 namespace PR19411 {
Index: clang/test/CodeGenCXX/attr-retain.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/attr-retain.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -Werror %s -o - | FileCheck %s
+
+struct X0 {
+  // RETAIN: define linkonce_odr{{.*}} @_ZN2X0C1Ev({{.*}} !retain
+  __attribute__((used, retain)) X0() {}
+  // RETAIN: define linkonce_odr{{.*}} @_ZN2X0D1Ev({{.*}} !retain
+  __attribute__((used, retain)) ~X0() {}
+};
+
+struct X1 {
+  struct Nested {
+// RETAIN-NOT: 2f0Ev
+// RETAIN: define linkonce_odr{{.*}} @_ZN2X16Nested2f1Ev({{.*}} !retain
+void __attribute__((retain)) f0() {}
+void __attribute__((used, retain)) f1() {}
+  };
+};
+
+// CHECK: define internal void @_ZN10merge_declL4funcEv(){{.*}} !retain
+namespace merge_decl {
+static void func();
+void bar() { void func() __attribute__((used, retain)); }
+static void func() {}
+} // namespace merge_decl
+
+namespace instantiate_member {
+template 
+struct S {
+  void __attribute__((used, retain)) f() {}
+};
+
+void test() {
+  // CHECK: define linkonce_odr{{.*}} void @_ZN18instantiate_member1SIiE1fEv({{.*}} !retain
+  S a;
+}
+} // namespace instantiate_member
Index: clang/test/CodeGen/attr-retain.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-retain.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+/// Set !retain regardless of the target. The backend will lower !retain to
+/// SHF_GNU_RETAIN on ELF and ignore the metadata for other binary formats.
+// CHECK:  @c0 ={{.*}} constant i32 {{.*}} !retain ![[#EMPTY:]]
+// CHECK:  @foo.l0 = internal global i32 {{.*}} !retain ![[#EMPTY]]
+// CHECK:  @g0 ={{.*}} global i32 {{.*}} !retain ![[#EMPTY]]
+// CHECK-NEXT: @g1 ={{.*}} global i32 {{.*}} !retain ![[#EMPTY]]
+// CHECK-NEXT: @g3 = internal global i32 {{.*}} !retai

[PATCH] D97158: [ASTMatchers] Make nullPointerConstant usable at top-level

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Won't this change require a regeneration of the documentation 
`clang/docs/tools/dump_ast_matchers.py`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97158

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

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a reviewer: klimek.
njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

This change is going to break the dump_ast_matchers script. So that will need 
updating to ensure everything is all correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

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

[PATCH] D97150: [clang-tidy] Simplify suspicious memset usage check

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp:39
-  unless(
-  eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())),
-  hasType(arrayType(hasElementType(

This eachOf looked suspicious inside of an unless. Removing it should have no 
effect tho.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97150

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

[PATCH] D96838: Add GNU attribute 'retain'

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

GCC 11 hasn't been released yet, so can we still engage with GCC about the 
semantics of this attribute?  This is a very low-level attribute, and I don't 
understand why it was made separate instead of just having `used` add the 
appropriate section flag on targets that support it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D96838#2578073 , @rjmccall wrote:

> GCC 11 hasn't been released yet, so can we still engage with GCC about the 
> semantics of this attribute?  This is a very low-level attribute, and I don't 
> understand why it was made separate instead of just having `used` add the 
> appropriate section flag on targets that support it.

GCC did overload `used` with the linker garbage collection semantics. Then 
after discussions (e.g. 
https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565478.html) they 
decided to pick the earliest suggestion: add `retain`.

This makes sense to me: `used` can emit a function which is only used by inline 
assembly, but the user intention can still be that the whole thing can be 
GCable.

IIUC macOS's always enabled `.subsections_via_symbols` means always 
-ffunction-sections/-fdata-sections, so there is no GC preciseness concern.
However, the current Windows behavior (`.drectve` include a linker option to 
force retaining the whole section) can unnecessarily retain the full section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

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

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:490
+  Instruction &Inst = *II++;
+  if (match(&Inst, m_Intrinsic()) ||
+  match(&Inst, m_Intrinsic()) ||

Should be better to use
```
if (auto *Inst  = dyn_cast&*II++)
  switch (Inst->getIntrinsicID()) {
  case Intrinsic::x86_tdpbssd_internal:
  ...
```



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:501
+  for (auto *Inst : WorkList) {
+if (match(Inst, m_Intrinsic()))
+  // %amx1 = bitcast <256 x i32> %vec to x86_amx

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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

[PATCH] D96838: Add GNU attribute 'retain'

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D96838#2578083 , @MaskRay wrote:

> In D96838#2578073 , @rjmccall wrote:
>
>> GCC 11 hasn't been released yet, so can we still engage with GCC about the 
>> semantics of this attribute?  This is a very low-level attribute, and I 
>> don't understand why it was made separate instead of just having `used` add 
>> the appropriate section flag on targets that support it.
>
> GCC did overload `used` with the linker garbage collection semantics. Then 
> after discussions (e.g. 
> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565478.html) they 
> decided to pick the earliest suggestion: add `retain`.
>
> This makes sense to me: `used` can emit a function which is only used by 
> inline assembly, but the user intention can still be that the whole thing can 
> be GCable.

There's definitely some merit to distinguishing the cases here — `used` doesn't 
just force the object to be emitted, it also forces the compiler to treat it as 
if there might be accesses to the variable / function that it can't see.  Thus, 
for example, a non-const `used` variable has to be assumed to be potentially 
mutated in arbitrary ways.  I guess my concern is that this new attribute still 
doesn't feel like it covers the use cases very well, especially because `used` 
does have long-established semantics of affecting the linker on non-ELF 
targets.  Basically, to get "full strength" semantics that affect the compiler 
and linker on all targets, you have to add a new attribute; to get "partial 
strength" semantics that only affect the compiler, you can use the old 
attribute on ELF, but there's no way to spell it on non-ELF; and if you wanted 
to get "weak" semantics (e.g. to force the symbol to be preserved through 
linking without broadly disabling the optimizer), there's no way to spell that 
anywhere.  Seems like if we're going to the trouble of adding an attribute, 
that's not the situation we want to end up in.

> IIUC macOS's always enabled `.subsections_via_symbols` means always 
> -ffunction-sections/-fdata-sections, so there is no GC preciseness concern.

Yes, the MachO linker will aggressively dead-strip, and `used` prevents that.

> However, the current Windows behavior (`.drectve` include a linker option to 
> force retaining the whole section) can unnecessarily retain the full section, 
> in -fno-function-sections or -fno-data-sections mode.

But that Windows behavior is a limitation of the format rather than an intent 
to provide different semantics on different targets, I'd say.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D96838#2578097 , @rjmccall wrote:

> In D96838#2578083 , @MaskRay wrote:
>
>> In D96838#2578073 , @rjmccall wrote:
>>
>>> GCC 11 hasn't been released yet, so can we still engage with GCC about the 
>>> semantics of this attribute?  This is a very low-level attribute, and I 
>>> don't understand why it was made separate instead of just having `used` add 
>>> the appropriate section flag on targets that support it.
>>
>> GCC did overload `used` with the linker garbage collection semantics. Then 
>> after discussions (e.g. 
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565478.html) they 
>> decided to pick the earliest suggestion: add `retain`.
>>
>> This makes sense to me: `used` can emit a function which is only used by 
>> inline assembly, but the user intention can still be that the whole thing 
>> can be GCable.
>
> There's definitely some merit to distinguishing the cases here — `used` 
> doesn't just force the object to be emitted, it also forces the compiler to 
> treat it as if there might be accesses to the variable / function that it 
> can't see.  Thus, for example, a non-const `used` variable has to be assumed 
> to be potentially mutated in arbitrary ways.  I guess my concern is that this 
> new attribute still doesn't feel like it covers the use cases very well, 
> especially because `used` does have long-established semantics of affecting 
> the linker on non-ELF targets.  Basically, to get "full strength" semantics 
> that affect the compiler and linker on all targets, you have to add a new 
> attribute;

Yes.

> to get "partial strength" semantics that only affect the compiler, you can 
> use the old attribute on ELF, but there's no way to spell it on non-ELF; and 
> if you wanted to get "weak" semantics (e.g. to force the symbol to be 
> preserved through linking without broadly disabling the optimizer), there's 
> no way to spell that anywhere.

The semantics can be represented on non-ELF, as long as the binary format 
supports multiple sections of the same name.

Garbage collection is done at section level on both ELF and PE-COFF. On ELF, 
GCC developers decided that a single `used` making the monolithic 
`.data`/`.text` retained loses GC preciseness and is not a good tradeoff. (I 
agree)

So they let the `used` object be placed in a separate section. There may be 
multiple sections of the same name, even in -fno-function-sections 
-fno-data-sections mode.

  .section .text,"ax",@progbits
  ...
  .section .text,"axR",@progbits,unique,1
  ...

This uncovered a Linux kernel problem (I'd say it is that the Linux kernel's 
linker scripts relies on too much on a not-entirely-guaranteed behavior) 
http://gcc.gnu.org/PR99113 
Their later discussion revealed that separation of concerns is good. So they 
re-picked the `retain` idea.

If PE-COFF wants to pick up the idea, it can do that for external linkage 
symbols: it can let `retain` create a separate section with `/INCLUDE:foo` in 
`.drectve`.
For an internal linkage symbol, I think that is likely non-representable in the 
binary format. (maybe rnk can correct me:) )
If PE-COFF decides to do this, perhaps it should revisit whether `used` should 
set `/INCLUDE:foo` as well, as it can nearly match ELF semantics.

For macOS, it can make `retain` pick up the `N_NO_DEAD_STRIP` behavior if it 
wants to match ELF.
Though the current `uses` already retains the subsection.

> Seems like if we're going to the trouble of adding an attribute, that's not 
> the situation we want to end up in.



>> IIUC macOS's always enabled `.subsections_via_symbols` means always 
>> -ffunction-sections/-fdata-sections, so there is no GC preciseness concern.
>
> Yes, the MachO linker will aggressively dead-strip, and `used` prevents that.
>
>> However, the current Windows behavior (`.drectve` include a linker option to 
>> force retaining the whole section) can unnecessarily retain the full 
>> section, in -fno-function-sections or -fno-data-sections mode.
>
> But that Windows behavior is a limitation of the format rather than an intent 
> to provide different semantics on different targets, I'd say.

It is a limitation for internal symbols. But for external symbols it can fully 
match ELF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96838: Add GNU attribute 'retain'

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D96838#2578101 , @MaskRay wrote:

> In D96838#2578097 , @rjmccall wrote:
>
>> In D96838#2578083 , @MaskRay wrote:
>>
>>> In D96838#2578073 , @rjmccall 
>>> wrote:
>>>
 GCC 11 hasn't been released yet, so can we still engage with GCC about the 
 semantics of this attribute?  This is a very low-level attribute, and I 
 don't understand why it was made separate instead of just having `used` 
 add the appropriate section flag on targets that support it.
>>>
>>> GCC did overload `used` with the linker garbage collection semantics. Then 
>>> after discussions (e.g. 
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565478.html) they 
>>> decided to pick the earliest suggestion: add `retain`.
>>>
>>> This makes sense to me: `used` can emit a function which is only used by 
>>> inline assembly, but the user intention can still be that the whole thing 
>>> can be GCable.
>>
>> There's definitely some merit to distinguishing the cases here — `used` 
>> doesn't just force the object to be emitted, it also forces the compiler to 
>> treat it as if there might be accesses to the variable / function that it 
>> can't see.  Thus, for example, a non-const `used` variable has to be assumed 
>> to be potentially mutated in arbitrary ways.  I guess my concern is that 
>> this new attribute still doesn't feel like it covers the use cases very 
>> well, especially because `used` does have long-established semantics of 
>> affecting the linker on non-ELF targets.  Basically, to get "full strength" 
>> semantics that affect the compiler and linker on all targets, you have to 
>> add a new attribute;
>
> Yes.
>
>> to get "partial strength" semantics that only affect the compiler, you can 
>> use the old attribute on ELF, but there's no way to spell it on non-ELF; and 
>> if you wanted to get "weak" semantics (e.g. to force the symbol to be 
>> preserved through linking without broadly disabling the optimizer), there's 
>> no way to spell that anywhere.
>
> The semantics can be represented on non-ELF, as long as the binary format 
> supports multiple sections of the same name.

My concern is less whether the semantics are implementable and more whether 
there's a way of requesting them in the source language.  `attribute((used))` 
is expected to prevent link-time stripping on Mach-O and PE/COFF.  We can't 
just change the long-accepted semantics of the existing attribute after 
something like 20 years; this is documented behavior.

That said, I can understand why GCC is reluctant to make `attribute((used))` 
alone start preventing the linker from stripping symbols that it's been able to 
strip for years.  So I guess really the attribute just has to be understood to 
have a different meaning on different targets because of this historical 
divergence.  This seems like something we need to document in the manual.

If we wanted to implement the PE/COFF precision improvement, so that `used` 
only affected one specific declaration, that seems like something we could 
probably reasonably do, since that behavior is not documented (and may be 
inconsistent between linkages anyway, if I understand you right).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D96838#2578127 , @rjmccall wrote:

> In D96838#2578101 , @MaskRay wrote:
>
>> In D96838#2578097 , @rjmccall wrote:
>>
>>> In D96838#2578083 , @MaskRay wrote:
>>>
 In D96838#2578073 , @rjmccall 
 wrote:

> GCC 11 hasn't been released yet, so can we still engage with GCC about 
> the semantics of this attribute?  This is a very low-level attribute, and 
> I don't understand why it was made separate instead of just having `used` 
> add the appropriate section flag on targets that support it.

 GCC did overload `used` with the linker garbage collection semantics. Then 
 after discussions (e.g. 
 https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565478.html) they 
 decided to pick the earliest suggestion: add `retain`.

 This makes sense to me: `used` can emit a function which is only used by 
 inline assembly, but the user intention can still be that the whole thing 
 can be GCable.
>>>
>>> There's definitely some merit to distinguishing the cases here — `used` 
>>> doesn't just force the object to be emitted, it also forces the compiler to 
>>> treat it as if there might be accesses to the variable / function that it 
>>> can't see.  Thus, for example, a non-const `used` variable has to be 
>>> assumed to be potentially mutated in arbitrary ways.  I guess my concern is 
>>> that this new attribute still doesn't feel like it covers the use cases 
>>> very well, especially because `used` does have long-established semantics 
>>> of affecting the linker on non-ELF targets.  Basically, to get "full 
>>> strength" semantics that affect the compiler and linker on all targets, you 
>>> have to add a new attribute;
>>
>> Yes.
>>
>>> to get "partial strength" semantics that only affect the compiler, you can 
>>> use the old attribute on ELF, but there's no way to spell it on non-ELF; 
>>> and if you wanted to get "weak" semantics (e.g. to force the symbol to be 
>>> preserved through linking without broadly disabling the optimizer), there's 
>>> no way to spell that anywhere.
>>
>> The semantics can be represented on non-ELF, as long as the binary format 
>> supports multiple sections of the same name.
>
> My concern is less whether the semantics are implementable and more whether 
> there's a way of requesting them in the source language.  `attribute((used))` 
> is expected to prevent link-time stripping on Mach-O and PE/COFF.  We can't 
> just change the long-accepted semantics of the existing attribute after 
> something like 20 years; this is documented behavior.
>
> That said, I can understand why GCC is reluctant to make `attribute((used))` 
> alone start preventing the linker from stripping symbols that it's been able 
> to strip for years.  So I guess really the attribute just has to be 
> understood to have a different meaning on different targets because of this 
> historical divergence.  This seems like something we need to document in the 
> manual.

I have documented the behaviors in `clang/include/clang/Basic/AttrDocs.td`. Do 
you have suggestions on the wording?

> If we wanted to implement the PE/COFF precision improvement, so that `used` 
> only affected one specific declaration, that seems like something we could 
> probably reasonably do, since that behavior is not documented (and may be 
> inconsistent between linkages anyway, if I understand you right).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

@vsavchenko, could you please review this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976

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

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

[Darwin Xu via Phabricator via cfe-commits]

darwin added a comment.

In D97137#2577794 , 
@HazardyKnusperkeks wrote:

> Just out of curiosity, in which language is `Const` used?

For example it is C's macro.

I found this issue in our C code. It has many macros and one function was 
defined like this:

  static IO_FLT_POSTOP_CALLBACK_STATUS FilterDriverClosePostOpCallback(
  IN OUT PIO_FLT_POSTOP_DATA  pPostopData,
  IN IO_FLT_RELATED_OBJECTS   FltObjects,
  IN OPTIONAL PIO_FLT_CONTEXT pCompletionContext,
  IN IO_FLT_POST_OPERATION_FLAGS  Flags,
  OUT PIO_FLT_POSTOP_CANCEL_CALLBACK* pCancellationCallback,
  OUT OPTIONAL PVOID* ppCancellationContext)

And because of this issue, the last two lines couldn't be formatted properly:

  static IO_FLT_POSTOP_CALLBACK_STATUS FilterDriverClosePostOpCallback(
  IN OUT PIO_FLT_POSTOP_DATA pPostopData,
  IN IO_FLT_RELATED_OBJECTS  FltObjects,
  IN OPTIONAL PIO_FLT_CONTEXTpCompletionContext,
  IN IO_FLT_POST_OPERATION_FLAGS Flags,
  OUT PIO_FLT_POSTOP_CANCEL_CALLBACK* pCancellationCallback,
  OUT OPTIONAL PVOID* ppCancellationContext)

Then, I started to investigate this issue. Interestingly, if the token before 
the asterisk is a keyword like `const` or `int`, it works fine sometimes:

  static IO_FLT_POSTOP_CALLBACK_STATUS FilterDriverClosePostOpCallback(
  IN OUT PIO_FLT_POSTOP_DATA pPostopData,
  IN IO_FLT_RELATED_OBJECTS  FltObjects,
  IN OPTIONAL PIO_FLT_CONTEXTpCompletionContext,
  IN IO_FLT_POST_OPERATION_FLAGS Flags,
  OUT int*   pCancellationCallback, 
<== I've changed PIO_FLT_POSTOP_CANCEL_CALLBACK into int, it could be 
formatted properly.
  OUT OPTIONAL const* ppCancellationContext)
<== I've changed PVOID into const, it still couldn't be formatted properly.

I didn't know the reason behind of this. Anyway, after this modification, it 
can handle all these types of code.




Comment at: clang/unittests/Format/FormatTest.cpp:14257
+
+  Alignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned int*   a;\n"

curdeius wrote:
> Could you please test as well mixed pointers and references?
Sure.



Comment at: clang/unittests/Format/FormatTest.cpp:14260
+   "int*b;\n"
+   "unsigned int Const* c;",
+   Alignment);

curdeius wrote:
> Could you please test `const` as well?
Sure.

Actually, clang-format worked well for `const` originally.



Comment at: clang/unittests/Format/FormatTest.cpp:14261
+   "unsigned int Const* c;",
+   Alignment);
 }

MyDeveloperDay wrote:
> can you test west const too?
> 
> ```
> Const unsigned int* c;
> const unsigned int* d;
> Const unsigned int& e;
> const unsigned int& f;
> const unsigned g;
> Const unsigned h;
> ```
> 
Sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

[PATCH] D96777: [clang][driver] Set the input type to Fortran when reading from stdin

[Sourabh Singh Tomar via Phabricator via cfe-commits]

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

Thanks! for the patch.
This code touches some of the `clang` part, Anyway changes are pretty 
self-explanatory. I'll leave this one to you, if you want land it or wait from 
someone from `clang` community to take a look :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96777

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

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

[Darwin Xu via Phabricator via cfe-commits]

darwin added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:716-717
 break;
   if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
 tok::kw_operator))
 return false;

Maybe I should combine the conditions into this `isOneOf(...)` together since 
they all return false.

Well, when I was writing this, I remembered why I had put the check before line 
714. It was because line 714 would be executed and breaks in that scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

LLVM buildmaster will be restarted soon

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be restarted in the nearest hour.

Thanks

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

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

[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.

Looks good, thanks! Do you need someone to commit this for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96147

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

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

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

rsmith added a comment.

Hm, before we go forward with this: can we get a test for the "shadows a 
structured binding" case, please? You might also need to add a new case to the 
"declaration shadows ..." warning text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96147

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

[PATCH] D97182: [clang] fix cmake wrong variable name from RTLIBS to RTLIB

[Philip Lassen via Phabricator via cfe-commits]

philass created this revision.
philass added a reviewer: chandlerc.
Herald added a subscriber: mgorny.
philass requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

RTSLIBS should instread be RTLIB

   if (CLANG_DEFAULT_UNWINDLIB STREQUAL "")
 if (CLANG_DEFAULT_RTLIB STREQUAL "libgcc")
   set (CLANG_DEFAULT_UNWINDLIB "libgcc" CACHE STRING "" FORCE)
  -  elseif (CLANG_DEFAULT_RTLIBS STREQUAL "libunwind") 
  +  elseif (CLANG_DEFAULT_RTLIB STREQUAL "libunwind")
   set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)
 endif()
   endif()

Since CLANG_DEFAULT_RTLIBS should be CLANG_DEFAULT_RTLIB the line

  set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)

will never be reached.

This addresses `https://bugs.llvm.org/show_bug.cgi?id=48291`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97182

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -262,7 +262,7 @@
 if (CLANG_DEFAULT_UNWINDLIB STREQUAL "")
   if (CLANG_DEFAULT_RTLIB STREQUAL "libgcc")
 set (CLANG_DEFAULT_UNWINDLIB "libgcc" CACHE STRING "" FORCE)
-  elseif (CLANG_DEFAULT_RTLIBS STREQUAL "libunwind")
+  elseif (CLANG_DEFAULT_RTLIB STREQUAL "libunwind")
 set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)
   endif()
 endif()


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -262,7 +262,7 @@
 if (CLANG_DEFAULT_UNWINDLIB STREQUAL "")
   if (CLANG_DEFAULT_RTLIB STREQUAL "libgcc")
 set (CLANG_DEFAULT_UNWINDLIB "libgcc" CACHE STRING "" FORCE)
-  elseif (CLANG_DEFAULT_RTLIBS STREQUAL "libunwind")
+  elseif (CLANG_DEFAULT_RTLIB STREQUAL "libunwind")
 set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)
   endif()
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits