[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1103
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 

`Args.hasArg(OPT_fxray_ignore_loops);`

There is no need to check a fno option in cc1, if the option is always 
enabled/disabled by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73842



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

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/rename_check.py:172-174
+  lineMatcher = re.compile('Renamed checks')
+  nextSectionMatcher = re.compile('Improvements to include-fixer')
+  checkMatcher = re.compile('- The \'(.*)')

What's the preferred variable naming convention in Python in LLVM? Looking at 
the two scripts now, I see that the naming style is already quite inconsistent. 
It would be nice to make naming consistent in these scripts.

Not an action item for this patch, just a drive-by comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73580



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

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73841



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

[PATCH] D73843: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62371 tests passed, 0 failed 
and 839 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73843



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

[PATCH] D73843: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: cfe-commits, kbarton, nemanjai.
Herald added a project: clang.
njames93 retitled this revision from "[clang-tidt] Fix false positive for 
cppcoreguidelines-init-variables" to "[clang-tidy] Fix false positive for 
cppcoreguidelines-init-variables".
njames93 edited the summary of this revision.
njames93 added reviewers: aaron.ballman, alexfh, hokein, JonasToth, gribozavr2.
njames93 added a project: clang-tools-extra.
Herald added subscribers: wuzish, xazax.hun.
njames93 marked an inline comment as done.
njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:37
+  optionally(hasParent(declStmt(hasParent(
+  
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))

No point checking if its a template definition or not. According to the 
standard it's not possible to have a for range statement where the loop 
variable isn't initialized.


Fixes False positive for cppcoreguidelines-init-variables in range based for 
loop in template function 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73843

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -78,3 +78,9 @@
   int parens(42);
   int braces{42};
 }
+
+template 
+void f(RANGE r) {
+  for (char c : r) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -29,11 +29,15 @@
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(varDecl(unless(hasInitializer(anything())),
- unless(isInstantiated()), isLocalVarDecl(),
- unless(isStaticLocal()), isDefinition())
- .bind("vardecl"),
- this);
+  std::string BadDecl = "badDecl";
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
+  isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  optionally(hasParent(declStmt(hasParent(
+  
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))
+  .bind("vardecl"),
+  this);
 }
 
 void InitVariablesCheck::registerPPCallbacks(const SourceManager ,


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -78,3 +78,9 @@
   int parens(42);
   int braces{42};
 }
+
+template 
+void f(RANGE r) {
+  for (char c : r) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -29,11 +29,15 @@
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(varDecl(unless(hasInitializer(anything())),
- unless(isInstantiated()), isLocalVarDecl(),
- unless(isStaticLocal()), isDefinition())
- .bind("vardecl"),
- this);
+  std::string BadDecl = "badDecl";
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
+  isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  optionally(hasParent(declStmt(hasParent(
+  cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))
+  .bind("vardecl"),
+  this);
 }
 
 void InitVariablesCheck::registerPPCallbacks(const SourceManager ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73843: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

[Nathan James via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:37
+  optionally(hasParent(declStmt(hasParent(
+  
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))

No point checking if its a template definition or not. According to the 
standard it's not possible to have a for range statement where the loop 
variable isn't initialized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73843



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

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque created this revision.
ianlevesque added reviewers: hiraditya, smeenai, dberris.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The function attributes xray-skip-entry, xray-skip-exit, and
xray-ignore-loops were only being applied if a function had an
xray-instrument attribute, but they should apply if xray is enabled
globally too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1100,7 +1100,7 @@
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,26 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionExit)) {
-  Fn->addFnAttr("xray-skip-exit");
-}
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionEntry)) {
-  Fn->addFnAttr("xray-skip-entry");
-}
   }
 } else {
   if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
+}
+
+if (ShouldXRayInstrumentFunction()) {
   if (CGM.getCodeGenOpts().XRayIgnoreLoops) {
 Fn->addFnAttr("xray-ignore-loops");
   }
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionExit)) {
+Fn->addFnAttr("xray-skip-exit");
+  }
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionEntry)) {
+Fn->addFnAttr("xray-skip-entry");
+  }
 }
 
 unsigned Count, Offset;
Index: clang/include/clang/Driver/XRayArgs.h
===
--- clang/include/clang/Driver/XRayArgs.h
+++ clang/include/clang/Driver/XRayArgs.h
@@ -30,6 +30,7 @@
   bool XRayAlwaysEmitCustomEvents = false;
   bool XRayAlwaysEmitTypedEvents = false;
   bool XRayRT = true;
+  bool XRayIgnoreLoops = false;
 
 public:
   /// Parses the XRay arguments from an argument list.


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1100,7 +1100,7 @@
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62371 tests passed, 0 failed 
and 839 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73841



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

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62371 tests passed, 0 failed 
and 839 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73841



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

[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin abandoned this revision.
AlexanderLanin added a comment.

This is now detected by cppcoreguidelines-macro-usage, so this seems out of 
date.


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

https://reviews.llvm.org/D29692



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

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

[Nathan James via Phabricator via cfe-commits]

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

- new line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73841

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' 
[readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;


Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' [readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73841

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' 
[readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;


Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' [readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72730: [clang-tidy] run-clang-tidy -export-fixes exports only fixes, not all warnings

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin updated this revision to Diff 241895.
AlexanderLanin added a comment.

Review findings applied (no real measurable difference in speed, maybe 100ms) 
but it's indeed more readable.
Without this fix the export took 12.96 seconds, with this patch 11.07 seconds.
Difference is even bigger when there are less auto fixable findings (70% with 
FIX-IT in my example).

As there is no test suite available I run this without and with the change and 
manually compared the output.
All warnings without FIX-IT have disappeared from exported, all warnings with 
FIX-IT are unmodified.


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

https://reviews.llvm.org/D72730

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -105,6 +105,16 @@
   start.append(f)
   return start
 
+# returns whether passed yaml Diagnostic (see mergekey) element contains FIX-IT
+# FIX-IT replacements can be within the warning directly or within the notes.
+def isAutoFixable(diag):
+  if diag["DiagnosticMessage"]["Replacements"]: return True
+
+  for note in diag.get("Notes", []):
+if note.get("Replacements", []):
+  return True
+
+  return False
 
 def merge_replacement_files(tmpdir, mergefile):
   """Merge all replacement files in a directory into a single file"""
@@ -116,7 +126,9 @@
 content = yaml.safe_load(open(replacefile, 'r'))
 if not content:
   continue # Skip empty files.
-merged.extend(content.get(mergekey, []))
+for d in content.get(mergekey, []):
+  if isAutoFixable(d):
+merged.append(d)
 
   if merged:
 # MainSourceFile: The key is required by the definition inside


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -105,6 +105,16 @@
   start.append(f)
   return start
 
+# returns whether passed yaml Diagnostic (see mergekey) element contains FIX-IT
+# FIX-IT replacements can be within the warning directly or within the notes.
+def isAutoFixable(diag):
+  if diag["DiagnosticMessage"]["Replacements"]: return True
+
+  for note in diag.get("Notes", []):
+if note.get("Replacements", []):
+  return True
+
+  return False
 
 def merge_replacement_files(tmpdir, mergefile):
   """Merge all replacement files in a directory into a single file"""
@@ -116,7 +126,9 @@
 content = yaml.safe_load(open(replacefile, 'r'))
 if not content:
   continue # Skip empty files.
-merged.extend(content.get(mergekey, []))
+for d in content.get(mergekey, []):
+  if isAutoFixable(d):
+merged.append(d)
 
   if merged:
 # MainSourceFile: The key is required by the definition inside
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 268e57b - [clang][driver] Remove an unused parameter. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-02-01T16:18:05-05:00
New Revision: 268e57bd35d7e05928820ad90f325e19e7a809d0

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

LOG: [clang][driver] Remove an unused parameter. NFC.

- Group relevant code together.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 510dc19f7a90..c8195f0f4ccf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3607,8 +3607,7 @@ static DwarfFissionKind getDebugFissionKind(const Driver 
,
 
 static void RenderDebugOptions(const ToolChain , const Driver ,
const llvm::Triple , const ArgList ,
-   bool EmitCodeView, bool IsWindowsMSVC,
-   ArgStringList ,
+   bool EmitCodeView, ArgStringList ,
codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
@@ -4651,8 +4650,8 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 AddClangCLArgs(Args, InputType, CmdArgs, , );
 
   DwarfFissionKind DwarfFission;
-  RenderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, IsWindowsMSVC,
- CmdArgs, DebugInfoKind, DwarfFission);
+  RenderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, CmdArgs,
+ DebugInfoKind, DwarfFission);
 
   // Add the split debug info name to the command lines here so we
   // can propagate it to the backend.
@@ -5352,16 +5351,16 @@ void Clang::ConstructJob(Compilation , const 
JobAction ,
RawTriple.isOSDarwin() && !KernelOrKext))
 CmdArgs.push_back("-fregister-global-dtors-with-atexit");
 
-  // -fms-extensions=0 is default.
-  if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
-   IsWindowsMSVC))
-CmdArgs.push_back("-fms-extensions");
-
   // -fno-use-line-directives is default.
   if (Args.hasFlag(options::OPT_fuse_line_directives,
options::OPT_fno_use_line_directives, false))
 CmdArgs.push_back("-fuse-line-directives");
 
+  // -fms-extensions=0 is default.
+  if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
+   IsWindowsMSVC))
+CmdArgs.push_back("-fms-extensions");
+
   // -fms-compatibility=0 is default.
   bool IsMSVCCompat = Args.hasFlag(
   options::OPT_fms_compatibility, options::OPT_fno_ms_compatibility,



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

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

@rsmith Could you have a look at this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67113



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

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D68923#1853302 , @aaronpuchert 
wrote:

> Thanks for the reviews! Do you think it makes sense to bring this to Clang 10?


I think it's a simple enough fix that it may be worth it, but it isn't fixing a 
regression in behavior so it's not critical. If it helps you out to move it 
onto the 10 branch, I think it's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Thanks for the reviews! Do you think it makes sense to bring this to Clang 10?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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

[PATCH] D73834: Update for Clang 10 release notes

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73834



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

[PATCH] D73834: Update for Clang 10 release notes

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: hans.
Mordante added a project: clang.
Herald added a subscriber: cfe-commits.

This contains the update for the release notes for D73007 
 and D73434 .

You already cherry-picked  the changes for D73007 
. 
Can you cherry-pick c03349e40f21 
 for 
D73434 ?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73834

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -72,7 +72,9 @@
 - -Wrange-loop-analysis got several improvements. It no longer warns about a
   copy being made when the result is bound to an rvalue reference. It no longer
   warns when an object of a small, trivially copyable type is copied. The
-  warning now offers fixits. It is now part of -Wall.
+  warning now offers fixits. Excluding -Wrange-loop-bind-reference it is now
+  part of -Wall. To reduce the number of false positives the diagnostic is
+  disabled in macros and template instantiations.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -72,7 +72,9 @@
 - -Wrange-loop-analysis got several improvements. It no longer warns about a
   copy being made when the result is bound to an rvalue reference. It no longer
   warns when an object of a small, trivially copyable type is copied. The
-  warning now offers fixits. It is now part of -Wall.
+  warning now offers fixits. Excluding -Wrange-loop-bind-reference it is now
+  part of -Wall. To reduce the number of false positives the diagnostic is
+  disabled in macros and template instantiations.
 
 Non-comprehensive list of changes in this release
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73434: [Sema] Remove a -Wrange warning from -Wall

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Created D73834  to cherry-pick the changes to 
the release branch and updates the release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73434



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

[PATCH] D73434: [Sema] Remove a -Wrange warning from -Wall

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc03349e40f21: [Sema] Remove a -Wrange warning from -Wall 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73434

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-range-loop-analysis.cpp


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wrange-loop-bind-reference 
-Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -15,14 +15,12 @@
 CHECK-NEXT:  -Wformat-security
 CHECK-NEXT:  -Wformat-y2k
 CHECK-NEXT:  -Wformat-invalid-specifier
+CHECK-NEXT:-Wfor-loop-analysis
 CHECK-NEXT:-Wimplicit
 CHECK-NEXT:  -Wimplicit-function-declaration
 CHECK-NEXT:  -Wimplicit-int
 CHECK-NEXT:-Winfinite-recursion
 CHECK-NEXT:-Wint-in-bool-context
-CHECK-NEXT:-Wloop-analysis
-CHECK-NEXT:  -Wfor-loop-analysis
-CHECK-NEXT:  -Wrange-loop-analysis
 CHECK-NEXT:-Wmismatched-tags
 CHECK-NEXT:-Wmissing-braces
 CHECK-NEXT:-Wmove
@@ -31,6 +29,7 @@
 CHECK-NEXT:  -Wreturn-std-move
 CHECK-NEXT:  -Wself-move
 CHECK-NEXT:-Wmultichar
+CHECK-NEXT:-Wrange-loop-construct
 CHECK-NEXT:-Wreorder
 CHECK-NEXT:  -Wreorder-ctor
 CHECK-NEXT:  -Wreorder-init-list
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2384,17 +2384,17 @@
   "loop variable %0 "
   "%diff{has type $ but is initialized with type $"
   "| is initialized with a value of a different type}1,2 resulting in a copy">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_type_or_non_reference : Note<
   "use non-reference type %0 to keep the copy or type %1 to prevent copying">;
 def warn_for_range_variable_always_copy : Warning<
   "loop variable %0 is always a copy because the range of type %1 does not "
   "return a reference">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_non_reference_type : Note<"use non-reference type %0">;
 def warn_for_range_copy : Warning<
   "loop variable %0 of type %1 creates a copy from type %2">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
 def err_objc_for_range_init_stmt : Error<
   "initialization statement is not supported when iterating over Objective-C "
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -384,7 +384,10 @@
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
   [CXX98CompatLocalTypeTemplateArgs]>;
-def RangeLoopAnalysis : DiagGroup<"range-loop-analysis">;
+def RangeLoopConstruct : DiagGroup<"range-loop-construct">;
+def RangeLoopBindReference : DiagGroup<"range-loop-bind-reference">;
+def RangeLoopAnalysis : DiagGroup<"range-loop-analysis",
+  [RangeLoopConstruct, 
RangeLoopBindReference]>;
 def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;
 def LoopAnalysis : DiagGroup<"loop-analysis", [ForLoopAnalysis,
RangeLoopAnalysis]>;
@@ -858,14 +861,15 @@
 Comment,
 DeleteNonVirtualDtor,
 Format,
+ForLoopAnalysis,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
-LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
 MultiChar,
+RangeLoopConstruct,
 Reorder,
 ReturnType,
 SelfAssignment,


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
+// RUN: 

[clang] c03349e - [Sema] Remove a -Wrange warning from -Wall

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2020-02-01T18:44:27+01:00
New Revision: c03349e40f21f0375278138992a32694a99c830e

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

LOG: [Sema] Remove a -Wrange warning from -Wall

During the review of D73007 Aaron Puchert mentioned
`warn_for_range_variable_always_copy` shouldn't be part of -Wall since
some coding styles require `for(const auto  : bars)`. This warning
would cause false positives for these users. Based on Aaron's proposal
refactored the warnings:

* -Wrange-loop-construct warns about possibly unintended constructor
  calls. This is part of -Wall. It contains
  * warn_for_range_copy: loop variable A of type B creates a copy from
type C
  * warn_for_range_const_reference_copy: loop variable A is initialized
with a value of a different type resulting in a copy
* -Wrange-loop-bind-reference warns about misleading use of reference
  types. This is not part of -Wall. It contains
  * warn_for_range_variable_always_copy: loop variable A is always a copy
because the range of type B does not return a reference

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/Misc/warning-wall.c
clang/test/SemaCXX/warn-range-loop-analysis.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index f11a69d5f2cd..12551b13e7bb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -384,7 +384,10 @@ def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
   [CXX98CompatLocalTypeTemplateArgs]>;
-def RangeLoopAnalysis : DiagGroup<"range-loop-analysis">;
+def RangeLoopConstruct : DiagGroup<"range-loop-construct">;
+def RangeLoopBindReference : DiagGroup<"range-loop-bind-reference">;
+def RangeLoopAnalysis : DiagGroup<"range-loop-analysis",
+  [RangeLoopConstruct, 
RangeLoopBindReference]>;
 def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;
 def LoopAnalysis : DiagGroup<"loop-analysis", [ForLoopAnalysis,
RangeLoopAnalysis]>;
@@ -858,14 +861,15 @@ def Most : DiagGroup<"most", [
 Comment,
 DeleteNonVirtualDtor,
 Format,
+ForLoopAnalysis,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
-LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
 MultiChar,
+RangeLoopConstruct,
 Reorder,
 ReturnType,
 SelfAssignment,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7b3f591795bb..8d2aacce2eb8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2384,17 +2384,17 @@ def warn_for_range_const_reference_copy : Warning<
   "loop variable %0 "
   "%
diff {has type $ but is initialized with type $"
   "| is initialized with a value of a 
diff erent type}1,2 resulting in a copy">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_type_or_non_reference : Note<
   "use non-reference type %0 to keep the copy or type %1 to prevent copying">;
 def warn_for_range_variable_always_copy : Warning<
   "loop variable %0 is always a copy because the range of type %1 does not "
   "return a reference">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_non_reference_type : Note<"use non-reference type %0">;
 def warn_for_range_copy : Warning<
   "loop variable %0 of type %1 creates a copy from type %2">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
 def err_objc_for_range_init_stmt : Error<
   "initialization statement is not supported when iterating over Objective-C "

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a98054e75895..737ed76859c4 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -15,14 +15,12 @@ CHECK-NEXT:  -Wnonnull
 CHECK-NEXT:  -Wformat-security
 CHECK-NEXT:  -Wformat-y2k
 CHECK-NEXT:  -Wformat-invalid-specifier
+CHECK-NEXT:-Wfor-loop-analysis
 CHECK-NEXT:-Wimplicit
 CHECK-NEXT:  -Wimplicit-function-declaration
 CHECK-NEXT:  -Wimplicit-int
 CHECK-NEXT:-Winfinite-recursion
 CHECK-NEXT:-Wint-in-bool-context
-CHECK-NEXT:-Wloop-analysis
-CHECK-NEXT:  -Wfor-loop-analysis
-CHECK-NEXT:  -Wrange-loop-analysis
 CHECK-NEXT:

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG70c98671fa7b: [clang-format] Add option for not breaking 
line before ObjC params (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1398,6 +1398,37 @@
   "}");
 }
 
+TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, NSNumber *u, "
+   "NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = c;\n"
+   "} w:self callback2:^(typeof(self) self, NSNumber *a, NSNumber "
+   "*b, NSNumber *c) {\n"
+   "  b = c;\n"
+   "}]");
+
+  Style.ColumnLimit = 80;
+  verifyFormat(
+  "[self.test_method a:self b:self\n"
+  "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+  " u = v;\n"
+  "   }]");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -506,6 +506,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -807,6 +809,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -861,8 +861,10 @@
   // Any break on this level means that the parent level has been broken
   // and we need to avoid bin packing there.
   bool NestedBlockSpecialCase =
-  !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
-  State.Stack[State.Stack.size() - 2].NestedBlockInlined;
+  (!Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
+   State.Stack[State.Stack.size() - 2].NestedBlockInlined) ||
+  (Style.Language == FormatStyle::LK_ObjC && Current.is(tok::r_brace) &&
+   State.Stack.size() > 1 && !Style.ObjCBreakBeforeNestedBlockParam);
   if (!NestedBlockSpecialCase)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
@@ -1380,7 +1382,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1688,6 +1688,29 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- 

[clang] 70c9867 - [clang-format] Add option for not breaking line before ObjC params

[via cfe-commits]

Author: mydeveloperday
Date: 2020-02-01T17:39:34Z
New Revision: 70c98671fa7b395a52829b91782393f4c2613562

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

LOG: [clang-format] Add option for not breaking line before ObjC params

Summary:
From `clang-format` version 3.7.0 and up, , there is no way to keep following 
format of ObjectiveC block:
```
- (void)_aMethod
{
[self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, 
NSNumber *v) {
u = c;
}]
}
```
Regardless of the change in `.clang-format` configuration file, all parameters 
will be lined up so that colons will be on the same column, like following:
```
- (void)_aMethod
{
[self.test1 t:self
w:self
 callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
 u = c;
 }]
}
```

Considering with ObjectiveC, the first code style is cleaner & more readable 
for some people, I've added a config option: 
`ObjCDontBreakBeforeNestedBlockParam` (boolean) so that if it is enable, the 
first code style will be favored.

Reviewed By: MyDeveloperDay

Patch By: ghvg1313

Tags: #clang, #clang-format

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTestObjC.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index fd77e1e727a9..981542451b29 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2017,6 +2017,29 @@ the configuration (without a prefix: ``Auto``).
  [self onOperationDone];
  }];
 
+**ObjCBreakBeforeNestedBlockParam** (``bool``)
+  Break parameters list into lines when there is nested block
+  parameters in a fuction call.
+
+  .. code-block:: c++
+
+false:
+ - (void)_aMethod
+ {
+ [self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, 
NSNumber *v) {
+ u = c;
+ }]
+ }
+ true:
+ - (void)_aMethod
+ {
+[self.test1 t:self
+ w:self
+callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+ u = c;
+ }]
+ }
+
 **ObjCSpaceAfterProperty** (``bool``)
   Add a space after ``@property`` in Objective-C, i.e. use
   ``@property (readonly)`` instead of ``@property(readonly)``.

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6219d6b072cb..eae0dfb12151 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -158,8 +158,11 @@ clang-format
   It helps avoid having the closing bracket align with the switch statement's
   closing bracket (when ``IndentCaseLabels`` is ``false``).
 
+- Option ``ObjCBreakBeforeNestedBlockParam`` has been added to optionally apply
+  linebreaks for function arguments declarations before nested blocks.
+
   .. code-block:: c++
-  
+
 switch (fool) {vs. switch (fool) {
 case 1:case 1: {
   {  bar();

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 3891029a6f3a..82afaa721ed9 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1688,6 +1688,29 @@ struct FormatStyle {
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self callback:^(typeof(self) self, NSNumber
+  ///*u, NSNumber *v) {
+  ///u = c;
+  ///}]
+  ///}
+  ///true:
+  ///- (void)_aMethod
+  ///{
+  ///   [self.test1 t:self
+  ///w:self
+  ///   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
+  ///u = c;
+  ///}]
+  ///}
+  /// \endcode
+  bool ObjCBreakBeforeNestedBlockParam;
+
   /// Add a space in front of an Objective-C protocol list, i.e. use
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
@@ -2178,6 +2201,8 @@ struct FormatStyle {
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
+   ObjCBreakBeforeNestedBlockParam ==
+   R.ObjCBreakBeforeNestedBlockParam &&
ObjCSpaceAfterProperty == 

[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:74
   if (ReplaceRange.getBegin().isMacroID() ||
-  ReplaceRange.getBegin() >= LastReplacementEnd) {
+  (Result.SourceManager->getFileID(ReplaceRange.getBegin()) != 
Result.SourceManager->getFileID(LastReplacementEnd)) ||
+  (ReplaceRange.getBegin() >= LastReplacementEnd)) {

Be sure to run clang-format over the patch; this looks well beyond the usual 
80-col limit. You can also drop the unnecessary parens around the sub 
expressions.

Also, a comment explaining why this code is needed would help future folks as 
would a test case showing what this corrects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73090



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D71973#1817941 , @gbencze wrote:

> Another option that came to my mind is using a BitVector to (recursively) 
> flag bits that are occupied by the fields. This solution would be slightly 
> slower and uses more memory but is probably a lot easier to understand, 
> maintain and more robust than the currently proposed implementation.  This 
> would also catch a few additional cases as it could also look inside unions.
>
> I stared to experiment with an implementation of this here 
> .
>
> Which direction do you think would be a better solution?


This is a neat approach, but I think I still prefer asking the RecordLayout for 
this information if possible.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:82-83
+
+static bool hasPadding(const ASTContext , const RecordDecl *RD,
+   uint64_t ComparedBits) {
+  assert(RD && RD->isCompleteDefinition() &&

gbencze wrote:
> aaron.ballman wrote:
> > I am surprised that we have to do this much manual work, I would have 
> > expected this to be something that `RecordLayout` would handle for us. I am 
> > a bit worried about this manual work being split in a few places because 
> > we're likely to get it wrong in at least one of them. For instance, this 
> > doesn't seem to be taking into account things like `[[no_unique_address]]` 
> > or alignment when considering the layout of fields.
> > 
> > I sort of wonder if we should try using the `RecordLayout` class to do this 
> > work instead, even if that requires larger changes.
> Thanks, I didn't realize the [[no_unique_address]] attribute existed but I 
> fixed it (in this check for now) and also added some tests to cover it as 
> well as alignas. 
> 
> If you think this information should be provided by RecordLayout I'm willing 
> to work on that as well (though I guess those changes should be in a 
> different patch). Do you have any ideas as to how it should expose the 
> necessary information? 
I do think this should be provided by `RecordLayout` (and yes, please as a 
separate patch -- feel free to add myself and Richard Smith as reviewers for 
it). Intuitively, I would expect to be able to query for whether a record has 
any padding within it whatsoever. For your needs, I would imagine an interface 
like `bool hasPaddingWithin(CharUnits InitialOffset, CharUnits FinalOffset)` 
would also help -- though I suspect we may not want to use `CharUnits` because 
of bit-fields that may have padding due to 0-sized bit-fields.


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

https://reviews.llvm.org/D71973



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:47
+CheckFactories.registerCheck(
+"cppcoreguidelines-prefer-member-initializer");
 CheckFactories.registerCheck(

Please keep this list sorted alphabetically.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43
+static bool isLiteral(const Expr *E) {
+  return isa(E) ||
+ isa(E) ||

What about other kinds of literals like user-defined literals, or literal class 
types? Should this be using `E->getType()->isLiteralType()`?


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

https://reviews.llvm.org/D71199



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

[PATCH] D73768: clang-format: [JS] document InsertTrailingCommas.

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73768



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 241874.
Wawha marked an inline comment as done.
Wawha added a comment.

@MyDeveloperDay
I launch docs/tools/dump_style.py on ClangFormatStyleOptions.rst (it seems to 
generate the same result as the previous diff) and add an entry in the Release 
Note. Tell me if there other documentation to update.

I also removed the "auto" keyword! And rebase with the last version of master 
to check the compilation and UnitTest are still OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12673,6 +12673,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13962,6 +13963,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   

[PATCH] D73651: [OpenCL][CUDA][HIP][SYCL] Add norecurse

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 241877.
yaxunl added a comment.

Fix SYCL version.


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

https://reviews.llvm.org/D73651

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGenCUDA/norecurse.cu
  clang/test/CodeGenOpenCL/norecurse.cl
  clang/test/SemaCUDA/call-kernel-from-kernel.cu


Index: clang/test/SemaCUDA/call-kernel-from-kernel.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/call-kernel-from-kernel.cu
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - 
\
+// RUN:   -verify -fsyntax-only -verify-ignore-unexpected=note
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1();
+__global__ void kernel2() {
+  kernel1<<<1,1>>>(); // expected-error {{reference to __global__ function 
'kernel1' in __global__ function}}
+}
Index: clang/test/CodeGenOpenCL/norecurse.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/norecurse.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -O0 -emit-llvm -o - %s | FileCheck %s
+
+kernel void kernel1(int a) {}
+// CHECK: define{{.*}}@kernel1{{.*}}#[[ATTR:[0-9]*]]
+
+// CHECK: attributes #[[ATTR]] = {{.*}}norecurse
Index: clang/test/CodeGenCUDA/norecurse.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/norecurse.cu
@@ -0,0 +1,15 @@
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -triple nvptx-nvidia-cuda -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - -x hip %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1(int a) {}
+// CHECK: define{{.*}}@_Z7kernel1i{{.*}}#[[ATTR:[0-9]*]]
+
+// CHECK: attributes #[[ATTR]] = {{.*}}norecurse
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -907,10 +907,24 @@
   // If we're in C++ mode and the function name is "main", it is guaranteed
   // to be norecurse by the standard (3.6.1.3 "The function main shall not be
   // used within a program").
-  if (getLangOpts().CPlusPlus)
-if (const FunctionDecl *FD = dyn_cast_or_null(D))
-  if (FD->isMain())
-Fn->addFnAttr(llvm::Attribute::NoRecurse);
+  //
+  // OpenCL C 2.0 v2.2-11 s6.9.i:
+  // Recursion is not supported.
+  //
+  // OpenCL C++ 1.0 v2.1-11 s2.9:
+  // recursive function calls (ISO C++ Section 5.2.2, item 9) unless
+  // they are a compile-time constant expression.
+  //
+  // SYCL v1.2.1 s3.10:
+  // kernels cannot include RTTI information, exception classes,
+  // recursive code, virtual functions or make use of C++ libraries that
+  // are not compiled for the device.
+  if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if ((getLangOpts().CPlusPlus && FD->isMain()) || getLangOpts().OpenCL ||
+getLangOpts().SYCLIsDevice ||
+(getLangOpts().CUDA && FD->hasAttr()))
+  Fn->addFnAttr(llvm::Attribute::NoRecurse);
+  }
 
   if (const FunctionDecl *FD = dyn_cast_or_null(D))
 if (FD->usesFPIntrin())


Index: clang/test/SemaCUDA/call-kernel-from-kernel.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/call-kernel-from-kernel.cu
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
+// RUN:   -verify -fsyntax-only -verify-ignore-unexpected=note
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1();
+__global__ void kernel2() {
+  kernel1<<<1,1>>>(); // expected-error {{reference to __global__ function 'kernel1' in __global__ function}}
+}
Index: clang/test/CodeGenOpenCL/norecurse.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/norecurse.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -O0 -emit-llvm -o - %s | FileCheck %s
+
+kernel void kernel1(int a) {}
+// CHECK: define{{.*}}@kernel1{{.*}}#[[ATTR:[0-9]*]]
+
+// CHECK: attributes #[[ATTR]] = {{.*}}norecurse
Index: clang/test/CodeGenCUDA/norecurse.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/norecurse.cu
@@ -0,0 +1,15 @@
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -triple nvptx-nvidia-cuda -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - -x hip %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1(int a) {}
+// CHECK: 

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 241875.
Wawha marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12673,6 +12673,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13962,6 +13963,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat(
+  "FctWithLongLineInLambda_SLS_Empty(\n"
+  "[]()\n"
+  "{\n"
+  "  return HereAVeryLongLine(ThatWillBeFormatted, OnMultipleLine,\n"
+  "   AndShouldNotBeConsiderAsInline,\n"
+  "   LambdaBodyMustBeBreak);\n"
+  "});",
+  LLVMWithBeforeLambdaBody);
+
+  

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62372 tests passed, 0 failed 
and 839 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 241872.
bader added a comment.

Fix clang-format and clang-tidy issues reported by merge_guards_bot


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL,CHECK-SYCL-STD %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2545,6 +2545,25 @@
   LangStd = OpenCLLangStd;
   }
 
+  // -sycl-std applies to any SYCL source, not only those containing kernels,
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(
+llvm::StringSwitch(A->getValue())
+.Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+   LangOptions::SYCLVersionList::SYCL_2015)
+.Default(LangOptions::SYCLVersionList::undefined));
+
+if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::undefined) {
+  // User has passed an invalid value to the flag, this is an error
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+}
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+ Args.hasArg(options::OPT_fsycl)) {
+Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);
+  }
+
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4010,6 +4010,18 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
+  if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+A->render(Args, CmdArgs);
+  } else if (IsSYCL) {
+// Ensure the default version in SYCL mode is 1.2.1
+CmdArgs.push_back("-sycl-std=1.2.1");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
@@ -5259,6 +5271,9 @@
options::OPT_fno_hip_new_launch_api, false))
 CmdArgs.push_back("-fhip-new-launch-api");
 
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+
   if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
 

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

[Nathan James via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75
+  continue;
+if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart))
+  return; // Found a good candidate for matching decl

tonyelewis wrote:
> njames93 wrote:
> > tonyelewis wrote:
> > > njames93 wrote:
> > > > gribozavr2 wrote:
> > > > > njames93 wrote:
> > > > > > gribozavr2 wrote:
> > > > > > > njames93 wrote:
> > > > > > > > gribozavr2 wrote:
> > > > > > > > > This heuristic should be a lot more complex. In practice 
> > > > > > > > > people have more complex naming conventions (for example, in 
> > > > > > > > > Clang, Sema.h corresponds to many files named 
> > > > > > > > > SemaSomethingSomething.cpp).
> > > > > > > > > 
> > > > > > > > > I think there is a high chance that checking only a header 
> > > > > > > > > with a matching name will satisfy too few projects to be 
> > > > > > > > > worth implementing.
> > > > > > > > > 
> > > > > > > > > On the other hand, if you could use C++ or Clang modules to 
> > > > > > > > > narrow down the list of headers where the declaration should 
> > > > > > > > > appear, that would be a much stronger signal.
> > > > > > > > That is the reason I added the CheckAnyHeader option. For small 
> > > > > > > > projects the matching name would be usually be enough, but for 
> > > > > > > > big complex projects there is no feasible check that would 
> > > > > > > > work. Falling back to making sure every external definition has 
> > > > > > > > a declaration in at least one header will always be a good 
> > > > > > > > warning
> > > > > > > That's the thing -- due to the lack of consistency in projects 
> > > > > > > and style guides for C++, I think most projects will have to turn 
> > > > > > > on CheckAnyHeader. We get implementation complexity in ClangTidy, 
> > > > > > > false positives in high-profile projects, force users to 
> > > > > > > configure ClangTidy to work well for their projects (unless we 
> > > > > > > set CheckAnyHeader=1 by default... which then nobody would flip 
> > > > > > > back to zero), and little benefit for end users.
> > > > > > Would you say the best way would be check if the source file name 
> > > > > > starts with the header file name by default. Or is that very 
> > > > > > specific to Clang?
> > > > > > 
> > > > > > ```
> > > > > > /// 
> > > > > > #include "SomeHeader.h"
> > > > > > ```
> > > > > > This file would check for declarations in `SomeHeader.h`
> > > > > > 
> > > > > > Or maybe check if the header file name starts with the source file?
> > > > > > 
> > > > > > ```
> > > > > > /// 
> > > > > > #include "SomeHeaderImpl.h"
> > > > > > ```
> > > > > > This file would check for declarations in `SomeHeaderImpl.h`.
> > > > > > Or even check both?
> > > > > > Would you say the best way would be check if the source file name 
> > > > > > starts with the header file name by default. Or is that very 
> > > > > > specific to Clang?
> > > > > 
> > > > > I don't think it is too specific, it should cover many projects I 
> > > > > think.
> > > > > 
> > > > > > Or maybe check if the header file name starts with the source file?
> > > > > 
> > > > > Seems like an unusual case to me, but I'm willing to be convinced 
> > > > > otherwise.
> > > > I thought it was an unusual case and wasn't thinking it would be a good 
> > > > idea to add. I'll just set it up so that it will always look in header 
> > > > files whose names appear at the start of the main source file.
> > > FWIW, my instinct is that there are two separate questions:
> > > 
> > >  1. What sort of files should have their external-linkage definitions 
> > > checked?
> > >  2. What sort of included files should be part of the search for adequate 
> > > declarations that match those definitions?
> > > 
> > > ...and that my answers, in reverse order, are:
> > > 
> > >  2. All included files should be included (ie a check for a given 
> > > external-linkage definition should be appeased by _any_ matching 
> > > declaration in _any_ included file)
> > >  1. Only main files. And (to prevent lots of false-positives when the 
> > > tool is run over headers), only "c", "cc", "cxx", "cpp" files by default 
> > > but with an option to check in _all_ main files.
> > > 
> > > I think this has the merits that it:
> > >  * allows users to put their valid declaration in files with any 
> > > extension they like
> > >  * defaults to only firing when run against c/cc/cxx/cpp files but 
> > > provides the option to fire when run against a file with _any_ extension
> > > 
> > > 
> > > So I propose that there be one option and it be as follows:
> > > 
> > > 
> > > > .. option:: CheckExtDefnsInAllMainFiles
> > > > 
> > > > If set to `0` only check external-linkage definitions in main files 
> > > > with typical source-file extensions ("c", "cc", "cxx", "cpp").
> > > > If set to `1` 

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

So I tried to ran `./rename_check.py readability-braces-around-statements 
readability-braces` and results were less than desirable. It renamed the 
`readability-braces-around-statements` but it also renamed the google alias to 
`google-readability-braces`. In the documentation for google-readability-braces 
it changed the subject line to

  google-readability-braces
  ===

Feel like the correct course of action is to match against the entire check 
name rather than a sub string of it (which i guess was unintentional). This 
doesn't need to be done in this review and could (should) be a follow up.

The actual alphabetical ordering of the release notes works as intended though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73580



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

[clang] 1acf129 - [Frontend] Delete a redundant check of -pg for setFramePointer()

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-02-01T00:29:29-08:00
New Revision: 1acf129bcf9a1b51e301a9fef151254ec4c7ec43

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

LOG: [Frontend] Delete a redundant check of -pg for setFramePointer()

Driver errors if -fomit-frame-pointer is used together with -pg.
useFramePointerForTargetByDefault() returns true if -pg is specified.
=>
(!OmitFP && useFramePointerForTargetByDefault(Args, Triple)) is true
=>
We cannot get FramePointerKind::None

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/x86_64-profiling-keep-fp.c

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 335f8cd0e323..f3015d0f7085 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -896,11 +896,6 @@ static bool ParseCodeGenArgs(CodeGenOptions , ArgList 
, InputKind IK,
   Opts.setFramePointer(FP);
   }
 
-  // -pg may override -mframe-pointer
-  // TODO: This should be merged into getFramePointerKind in Clang.cpp.
-  if (Args.hasArg(OPT_pg))
-Opts.setFramePointer(CodeGenOptions::FramePointerKind::All);
-
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);

diff  --git a/clang/test/CodeGen/x86_64-profiling-keep-fp.c 
b/clang/test/CodeGen/x86_64-profiling-keep-fp.c
index a51d87fa6365..c22fafd74949 100644
--- a/clang/test/CodeGen/x86_64-profiling-keep-fp.c
+++ b/clang/test/CodeGen/x86_64-profiling-keep-fp.c
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -pg -S -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -mframe-pointer=non-leaf 
-pg -S -o - %s | \
 // RUN:   FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -mframe-pointer=all -pg 
-S -o - %s | \
 // RUN:   FileCheck %s



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