[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:26
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);

Eugene.Zelenko wrote:
> `auto` could be used because type is spelled in same statement.
No need for `dyn_cast` here, we already know it's an `Expr` so `cast` would be 
sufficient.



Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:28
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;

Surely we can bail if only one of the sides contains an errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128402

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

[PATCH] D128372: Clang-Tidy Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Tests locations were changed recently too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please add documentation for check.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:121
+- New :doc:`bugprone-standalone-empty
+  ` check.
+

Path to documentation was changed recently.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:39
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, 
"Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion 
`_, "Yes"
-   `abseil-faster-strsplit-delimiter 
`_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, 
"Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions 
`_, "Yes"
-   `altera-id-dependent-backward-branch 
`_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   `android-cloexec-pipe2 `_, "Yes"
-   `android-cloexec-socket `_, "Yes"
-   `android-comparison-in-temp-failure-retry 
`_,
-   `boost-use-to-string `_, "Yes"
-   `bugprone-argument-comment `_, "Yes"
-   `bugprone-assert-side-effect `_,
-   `bugprone-bad-signal-to-kill-thread 
`_,
-   `bugprone-bool-pointer-implicit-conversion 
`_, "Yes"
-   `bugprone-branch-clone `_,
-   `bugprone-copy-constructor-init `_, 
"Yes"
-   `bugprone-dangling-handle `_,
-   `bugprone-dynamic-static-initializers 
`_,
-   `bugprone-easily-swappable-parameters 
`_,
-   `bugprone-exception-escape `_,
-   `bugprone-fold-init-type `_,
-   `bugprone-forward-declaration-namespace 
`_,
-   `bugprone-forwarding-reference-overload 
`_,
-   `bugprone-implicit-widening-of-multiplication-result 
`_, "Yes"
-   `bugprone-inaccurate-erase `_, "Yes"
-   `bugprone-incorrect-roundings `_,
-   `bugprone-infinite-loop `_,
-   `bugprone-integer-division `_,
-   `bugprone-lambda-function-name `_,
-   `bugprone-macro-parentheses `_, "Yes"
-   `bugprone-macro-repeated-side-effects 
`_,
-   `bugprone-misplaced-operator-in-strlen-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-pointer-arithmetic-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-widening-cast `_,
-   `bugprone-move-forwarding-reference 
`_, "Yes"
-   `bugprone-multiple-statement-macro 
`_,
-   `bugprone-no-escape `_,
-   `bugprone-not-null-terminated-result 
`_, "Yes"
-   `bugprone-parent-virtual-call `_, "Yes"
-   `bugprone-posix-return `_, "Yes"
-   `bugprone-redundant-branch-condition 
`_, "Yes"
-   `bugprone-reserved-identifier `_, "Yes"
-   `bugprone-shared-ptr-array-mismatch 
`_, "Yes"
-   `bugprone-signal-handler `_,
-   `bugprone-signed-char-misuse `_,
-   `bugprone-sizeof-container `_,
-   `bugprone-sizeof-expression `_,
-   `bugprone-spuriously-wake-up-functions 
`_,
-   `bugprone-string-constructor `_, "Yes"
-   `bugprone-string-integer-assignment 
`_, "Yes"
-   `bugprone-string-literal-with-embedded-nul 
`_,
-   `bugprone-stringview-nullptr `_, "Yes"
-   `bugprone-suspicious-enum-usage `_,
-   `bugprone-suspicious-include `_,
-   `bugprone-suspicious-memory-comparison 
`_,
-   `bugprone-suspicious-memset-usage 
`_, "Yes"
-   `bugprone-suspicious-missing-comma 
`_,
-   `bugprone-suspicious-semicolon `_, "Yes"
-   `bugprone-suspicious-string-compare 
`_, "Yes"
-   `bugprone-swapped-arguments `_, "Yes"
-   `bugprone-terminating-continue `_, "Yes"
-   `bugprone-throw-keyword-missing `_,
-   `bugprone-too-small-loop-variable `_,
-   `bugprone-unchecked-optional-access 
`_,
-   `bugprone-undefined-memory-manipulation 
`_,
-   `bugprone-undelegated-constructor `_,
-   `bugprone-unhandled-exception-at-new 
`_,
-   `bugprone-unhandled-self-assignment 
`_,
-   `bugprone-unused-raii `_, "Yes"
-   `bugprone-unused-return-value `_,
-   `bugprone-use-after-move `_,
-   `bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_, "Yes"
-   `cert-dcl50-cpp `_,
-   `cert-dcl58-cpp `_,
-   `cert-env33-c `_,
-   `cert-err33-c `_,
-   `cert-err34-c `_,
-   `cert-err52-cpp `_,
-   `cert-err58-cpp `_,
-   `cert-err60-cpp `_,
-   `cert-flp30-c `_,
-   `cert-mem57-cpp `_,
-   `cert-msc50-cpp `_,
-   

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:26
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);

`auto` could be used because type is spelled in same statement.



Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:27
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {

Ditto.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

Documentation path was changed recently. Please also keep alphabetical order 
inside section.


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

https://reviews.llvm.org/D128402

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

[PATCH] D128406: clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body

[Brad Moody via Phabricator via cfe-commits]

bmoody created this revision.
bmoody added reviewers: rnk, gribozavr.
Herald added a project: All.
bmoody requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use the if/while statement right paren location instead of the end of the
condition expression to determine if the semicolon is on its own line, for the
purpose of not warning about code like this:

  while (foo())
;

Using the condition location meant that we would also not report a warning on
code like this:

  while (MACRO(a,
   b));
body();

The right paren loc wasn't stored in the AST or passed into Sema::ActOnIfStmt
when this logic was first written.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128406

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
  clang/test/SemaCXX/warn-empty-body.cpp


Index: clang/test/SemaCXX/warn-empty-body.cpp
===
--- clang/test/SemaCXX/warn-empty-body.cpp
+++ clang/test/SemaCXX/warn-empty-body.cpp
@@ -6,6 +6,8 @@
 
 #define MACRO_A 0
 
+#define AND(x, y) ((x) && (y))
+
 void test1(int x, int y) {
   while(true) {
 if (x); // expected-warning {{if statement has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
@@ -15,6 +17,15 @@
 if (x == MACRO_A); // expected-warning {{if statement has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
 if (MACRO_A == x); // expected-warning {{if statement has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
 
+// Check that we handle the case where the condition comes from a macro
+// expansion over multiple lines.
+if (AND(b(),
+c())); // expected-warning {{if statement has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
+
+while (AND(b(),
+   c())); // expected-warning{{while loop has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
+  a(0);
+
 int i;
 // PR11329
 for (i = 0; i < x; i++); { // expected-warning{{for loop has empty body}} 
expected-note{{put the semicolon on a separate line to silence this warning}}
Index: clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -63,8 +63,6 @@
   // expected-note@-1 {{to match this '('}}
   // expected-error@-2 {{expected ';' after expression}}
   // expected-error@-3 {{expected expression}}
-  // expected-warning@-4 {{while loop has empty body}}
-  // expected-note@-5 {{put the semicolon on a separate line to silence this 
warning}}
 }
 
 // TODO: This is needed because clang can't seem to diagnose invalid syntax 
after the
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -888,8 +888,7 @@
 CommaVisitor(*this).Visit(CondExpr);
 
   if (!ConstevalOrNegatedConsteval && !elseStmt)
-DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
-  diag::warn_empty_if_body);
+DiagnoseEmptyStmtBody(RParenLoc, thenStmt, diag::warn_empty_if_body);
 
   if (ConstevalOrNegatedConsteval ||
   StatementKind == IfStatementKind::Constexpr) {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -16673,7 +16673,7 @@
 Body = FS->getBody();
 DiagID = diag::warn_empty_for_body;
   } else if (const WhileStmt *WS = dyn_cast(S)) {
-StmtLoc = WS->getCond()->getSourceRange().getEnd();
+StmtLoc = WS->getRParenLoc();
 Body = WS->getBody();
 DiagID = diag::warn_empty_while_body;
   } else


Index: clang/test/SemaCXX/warn-empty-body.cpp
===
--- clang/test/SemaCXX/warn-empty-body.cpp
+++ clang/test/SemaCXX/warn-empty-body.cpp
@@ -6,6 +6,8 @@
 
 #define MACRO_A 0
 
+#define AND(x, y) ((x) && (y))
+
 void test1(int x, int y) {
   while(true) {
 if (x); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
@@ -15,6 +17,15 @@
 if (x == MACRO_A); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
 if (MACRO_A == x); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
 
+// Check that we handle the case where the condition comes from a macro
+// expansion over multiple lines.
+if (AND(b(),
+  

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D128328#3603781 , @vsapsai wrote:

> From the perspective of handling `err_export_inline_not_defined` error as a 
> developer what about the following option?
>
>   export inline void fn_e(); // note: function 'fn_e' exported as 'inline' 
> here
>   
>   // ...
>   module :private;
>   void fn_e() {}  // error: definition of function 'fn_e' can not be inlined 
> because it is private
>
> My suggestion isn't about specific wording but about emitting an error for 
> the definition and mention declaration in the note. Will it make easier to 
> explain the situation? Because I'm not a fan of "must be defined within the 
> module purview" and don't know how digestible it will be for others. Please 
> note that the suggestion is purely from user perspective, I haven't checked 
> how it might affect the implementation.

The suggested diagnostic message might not be right since the following one 
should be legal:

  export module m;
  ...
  module :private;
  // foo() is not declared earlier.
  inline void foo() {...}

The definition of 'foo' here should be legal although it is inline and is in 
private fragment.




Comment at: clang/test/Modules/cxx20-10-5-ex1.cpp:25-28
+export void g(X *x) {
+  fn_s();
+  fn_m();
+}

vsapsai wrote:
> Can `export inline` function call other non-`export inline` functions? Sorry 
> if it is tested somewhere else. Curious what are the transitive restrictions, 
> so we test edge cases.
I guess it is not tested. But a non-export function wouldn't be exported if it 
is called by export function from the perspective of std. Although more tests 
should be fine all the time, the current test case should come from the example 
in the standard. We could send another test if we want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D128351: [clang] missing outer template levels when checking template constraints

[Sirui Mu via Phabricator via cfe-commits]

Lancern updated this revision to Diff 439253.
Lancern added a comment.

Fix some regression issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128351

Files:
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaCXX/template-member-alias-constraint.cpp


Index: clang/test/SemaCXX/template-member-alias-constraint.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-member-alias-constraint.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+
+template 
+concept Foo = true;
+
+template 
+struct Bar {};
+
+template 
+struct Baz {
+  template 
+  using BazBar = Bar;
+
+  using BazBarInt = BazBar; // expected-no-diagnostics
+};
+
+template 
+struct contiguous_range {
+  template 
+requires(const_range == false)
+  using basic_iterator = int;
+
+  auto begin() {
+return basic_iterator{}; // expected-no-diagnostics
+  }
+};
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -293,6 +293,10 @@
 
   MultiLevelTemplateArgumentList MLTAL;
   MLTAL.addOuterTemplateArguments(TemplateArgs);
+  if (const TypeAliasTemplateDecl *AliasTemplate =
+  dyn_cast_or_null(Template)) {
+MLTAL.addOuterRetainedLevels(AliasTemplate->getTemplateDepth());
+  }
 
   for (const Expr *ConstraintExpr : ConstraintExprs) {
 if (calculateConstraintSatisfaction(S, Template, TemplateArgs,


Index: clang/test/SemaCXX/template-member-alias-constraint.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-member-alias-constraint.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+
+template 
+concept Foo = true;
+
+template 
+struct Bar {};
+
+template 
+struct Baz {
+  template 
+  using BazBar = Bar;
+
+  using BazBarInt = BazBar; // expected-no-diagnostics
+};
+
+template 
+struct contiguous_range {
+  template 
+requires(const_range == false)
+  using basic_iterator = int;
+
+  auto begin() {
+return basic_iterator{}; // expected-no-diagnostics
+  }
+};
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -293,6 +293,10 @@
 
   MultiLevelTemplateArgumentList MLTAL;
   MLTAL.addOuterTemplateArguments(TemplateArgs);
+  if (const TypeAliasTemplateDecl *AliasTemplate =
+  dyn_cast_or_null(Template)) {
+MLTAL.addOuterRetainedLevels(AliasTemplate->getTemplateDepth());
+  }
 
   for (const Expr *ConstraintExpr : ConstraintExprs) {
 if (calculateConstraintSatisfaction(S, Template, TemplateArgs,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

From the perspective of handling `err_export_inline_not_defined` error as a 
developer what about the following option?

  export inline void fn_e(); // note: function 'fn_e' exported as 'inline' here
  
  // ...
  module :private;
  void fn_e() {}  // error: definition of function 'fn_e' can not be inlined 
because it is private

My suggestion isn't about specific wording but about emitting an error for the 
definition and mention declaration in the note. Will it make easier to explain 
the situation? Because I'm not a fan of "must be defined within the module 
purview" and don't know how digestible it will be for others. Please note that 
the suggestion is purely from user perspective, I haven't checked how it might 
affect the implementation.




Comment at: clang/test/Modules/cxx20-10-5-ex1.cpp:25-28
+export void g(X *x) {
+  fn_s();
+  fn_m();
+}

Can `export inline` function call other non-`export inline` functions? Sorry if 
it is tested somewhere else. Curious what are the transitive restrictions, so 
we test edge cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155
+def err_export_inline_not_defined : Error<
+  "exported inline functions must be defined within the module purview"
+  " and before any private module fragment">;

iains wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > From my reading, 'exported' is not emphasized.
> > it is here:
> > https://eel.is/c++draft/module#private.frag-2.1
> > ( I agree it is somewhat confusing, but the export makes the linkage 
> > external, which the example treats differently from the fn_m() case which 
> > has module linkage).
> > 
> > It is possible that we might need to pull together several pieces of the 
> > std and maybe ask core for clarification?
> > it is here:
> > https://eel.is/c++draft/module#private.frag-2.1
> > ( I agree it is somewhat confusing, but the export makes the linkage 
> > external, which the example treats differently from the fn_m() case which 
> > has module linkage).
> 
> hmm... my linkage comment is wrong - however the distinction between exported 
> and odr-used seems to be made here (fn_m() and fn_e()).
> > 
> > It is possible that we might need to pull together several pieces of the 
> > std and maybe ask core for clarification?
> 
> 
What I read is:
> [dcl.inline]p7: https://eel.is/c++draft/dcl.inline#7
> If an inline function or variable that is attached to a named module is 
> declared in a definition domain, it shall be defined in that domain.

and the definition of `definition domain` is:
> [basic.def.odr]p12: https://eel.is/c++draft/basic#def.odr-12
>   A definition domain is a private-module-fragment or the portion of a 
> translation unit excluding its private-module-fragment (if any).

The definition of "attached to a named module" is:
> [module.unit]p7: https://eel.is/c++draft/module.unit#7
>  A module is either a named module or the global module. A declaration is 
> attached to a module as follows: ...

So it is clearly not consistency with [module.private.frag]p2.1. I would send 
this to WG21.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> Supporting the lowering in the backend is sensible in order to support 
> -fexcess-precision=16, because I agree that the most reasonable IR output in 
> that configuration is to simply generate half operations. But under 
> -fexcess-precision=32, I do not want the frontend to be in the business of 
> recognizing cases where promoted emission is unnecessary, because that is 
> just a lot of extra complexity for something that's already fairly 
> special-case logic; among other things, it will tend to hide bugs.

Fair enough. I agree we should choose the easy way to go. Thanks for the 
explanation!

> So I think we should hold off on adding this option until we know what 
> behavior we actually want to expose via it.

I don't have strong opinion on the specific `-fexcess-precision` option, but we 
do need an option to stop the promotion in the FE. Some user requires the 
emulation to gets the identical result as running on real HW. How about we make 
`EmitFloat16WithExcessPrecision` an Clang option and set it true by default and 
false for feature `avx512fp16`?


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

https://reviews.llvm.org/D113107

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

[PATCH] D127690: [NFC] clang/Parser: remove dead code

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 439246.
mizvekov added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127690

Files:
  clang/lib/Parse/Parser.cpp


Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -2056,36 +2056,6 @@
   return false;
 }
 
-// If this is a template-id, annotate with a template-id or type token.
-// FIXME: This appears to be dead code. We already have formed template-id
-// tokens when parsing the scope specifier; this can never form a new one.
-if (NextToken().is(tok::less)) {
-  TemplateTy Template;
-  UnqualifiedId TemplateName;
-  TemplateName.setIdentifier(Tok.getIdentifierInfo(), Tok.getLocation());
-  bool MemberOfUnknownSpecialization;
-  if (TemplateNameKind TNK = Actions.isTemplateName(
-  getCurScope(), SS,
-  /*hasTemplateKeyword=*/false, TemplateName,
-  /*ObjectType=*/nullptr, /*EnteringContext*/false, Template,
-  MemberOfUnknownSpecialization)) {
-// Only annotate an undeclared template name as a template-id if the
-// following tokens have the form of a template argument list.
-if (TNK != TNK_Undeclared_template ||
-isTemplateArgumentList(1) != TPResult::False) {
-  // Consume the identifier.
-  ConsumeToken();
-  if (AnnotateTemplateIdToken(Template, TNK, SS, SourceLocation(),
-  TemplateName)) {
-// If an unrecoverable error occurred, we need to return true here,
-// because the token stream is in a damaged state.  We may not
-// return a valid identifier.
-return true;
-  }
-}
-  }
-}
-
 // The current token, which is either an identifier or a
 // template-id, is not part of the annotation. Fall through to
 // push that token back into the stream and complete the C++ scope


Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -2056,36 +2056,6 @@
   return false;
 }
 
-// If this is a template-id, annotate with a template-id or type token.
-// FIXME: This appears to be dead code. We already have formed template-id
-// tokens when parsing the scope specifier; this can never form a new one.
-if (NextToken().is(tok::less)) {
-  TemplateTy Template;
-  UnqualifiedId TemplateName;
-  TemplateName.setIdentifier(Tok.getIdentifierInfo(), Tok.getLocation());
-  bool MemberOfUnknownSpecialization;
-  if (TemplateNameKind TNK = Actions.isTemplateName(
-  getCurScope(), SS,
-  /*hasTemplateKeyword=*/false, TemplateName,
-  /*ObjectType=*/nullptr, /*EnteringContext*/false, Template,
-  MemberOfUnknownSpecialization)) {
-// Only annotate an undeclared template name as a template-id if the
-// following tokens have the form of a template argument list.
-if (TNK != TNK_Undeclared_template ||
-isTemplateArgumentList(1) != TPResult::False) {
-  // Consume the identifier.
-  ConsumeToken();
-  if (AnnotateTemplateIdToken(Template, TNK, SS, SourceLocation(),
-  TemplateName)) {
-// If an unrecoverable error occurred, we need to return true here,
-// because the token stream is in a damaged state.  We may not
-// return a valid identifier.
-return true;
-  }
-}
-  }
-}
-
 // The current token, which is either an identifier or a
 // template-id, is not part of the annotation. Fall through to
 // push that token back into the stream and complete the C++ scope
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128276: clang: perform deduction at the right depth on Sema::AddMethodTemplateCandidate

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 439245.
mizvekov added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128276

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5-cxx20.cpp


Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5-cxx20.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+namespace pr28087 {
+template  auto apply(F f) { f(); }
+template  struct S {
+  template  auto f(decltype(apply([](auto...) {})));
+};
+template struct S<>;
+} // namespace pr28087
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -7078,7 +7078,8 @@
   //   functions. In such a case, the candidate functions generated from each
   //   function template are combined with the set of non-template candidate
   //   functions.
-  TemplateDeductionInfo Info(CandidateSet.getLocation());
+  TemplateDeductionInfo Info(CandidateSet.getLocation(),
+ MethodTmpl->getTemplateParameters()->getDepth());
   FunctionDecl *Specialization = nullptr;
   ConversionSequenceList Conversions;
   if (TemplateDeductionResult Result = DeduceTemplateArguments(
@@ -14157,8 +14158,12 @@
   IsError |= InputInit.isInvalid();
   Arg = InputInit.getAs();
 } else {
-  ExprResult DefArg =
-  S.BuildCXXDefaultArgExpr(LParenLoc, Method, Method->getParamDecl(i));
+  ParmVarDecl *Param = Method->getParamDecl(i);
+  if (!Param->hasDefaultArg()) {
+IsError = true;
+break;
+  }
+  ExprResult DefArg = S.BuildCXXDefaultArgExpr(LParenLoc, Method, Param);
   if (DefArg.isInvalid()) {
 IsError = true;
 break;


Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5-cxx20.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+namespace pr28087 {
+template  auto apply(F f) { f(); }
+template  struct S {
+  template  auto f(decltype(apply([](auto...) {})));
+};
+template struct S<>;
+} // namespace pr28087
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -7078,7 +7078,8 @@
   //   functions. In such a case, the candidate functions generated from each
   //   function template are combined with the set of non-template candidate
   //   functions.
-  TemplateDeductionInfo Info(CandidateSet.getLocation());
+  TemplateDeductionInfo Info(CandidateSet.getLocation(),
+ MethodTmpl->getTemplateParameters()->getDepth());
   FunctionDecl *Specialization = nullptr;
   ConversionSequenceList Conversions;
   if (TemplateDeductionResult Result = DeduceTemplateArguments(
@@ -14157,8 +14158,12 @@
   IsError |= InputInit.isInvalid();
   Arg = InputInit.getAs();
 } else {
-  ExprResult DefArg =
-  S.BuildCXXDefaultArgExpr(LParenLoc, Method, Method->getParamDecl(i));
+  ParmVarDecl *Param = Method->getParamDecl(i);
+  if (!Param->hasDefaultArg()) {
+IsError = true;
+break;
+  }
+  ExprResult DefArg = S.BuildCXXDefaultArgExpr(LParenLoc, Method, Param);
   if (DefArg.isInvalid()) {
 IsError = true;
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128095: clang: fix checking parameter packs for expansion

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 439244.
mizvekov added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp

Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -469,3 +469,25 @@
   bar(b);
 }
 }
+
+namespace pr56094 {
+template  struct D {
+  template  using B = int(int (*...p)(T, U));
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
+  template  D(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
+};
+using t1 = D::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::D' requested here}}
+
+template  struct F {};
+template  struct G {};
+template  struct E {
+  template  using B = G...>;
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  template  E(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
+};
+using t2 = E::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::E' requested here}}
+} // namespace pr56094
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,17 @@
   return true;
 }
 
+bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+  Unexpanded.push_back({T, SourceLocation()});
+  return true;
+}
+
+bool
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}
+
 /// Record occurrences of function and non-type template
 /// parameter packs in an expression.
 bool VisitDeclRefExpr(DeclRefExpr *E) {
@@ -306,7 +317,8 @@
   auto *TTPD = dyn_cast(LocalPack);
   return TTPD && TTPD->getTypeForDecl() == TTPT;
 }
-return declaresSameEntity(Pack.first.get(), LocalPack);
+return declaresSameEntity(Pack.first.get(),
+  LocalPack);
   };
   if (llvm::any_of(LSI->LocalPacks, DeclaresThisPack))
 LambdaParamPackReferences.push_back(Pack);
@@ -358,7 +370,7 @@
   = Unexpanded[I].first.dyn_cast())
   Name = TTP->getIdentifier();
 else
-  Name = Unexpanded[I].first.get()->getIdentifier();
+  Name = Unexpanded[I].first.get()->getIdentifier();
 
 if (Name && NamesKnown.insert(Name).second)
   Names.push_back(Name);
@@ -421,7 +433,7 @@
   llvm::SmallPtrSet ParmSet(Parms.begin(), Parms.end());
   SmallVector UnexpandedParms;
   for (auto Parm : Unexpanded)
-if (ParmSet.contains(Parm.first.dyn_cast()))
+if (ParmSet.contains(Parm.first.dyn_cast()))
   UnexpandedParms.push_back(Parm);
   if (UnexpandedParms.empty())
 return false;
@@ -681,42 +693,51 @@
 // Compute the depth and index for this parameter pack.
 unsigned Depth = 0, Index = 0;
 IdentifierInfo *Name;
-bool IsVarDeclPack = false;
+unsigned NewPackSize;
+bool NotPack = true;
 
-if (const TemplateTypeParmType *TTP =
+if (const auto *TTP =
 ParmPack.first.dyn_cast()) {
   Depth = TTP->getDepth();
   Index = TTP->getIndex();
   Name = TTP->getIdentifier();
+} else if (const auto *STP =
+   ParmPack.first
+   .dyn_cast()) {
+  NewPackSize = STP->getNumArgs();
+  Name = STP->getIdentifier();
+  NotPack = false;
+} else if (const auto *SEP =
+   ParmPack.first
+   .dyn_cast()) {
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  Name = SEP->getParameterPack()->getIdentifier();
+  NotPack = false;
 } else {
-  NamedDecl *ND = ParmPack.first.get();
-  if (isa(ND))
-IsVarDeclPack = true;
-  else
+  const auto *ND = ParmPack.first.get();
+  if (isa(ND)) {
+// Figure out whether we're instantiating to an argument pack or not.
+using DeclArgumentPack = LocalInstantiationScope::DeclArgumentPack;
+llvm::PointerUnion *Instantiation =
+CurrentInstantiationScope->findInstantiationOf(
+ParmPack.first.get());
+if (Instantiation->is()) {
+  // We could expand this function parameter pack.
+  NewPackSize = Instantiation->get()->size();
+} else 

[PATCH] D127351: clang: fix early substitution of alias templates

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 439241.
mizvekov added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127351

Files:
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-template-decls.cpp

Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -121,7 +121,7 @@
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
 // CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
-// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 1 index 0
 // CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
@@ -155,10 +155,10 @@
 // CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar pack_index 0
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent contains_unexpanded_pack depth 0 index 0 pack
 // CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar pack_index 0
-// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent contains_unexpanded_pack depth 0 index 0 pack
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent contains_unexpanded_pack depth 1 index 0 pack
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (char, short)' cdecl
 // CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar pack_index 1
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent contains_unexpanded_pack depth 0 index 0 pack
 // CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar pack_index 1
-// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent contains_unexpanded_pack depth 0 index 0 pack
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent contains_unexpanded_pack depth 1 index 0 pack
 } // namespace PR56099
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1029,8 +1029,9 @@
   // will contain the instantiations of the template parameters.
   LocalInstantiationScope Scope(SemaRef);
 
-  TemplateParameterList *TempParams = D->getTemplateParameters();
-  TemplateParameterList *InstParams = SubstTemplateParams(TempParams);
+  auto EarlySubstitutionScope = TemplateArgs.getEarlySubstitutionRAII();
+  TemplateParameterList *InstParams =
+  SubstTemplateParams(D->getTemplateParameters());
   if (!InstParams)
 return nullptr;
 
@@ -2758,7 +2759,7 @@
 
   TemplateTypeParmDecl *Inst = TemplateTypeParmDecl::Create(
   SemaRef.Context, Owner, D->getBeginLoc(), D->getLocation(),
-  D->getDepth() - TemplateArgs.getNumSubstitutedLevels(), D->getIndex(),
+  TemplateArgs.getNewDepth(D->getDepth()), D->getIndex(),
   D->getIdentifier(), D->wasDeclaredWithTypename(), D->isParameterPack(),
   D->hasTypeConstraint(), NumExpanded);
 
@@ -2911,14 +2912,14 @@
   if (IsExpandedParameterPack)
 Param = NonTypeTemplateParmDecl::Create(
 SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
-D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
-D->getPosition(), D->getIdentifier(), T, DI, ExpandedParameterPackTypes,
+TemplateArgs.getNewDepth(D->getDepth()), D->getPosition(),
+D->getIdentifier(), T, DI, ExpandedParameterPackTypes,
 ExpandedParameterPackTypesAsWritten);
   else
 Param = NonTypeTemplateParmDecl::Create(
 SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
-D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
-D->getPosition(), D->getIdentifier(), T, D->isParameterPack(), DI);
+TemplateArgs.getNewDepth(D->getDepth()), D->getPosition(),
+D->getIdentifier(), T, D->isParameterPack(), DI);
 
   if (AutoTypeLoc AutoLoc = DI->getTypeLoc().getContainedAutoTypeLoc())
 if (AutoLoc.isConstrained())
@@ -3052,13 +3053,13 @@
   if (IsExpandedParameterPack)
 Param = TemplateTemplateParmDecl::Create(
 SemaRef.Context, Owner, D->getLocation(),
-D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
-D->getPosition(), D->getIdentifier(), InstParams, ExpandedParams);
+TemplateArgs.getNewDepth(D->getDepth()), D->getPosition(),
+D->getIdentifier(), InstParams, ExpandedParams);
   else
 Param = TemplateTemplateParmDecl::Create(
 SemaRef.Context, Owner, D->getLocation(),
-D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
-D->getPosition(), 

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439240.

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

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {// CHECK-MESSAGES: :[[@LINE]]:7: error: 
use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: error: 
use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  } else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: error: 
use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {// CHECK-MESSAGES: :[[@LINE]]:7: error: use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  } else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439239.
ishaangandhi added a comment.

Include `error: ` in error line


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

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: error: use of 
undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: error: 
use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: error: 
use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: error: use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439228.
ishaangandhi added a comment.

Check for error messages in test case


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

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: use of 
undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: use of 
undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: use of 
undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

[Félix Cloutier via Phabricator via cfe-commits]

fcloutier added a comment.

Would it be better if I asked a colleague to finish the review?


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

https://reviews.llvm.org/D112579

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

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439226.
ishaangandhi added a comment.

Move test to new file with `-fix-errors` flag


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

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128397: Add handling cases when filter(tid) appears with deafult(none)

[Mike Rice via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa35141d39501: [OpenMP] Add handling cases when filter(tid) 
appears with default(none) (authored by mdfazlay, committed by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128397

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_masked_ast_print.cpp


Index: clang/test/OpenMP/parallel_masked_ast_print.cpp
===
--- clang/test/OpenMP/parallel_masked_ast_print.cpp
+++ clang/test/OpenMP/parallel_masked_ast_print.cpp
@@ -198,6 +198,7 @@
 int main (int argc, char **argv) {
   long x;
   int b = argc, c, d, e, f, g;
+  int tid = 0;
   static int a;
   #pragma omp threadprivate(a)
   int arr[10][argc], arr1[2];
@@ -207,8 +208,8 @@
 // CHECK-NEXT: #pragma omp parallel masked
   a=2;
 // CHECK-NEXT: a = 2;
-#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) 
if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| 
: c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e)
-// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) 
firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) 
proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) 
allocate(e)
+#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) 
if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| 
: c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e) filter(tid)
+// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) 
firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) 
proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) 
allocate(e) filter(tid)
   foo();
 // CHECK-NEXT: foo();
 // CHECK-NEXT: #pragma omp parallel masked allocate(e) if(b) num_threads(c) 
proc_bind(close) reduction(^: e,f) reduction(&&: g,arr[0:argc][:10])
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6502,6 +6502,7 @@
   case OMPC_uses_allocators:
   case OMPC_affinity:
   case OMPC_bind:
+  case OMPC_filter:
 continue;
   case OMPC_allocator:
   case OMPC_flush:


Index: clang/test/OpenMP/parallel_masked_ast_print.cpp
===
--- clang/test/OpenMP/parallel_masked_ast_print.cpp
+++ clang/test/OpenMP/parallel_masked_ast_print.cpp
@@ -198,6 +198,7 @@
 int main (int argc, char **argv) {
   long x;
   int b = argc, c, d, e, f, g;
+  int tid = 0;
   static int a;
   #pragma omp threadprivate(a)
   int arr[10][argc], arr1[2];
@@ -207,8 +208,8 @@
 // CHECK-NEXT: #pragma omp parallel masked
   a=2;
 // CHECK-NEXT: a = 2;
-#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| : c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e)
-// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) allocate(e)
+#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| : c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e) filter(tid)
+// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) allocate(e) filter(tid)
   foo();
 // CHECK-NEXT: foo();
 // CHECK-NEXT: #pragma omp parallel masked allocate(e) if(b) num_threads(c) proc_bind(close) reduction(^: e,f) reduction(&&: g,arr[0:argc][:10])
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6502,6 +6502,7 @@
   case OMPC_uses_allocators:
   case OMPC_affinity:
   case OMPC_bind:
+  case OMPC_filter:
 continue;
   case OMPC_allocator:
   case OMPC_flush:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a35141d - [OpenMP] Add handling cases when filter(tid) appears with default(none)

[Mike Rice via cfe-commits]

Author: Fazlay Rabbi
Date: 2022-06-22T17:45:43-07:00
New Revision: a35141d395019c837d16419c6ef57662a6efefc5

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

LOG: [OpenMP] Add handling cases when filter(tid) appears with default(none)

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/parallel_masked_ast_print.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 84848c7be858d..e65f5a236a1ff 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -6502,6 +6502,7 @@ StmtResult Sema::ActOnOpenMPExecutableDirective(
   case OMPC_uses_allocators:
   case OMPC_affinity:
   case OMPC_bind:
+  case OMPC_filter:
 continue;
   case OMPC_allocator:
   case OMPC_flush:

diff  --git a/clang/test/OpenMP/parallel_masked_ast_print.cpp 
b/clang/test/OpenMP/parallel_masked_ast_print.cpp
index 3532cf61ec25f..ce1552ff5bcf1 100644
--- a/clang/test/OpenMP/parallel_masked_ast_print.cpp
+++ b/clang/test/OpenMP/parallel_masked_ast_print.cpp
@@ -198,6 +198,7 @@ enum Enum { };
 int main (int argc, char **argv) {
   long x;
   int b = argc, c, d, e, f, g;
+  int tid = 0;
   static int a;
   #pragma omp threadprivate(a)
   int arr[10][argc], arr1[2];
@@ -207,8 +208,8 @@ int main (int argc, char **argv) {
 // CHECK-NEXT: #pragma omp parallel masked
   a=2;
 // CHECK-NEXT: a = 2;
-#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) 
if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| 
: c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e)
-// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) 
firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) 
proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) 
allocate(e)
+#pragma omp parallel masked default(none), private(argc,b) firstprivate(argv) 
if (parallel: argc > 0) num_threads(ee) copyin(a) proc_bind(spread) reduction(| 
: c, d, arr1[argc]) reduction(* : e, arr[:10][0:argc]) allocate(e) filter(tid)
+// CHECK-NEXT: #pragma omp parallel masked default(none) private(argc,b) 
firstprivate(argv) if(parallel: argc > 0) num_threads(ee) copyin(a) 
proc_bind(spread) reduction(|: c,d,arr1[argc]) reduction(*: e,arr[:10][0:argc]) 
allocate(e) filter(tid)
   foo();
 // CHECK-NEXT: foo();
 // CHECK-NEXT: #pragma omp parallel masked allocate(e) if(b) num_threads(c) 
proc_bind(close) reduction(^: e,f) reduction(&&: g,arr[0:argc][:10])



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

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439221.
ishaangandhi added a comment.

Rebased


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

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -191,6 +191,16 @@
 return a + b * c;
 }
 
+// Unknown expressions are ignored
+int test_basic18() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
 //===//
 
 #define PASTE_CODE(x) x
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -191,6 +191,16 @@
 return a + b * c;
 }
 
+// Unknown expressions are ignored
+int test_basic18() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
 //===//
 
 #define PASTE_CODE(x) x
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
ishaangandhi requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The clang-tidy check `bugprone-branch-clone` has a false positive if some 
symbols are undefined. This patch silences the warning when the two sides of a 
branch are invalid.

See //github.com/llvm/llvm-project/issues/56057 for the original issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
@@ -191,6 +191,16 @@
 return a + b * c;
 }
 
+// Unknown expressions are ignored
+int test_basic18() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
 //===//
 
 #define PASTE_CODE(x) x
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
@@ -191,6 +191,16 @@
 return a + b * c;
 }
 
+// Unknown expressions are ignored
+int test_basic18() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
 //===//
 
 #define PASTE_CODE(x) x
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
const ASTContext ) {
+  if (isa(LHS) && isa(RHS)) {
+// If we have errors in expressions, we will be unable
+// to accurately profile and compute hashes for each
+// of the left and right statements.
+const Expr *LHSExpr = llvm::dyn_cast(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:546
 
+void diagnoseUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
+DiagnoseState> ) {

I think this should be moved closer to its use.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:692-693
+  .CaseOf(
+  expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
+ isOptionalOperatorCallWithName("->", IgnorableOptional))),
+  [](const CallExpr *E, const MatchFinder::MatchResult &,

This logic is duplicated between the check and the diagnosis. I think it would 
be nice to factor this out to avoid these two parts getting out of sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127898

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

[PATCH] D127901: [LinkerWrapper] Add PTX output to CUDA fatbinary in LTO-mode

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D127901#3603467 , @tra wrote:

> I'm not sure I follow. WDYM by "go inside the binary itself" ? I assume you 
> mean the per-GPU offload binaries inside per-TU .o. so that it could be used 
> when that GPU object gets linked into GPU executable?
>
> What if different TUs that we're linking were compiled using 
> different/contradictory options?
>
> The problem is that conceptually "--cuda-include-ptx" option ultimately 
> affects the final GPU executable. If we're in RDC mode, then PTX is probably 
> useless for JITT-ing purposes, as you can't link PTX and create the final 
> executable. Well, I guess it might sort of be possible by concatenating the 
> .s files and adding bunch of forward declarations for the functions, and 
> merging debug info, and removing duplicate weak functions,,... Well, 
> basically by writing a linker for a new "PTX" architecture. Doable, but so 
> not worth it, IMO.
>
> TUs are compiled to IR, then PTX generation shifts to the final link phase. I 
> think we may need to rely on the user to supply PTX controls there 
> explicitly. Or, at the very least, check that `cuda-include-ptx` propagated 
> from TUs is used consistently in all TUs.

I just mean that right now the `--[no-]cuda-include-ptx` is done at the 
compilation phase, whereas this in LTO so we'd need to make sure we have those 
arguments. It's true that we could just require the user to pass it to the 
linker instead, but conceptually PTX generation happens in the "compiler" and 
not the linker.

>> We'll probably just use the same default as that flag (which is on I think).
>>
>>> This brings another question -- which GPU variant will we generate PTX for? 
>>> One? All (if more than one is specified)? The ones specified by 
>>> `--[no-]cuda-include-ptx=` ?
>>
>> Right now, it'll be the one that's attached to the LTO job. So if the user 
>> specified `sm_70` they'll get PTX for `sm_70`.
>
> I mean, when the user specifies more than one GPU variant to target. 
> E.g. both `sm_70` and `sm_50`. 
> PTX for the former would probably provide better performance if we run on a 
> newer GPU (e.g. sm_80). 
> On the other hand, it will likely fail if we were to attempt running from PTX 
> on sm_60. 
> Both would probably fail if we were to run on sm_35. Including all PTX 
> variants is wasteful (Tensorflow-using applications are already pushing the 
> limits on small memory model and sometimes fail to link due to the executable 
> being too large).
>
> The point is that there's no "one true choice" for the PTX architecture (as 
> there's no safe/sensible choice for the offload target). Only the end user 
> would know their intent. We do need explicit controls and a documented policy 
> on what we produce by default.

This is a good point I haven't thought of. This right now is basically just a 
by-product of the LTO pass. We run LTO for the target and since we got a PTX 
output we might as well include it. This may be what we do in Clang as well, I 
think we just include the PTX output in with the Cubin for each offload job. 
Even if we went to LLVM-IR we'd still be restricted by some features I think. 
As it stands, this patch just makes `clang++ cuda.cu --offload-new-driver 
-fgpu-rdc --offload-arch=sm_60 -foffload-lto` give a fatbinary with sm_60 PTX / 
Cubins. I think that is controlled by the user as it's only going to generate 
PTX for the architecture they specified via --offload-arch (or default).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127901

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

[PATCH] D128363: [clang][dataflow] Implement functionality for flow condition variable substitution.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:197
+  /// `Substitutions`, it will be substituted with the value it maps to.
+  BoolValue (
+  AtomicBoolValue ,

Could you elaborate on why do we need this? Why do we care about how flow 
condition constraints are represented? We should use the solver to ask 
questions and as far as I understand the solver should work with both 
representations. Also this function feels like doing two things, building the 
flow condition from the tokens and doing substitutions. Any reason why those 
two are not separate functions? Is this for performance reasons?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128363

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

[PATCH] D128401: [Clang-tidy] Fixing a bug raising false alarms on static local variables in the Infinite Loop Checker

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, t-rasmud, usama54321, rsundahl, yln, 
kubamracek, krispy1994, jkorous, delcypher, chrisdangelo, thetruestblue, 
aaron.ballman, alexfh, gribozavr, njames93, LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, kristof.beyls, xazax.hun.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The Infinite Loop Checker could report a false positive on the example below:

  void f() {
static int i = 0;
i++;
while (i < 10)
   f();
  }

This patch adds checking for recursive calls in the Infinite Loop Checker when 
loop condition involves static local variables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128401

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
@@ -44,6 +44,27 @@
 j++;
   }
 }
+
++ (void)recursiveMethod {
+  static int i = 0;
+
+  i++;
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+[I classMethod];
+  }
+  
+  id x = [[I alloc] init];
+
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+[x instanceMethod];
+  }
+  while (i < 10) {
+// no warning, there is a recursive call that can mutate the static local variable
+[I recursiveMethod];
+  }
+}
 @end
 
 void testArrayCount() {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -685,3 +685,29 @@
 0;
   }) x;
 }
+
+void test_local_static_recursion() {
+  static int i = 10;
+  int j = 0;
+
+  i--;
+  while (i >= 0)
+test_local_static_recursion(); // no warning, recursively decrement i
+  for (; i >= 0;)
+test_local_static_recursion(); // no warning, recursively decrement i
+  for (; i + j >= 0;)
+test_local_static_recursion(); // no warning, recursively decrement i
+  for (; i >= 0; i--)
+; // no warning, i decrements
+  while (j >= 0)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (j) are updated in the loop body [bugprone-infinite-loop]
+test_local_static_recursion();
+
+  int (*p)(int) = 0;
+
+  while (i >= 0)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+p = 0;
+  while (i >= 0)
+p(0); // we don't know what p points to so no warning
+}
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -11,6 +11,9 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Analysis/CallGraph.h"
+#include "llvm/ADT/SCCIterator.h"
+#include "llvm/ADT/SmallVector.h"
 
 using namespace clang::ast_matchers;
 using clang::tidy::utils::hasPtrOrReferenceInFunc;
@@ -146,6 +149,111 @@
   return false;
 }
 
+/// populates the set `Callees` with all function (and objc method) declarations
+/// called in `StmtNode` if all visited call sites have resolved call targets.
+///
+/// \return true iff all `CallExprs` visited have callees; false otherwise
+/// indicating there is an unresolved indirect call.
+static bool populateCallees(const Stmt *StmtNode,
+llvm::SmallSet ) {
+if (const CallExpr *Call = dyn_cast(StmtNode)) {
+const Decl *Callee = Call->getDirectCallee();
+
+if (!Callee)
+return false; // unresolved call
+Callees.insert(Callee->getCanonicalDecl());
+}
+if (const ObjCMessageExpr *Call = dyn_cast(StmtNode)) {
+const ObjCMethodDecl *Callee = Call->getMethodDecl();
+
+if (!Callee)
+return false;// unresolved call
+Callees.insert(Callee->getCanonicalDecl());
+}
+  for (const Stmt *Child : StmtNode->children())
+if (Child 

[PATCH] D127901: [LinkerWrapper] Add PTX output to CUDA fatbinary in LTO-mode

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D127901#3603118 , @jhuber6 wrote:

> In D127901#3603006 , @tra wrote:
>
>> Then we do need a knob controlling whether we do want to embed PTX or not. 
>> The default should be "off" IMO.
>> We currently have `--[no-]cuda-include-ptx=` we may reuse for that purpose.
>
> We could definitely re-use that. It's another option that probably need to go 
> inside the binary itself since normally those options aren't passed to the 
> linker.

I'm not sure I follow. WDYM by "go inside the binary itself" ? I assume you 
mean the per-GPU offload binaries inside per-TU .o. so that it could be used 
when that GPU object gets linked into GPU executable?

What if different TUs that we're linking were compiled using 
different/contradictory options?

The problem is that conceptually "--cuda-include-ptx" option ultimately affects 
the final GPU executable. If we're in RDC mode, then PTX is probably useless 
for JITT-ing purposes, as you can't link PTX and create the final executable. 
Well, I guess it might sort of be possible by concatenating the .s files and 
adding bunch of forward declarations for the functions, and merging debug info, 
and removing duplicate weak functions,,... Well, basically by writing a linker 
for a new "PTX" architecture. Doable, but so not worth it, IMO.

TUs are compiled to IR, then PTX generation shifts to the final link phase. I 
think we may need to rely on the user to supply PTX controls there explicitly. 
Or, at the very least, check that `cuda-include-ptx` propagated from TUs is 
used consistently in all TUs.

> We'll probably just use the same default as that flag (which is on I think).
>
>> This brings another question -- which GPU variant will we generate PTX for? 
>> One? All (if more than one is specified)? The ones specified by 
>> `--[no-]cuda-include-ptx=` ?
>
> Right now, it'll be the one that's attached to the LTO job. So if the user 
> specified `sm_70` they'll get PTX for `sm_70`.

I mean, when the user specifies more than one GPU variant to target. 
E.g. both `sm_70` and `sm_50`. 
PTX for the former would probably provide better performance if we run on a 
newer GPU (e.g. sm_80). 
On the other hand, it will likely fail if we were to attempt running from PTX 
on sm_60. 
Both would probably fail if we were to run on sm_35. Including all PTX variants 
is wasteful (Tensorflow-using applications are already pushing the limits on 
small memory model and sometimes fail to link due to the executable being too 
large).

The point is that there's no "one true choice" for the PTX architecture (as 
there's no safe/sensible choice for the offload target). Only the end user 
would know their intent. We do need explicit controls and a documented policy 
on what we produce by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127901

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

[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.

[Gábor Horváth via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:69
+Res.first->second =
+(std::make_unique(PointeeLoc));
+  }

Should we have a `PointerValue` without `PointeeLoc` to represent null pointers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128056

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:5863
   // FIXME: derive from "Target" ?
-  return WCharTy;
+  return IntTy;
 }

rsmith wrote:
> This change seems surprising. Can you explain what's going on here?
Motivated by `__make_signed(wchar_t)` previously returning `wchar_t`, and that 
it was seemingly inconsistent with `getUnsignedWCharType` below.



Comment at: clang/lib/AST/ASTContext.cpp:10712-10713
 QualType ASTContext::getCorrespondingUnsignedType(QualType T) const {
-  assert((T->hasSignedIntegerRepresentation() || T->isSignedFixedPointType()) 
&&
+  assert((T->hasSignedIntegerRepresentation() || T->isIntegerType() ||
+  T->isEnumeralType() || T->isSignedFixedPointType()) &&
  "Unexpected type");

rsmith wrote:
> If we now allow unsigned types to be passed in here, can we do so 
> consistently?
> 
> This seems to do the wrong thing for a vector of scoped enumeration type. Is 
> that a problem for the builtins?
I think not, given we're not worrying about vector types right now?



Comment at: clang/lib/AST/TypePrinter.cpp:1158
raw_ostream ) {
   IncludeStrongLifetimeRAII Strong(Policy);
 }

rsmith wrote:
> Remove this line too. No point building an RAII scope with nothing in it.
Can we get rid of this function entirely in that case?



Comment at: clang/lib/Parse/ParseExpr.cpp:1750
+#undef TRANSFORM_TYPE_TRAIT_DEF
+if (NextToken().is(tok::less)) {
+  Tok.setKind(tok::identifier);

rsmith wrote:
> cjdb wrote:
> > rsmith wrote:
> > > Here you're checking for `trait<` and treating it as an identifier; 
> > > elsewhere you're checking for `trait(` and treating it as a builtin. Is 
> > > there a reason to treat `trait` followed by a token other than `(` or `<` 
> > > inconsistently?
> > The parser needs to treat `trait<` as an identifier to accommodate 
> > libstdc++'s usage of a few of these as alias templates. I've added a 
> > comment to explain why in the code.
> I agree we need to treat `trait<` as an identifier and `trait(` as the 
> builtin. My question is, why do we have inconsistent treatment of the 
> remaining cases (`trait` followed by any other token)? For example, 
> `MaybeParseTypeTransformTypeSpecifier` treats `trait + 1` as an identifier. 
> But this code treats it as the builtin name.
> 
> I think there are two choices that make the most sense, if they work:
> 
> 1) `trait(` is the builtin and any other utterance is an identifier, or
> 2) `trait<` is an identifier, `using trait =` is an identifier, and any other 
> utterance is the builtin.
> 
> I think I prefer option (2), given that it's defining the narrower special 
> case. But all I'm really looking for here is a consistent story one way or 
> the other, if it's feasible to have one.
I'd like for it to be (2) as well, but based on how we do alias-declarations, 
I'm concerned that will have performance implications for a rare occurrence.



Comment at: clang/lib/Sema/SemaType.cpp:9178
+  // in the same group of qualifiers as 'const' and 'volatile', we're extending
+  // '__decay(T)' so that it also removes '__restrict' in C++.
+  Quals.removeCVRQualifiers();

rsmith wrote:
> Why "in C++"? It looks like it does this in C too, which is probably 
> appropriate. However, this is a divergence from what `std::decay_t` does in 
> libc++ and libstdc++, where it does not remove `__restrict`.
That sentence is specifically calling out that `__restrict` is being removed in 
C++ mode too. Tightened up the wording. I'm partial to `__decay` removing 
`__restrict` because it would be what language decay does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaExprMember.cpp:1873-1877
+if (auto *PrivateCopy =
+isOpenMPFDCaptureDecl(Field, Base.get(), IsArrow, OpLoc, ,
+  /*TemplateKWLoc=*/SourceLocation(), Field,
+  FoundDecl, /*HadMultipleCandidates=*/false,
+  MemberNameInfo, MemberType, VK, OK))

ABataev wrote:
> Why do we need this function? Implicit private rule should apply (if should) 
> only to this-Юашдув kind of expressions, if something like parent.field 
> expression is used, parent should be private, not private.field. Or I'm 
> missing something?
You are right for "parent.a" only privatize "parent".

But if 'a' is a field access in a member function then 'a' is privatized, not 
"this".  It is same with firstprivate(a).  But for the explicit 
firstprivate(a), the capture is build in ActOnOpenMPFirstprivateClause, so it 
can be mapped to reference in the omp region.  For Implicit, I need to build 
capture for the first reference in the omp region with defalut(first|private) 
is used.  And used that to generate firstprivate clause at end when the call to 
ActOnOpenMPFirstprivateClause when generating implicit clause.

Is this reasonable?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:201
+using ImplicitFDCapTy = std::pair;
+// List of captuer fields
+llvm::SmallVector ImplicitDefaultFirstprivateFDs;

ABataev wrote:
> ///
Thanks Changed



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2410
 D,
-[](OpenMPClauseKind C, bool AppliedToPointee) {
+[](OpenMPClauseKind C, bool AppliedToPointee, ...) {
   return isOpenMPPrivate(C) && !AppliedToPointee;

ABataev wrote:
> Better to add an actual param type here but without Param name
Changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

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

[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 439195.
jyu2 marked 7 inline comments as done.
jyu2 retitled this revision from "Generate the capture for field when the field 
is used with implicit default." to "Generate the capture for field when the 
field is used in openmp region with implicit default in the member function.".
jyu2 edited the summary of this revision.
jyu2 added a comment.
Herald added subscribers: guansong, yaxunl.

Thanks Alexey for the review.  This is address Alexey's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/OpenMP/default_firstprivate_ast_print.cpp
  clang/test/OpenMP/default_private_ast_print.cpp

Index: clang/test/OpenMP/default_private_ast_print.cpp
===
--- clang/test/OpenMP/default_private_ast_print.cpp
+++ clang/test/OpenMP/default_private_ast_print.cpp
@@ -96,4 +96,57 @@
   // DUMP-NEXT:  -DeclRefExpr {{.*}} 'a'
   // DUMP-NEXT:  -DeclRefExpr {{.*}} 'yy'
 }
+
+void zoo(int);
+struct A {
+  int z;
+  int f;
+  A();
+  ~A();
+  void foo() {
+#pragma omp parallel private(z) default(private)
+{
+  z++;
+  f++;
+  zoo(z + f);
+  f++;
+}
+  }
+  // PRINT:#pragma omp parallel private(this->z) default(private)
+  // DUMP: -OMPParallelDirective
+  // DUMP-NEXT:  -OMPPrivateClause
+  // DUMP-NEXT:-DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT:  -OMPDefaultClause
+  // DUMP-NEXT:  -OMPPrivateClause
+  // DUMP-NEXT:-DeclRefExpr {{.*}} 'f'
+  // DUMP: -CXXThisExpr {{.*}} 'A *' implicit this
+  void bar() {
+#pragma omp parallel private(z) default(private)
+{
+#pragma omp parallel private(z) default(private)
+  {
+z++;
+f++;
+zoo(z + f);
+f++;
+  }
+}
+  }
+  // PRINT:#pragma omp parallel private(this->z) default(private)
+  // PRINT:  #pragma omp parallel private(this->z) default(private)
+  // DUMP: -OMPParallelDirective
+  // DUMP-NEXT:  -OMPPrivateClause
+  // DUMP-NEXT:-DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT:  -OMPDefaultClause
+  // DUMP:   -OMPParallelDirective
+  // DUMP-NEXT:-OMPPrivateClause
+  // DUMP-NEXT:   -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT:-OMPDefaultClause
+  // DUMP-NEXT:-OMPPrivateClause {{.*}} 
+  // DUMP-NEXT:   -DeclRefExpr {{.*}} 'f'
+  // DUMP: -CXXThisExpr
+  // DUMP: -MemberExpr
+  // DUMP-NEXT:   -CXXThisExpr
+  // DUMP: -CXXThisExpr
+};
 #endif // HEADER
Index: clang/test/OpenMP/default_firstprivate_ast_print.cpp
===
--- clang/test/OpenMP/default_firstprivate_ast_print.cpp
+++ clang/test/OpenMP/default_firstprivate_ast_print.cpp
@@ -45,7 +45,8 @@
 // PRINT-NEXT:  this->targetDev++;
 // CHECK-NEXT: }
 // DUMP: -OMPParallelDirective
-// DUMP->NEXT: -OMPDefaultClause
+// DUMP-NEXT: -OMPDefaultClause
+// DUMP-NOT:   -OMPFirstprivateClause
   }
   // PRINT: template<> void apply<32U>()
   // PRINT: #pragma omp parallel default(firstprivate)
@@ -54,6 +55,8 @@
   // CHECK-NEXT: }
   // DUMP: -OMPParallelDirective
   // DUMP-NEXT: -OMPDefaultClause
+  // DUMP-NEXT:  -OMPFirstprivateClause
+  // DUMP-NEXT:-DeclRefExpr {{.*}} 'targetDev'
 };
 
 void use_template() {
@@ -99,4 +102,60 @@
   // DUMP-NEXT: -DeclRefExpr {{.*}} 'yy'
   // DUMP-NEXT: -DeclRefExpr {{.*}} 'y'
 }
+void zoo(int);
+struct A {
+  int z;
+  int f;
+  A();
+  ~A();
+  void foo() {
+#pragma omp parallel firstprivate(z) default(firstprivate)
+{
+  z++;
+  f++;
+  zoo(z + f);
+  f++;
+}
+  }
+  // PRINT:  #pragma omp parallel firstprivate(this->z) default(firstprivate)
+  // DUMP:   -OMPParallelDirective
+  // DUMP-NEXT: -OMPFirstprivateClause
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT: -OMPDefaultClause
+  // DUMP-NEXT: -OMPFirstprivateClause {{.*}} 
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'f'
+  // DUMP:  -CXXThisExpr {{.*}} 'A *' implicit this
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'f'
+  void bar() {
+#pragma omp parallel firstprivate(z) default(firstprivate)
+{
+#pragma omp parallel private(z) default(firstprivate)
+  {
+z++;
+f++;
+zoo(z + f);
+f++;
+  }
+}
+  }
+  // PRINT:  #pragma omp parallel firstprivate(this->z) default(firstprivate)
+  // PRINT:#pragma omp parallel private(this->z) default(firstprivate)
+  // DUMP: -OMPParallelDirective
+  // DUMP-NEXT: -OMPFirstprivateClause
+  // DUMP-NEXT:  -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT:  -OMPDefaultClause
+  // DUMP:-OMPParallelDirective
+  // DUMP-NEXT:-OMPPrivateClaus
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 439177.
cor3ntin added a comment.

- Add a comment referencing the (to be) C++ wording and unicode discussion on 
replacemet characters
- Do not try to skip utf8 checks when the warning is not enabled as checking 
whether the warning is enabled is more expensive
- Fix a bug causing vectorization to not be aligned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/test/Lexer/comment-invalid-utf8.c
  llvm/include/llvm/Support/ConvertUTF.h
  llvm/lib/Support/ConvertUTF.cpp

Index: llvm/lib/Support/ConvertUTF.cpp
===
--- llvm/lib/Support/ConvertUTF.cpp
+++ llvm/lib/Support/ConvertUTF.cpp
@@ -417,6 +417,16 @@
 return isLegalUTF8(source, length);
 }
 
+/*
+ * Exported function to return the size of the first utf-8 code unit sequence,
+ * Or 0 if the sequence is not valid;
+ */
+unsigned getUTF8SequenceSize(const UTF8 *source, const UTF8 *sourceEnd) {
+  int length = trailingBytesForUTF8[*source] + 1;
+  return (length > sourceEnd - source && isLegalUTF8(source, length)) ? length
+  : 0;
+}
+
 /* - */
 
 static unsigned
Index: llvm/include/llvm/Support/ConvertUTF.h
===
--- llvm/include/llvm/Support/ConvertUTF.h
+++ llvm/include/llvm/Support/ConvertUTF.h
@@ -181,6 +181,8 @@
 
 Boolean isLegalUTF8String(const UTF8 **source, const UTF8 *sourceEnd);
 
+unsigned getUTF8SequenceSize(const UTF8 *source, const UTF8 *sourceEnd);
+
 unsigned getNumBytesForUTF8(UTF8 firstByte);
 
 /*/
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -2391,13 +2391,37 @@
   //
   // This loop terminates with CurPtr pointing at the newline (or end of buffer)
   // character that ends the line comment.
+
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   char C;
   while (true) {
 C = *CurPtr;
 // Skip over characters in the fast loop.
-while (C != 0 &&// Potentially EOF.
-   C != '\n' && C != '\r')  // Newline or DOS-style newline.
+while (isASCII(C) && C != 0 &&   // Potentially EOF.
+   C != '\n' && C != '\r') { // Newline or DOS-style newline.
   C = *++CurPtr;
+  UnicodeDecodingAlreadyDiagnosed = false;
+}
+
+if (!isASCII(C)) {
+  unsigned Length = llvm::getUTF8SequenceSize(
+  (const llvm::UTF8 *)CurPtr, (const llvm::UTF8 *)BufferEnd);
+  if (Length == 0) {
+if (!UnicodeDecodingAlreadyDiagnosed && !isLexingRawMode())
+  Diag(CurPtr, diag::warn_invalid_utf8_in_comment);
+UnicodeDecodingAlreadyDiagnosed = true;
+++CurPtr;
+  } else {
+UnicodeDecodingAlreadyDiagnosed = false;
+CurPtr += Length;
+  }
+  continue;
+}
 
 const char *NextLine = CurPtr;
 if (C != 0) {
@@ -2664,6 +2688,12 @@
   if (C == '/')
 C = *CurPtr++;
 
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   while (true) {
 // Skip over all non-interesting characters until we find end of buffer or a
 // (probably ending) '/' character.
@@ -2672,14 +2702,26 @@
 // doesn't check for '\0'.
 !(PP && PP->getCodeCompletionFileLoc() == FileLoc)) {
   // While not aligned to a 16-byte boundary.
-  while (C != '/' && ((intptr_t)CurPtr & 0x0F) != 0)
+  while (C != '/' && (intptr_t)CurPtr % 16 != 0) {
+if (!isASCII(C)) {
+  CurPtr--;
+  goto MultiByteUTF8;
+}
 C = *CurPtr++;
-
+  }
   if (C == '/') goto FoundSlash;
 
 #ifdef __SSE2__
   __m128i Slashes = _mm_set1_epi8('/');
-  while (CurPtr+16 <= BufferEnd) {
+  while (CurPtr + 16 < BufferEnd) {
+__m128i Bytes =
+_mm_load_si128(reinterpret_cast(CurPtr));
+int Mask = _mm_movemask_epi8(Bytes);
+if (LLVM_UNLIKELY(Mask != 0)) {
+  CurPtr += llvm::countTrailingZeros(Mask);
+  goto MultiByteUTF8;
+}
+// look for slashes
 int 

[PATCH] D117593: [clang-tidy] Change google-explicit-constructor to ignore conversions and operators marked `explicit(false)`

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.
Herald added a project: All.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117593

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

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118743

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

[PATCH] D127901: [LinkerWrapper] Add PTX output to CUDA fatbinary in LTO-mode

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D127901#3603006 , @tra wrote:

> Then we do need a knob controlling whether we do want to embed PTX or not. 
> The default should be "off" IMO.
> We currently have `--[no-]cuda-include-ptx=` we may reuse for that purpose.

We could definitely re-use that. It's another option that probably need to go 
inside the binary itself since normally those options aren't passed to the 
linker. We'll probably just use the same default as that flag (which is on I 
think).

> This brings another question -- which GPU variant will we generate PTX for? 
> One? All (if more than one is specified)? The ones specified by 
> `--[no-]cuda-include-ptx=` ?

Right now, it'll be the one that's attached to the LTO job. So if the user 
specified `sm_70` they'll get PTX for `sm_70`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127901

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

[PATCH] D128356: [clang][dataflow] Use NoopLattice in optional model

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
Herald added a subscriber: rnkovacs.

Should this PR delete `SourceLocationsLattice`? Are there other users left or 
do we anticipate new users in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128356

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

[PATCH] D128357: [clang][dataflow] Store flow condition constraints in a single `FlowConditionConstraints` map.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

I found a small nit inline, otherwise looks good.




Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:76
+  if (!Res.second) {
+FlowConditionConstraints[] =
+(*Res.first->second, Constraint);

Do you actually need a second lookup? Couldn't you use `Res.first` to update 
the corresponding value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128357

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

[PATCH] D127686: [Offloading] Embed the target features in the OffloadBinary

[Artem Belevich via Phabricator via cfe-commits]

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

LGTM with a nit.




Comment at: clang/test/Driver/openmp-offload-gpu-new.c:117
+
+// CHECK-LTO-FEATURES: clang-offload-packager{{.*}}feature={{.*}}ptx

tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > This should probably be a bit more specific/verbose. E.g. I'd want to 
> > > make sure that `feature=` is part of the `--image` argument and that ptx 
> > > belongs to it and is not part of some other argument (or even a file name 
> > > extension).
> > Sure, I was just hesitant to make it super specific since the specific 
> > feature will change depending on the CUDA installation (or lack thereof).
> I'm all for concise tests as long as they:
> 
> - express what you want to verify in a way that the reader would be able to 
> understand w/o having to look at the source code. 
> - are reasonably robust and do not produce false positives. `.*` wildcards 
> make it very easy to match things unintentionally.  Their use should be 
> carefully restricted. 
> 
> I wish FileCheck would have some sort of nested match check. E.g.
> 
> ```
> ; CHECK : [[CANDIDATE:--option.*?\s+]]
> ; CHECK:  [[SUBMATCH1: match something within [[CANDIDATE
> ; CHECK:  [[SUBMATCH2: match something within [[SUBMATCH1
> ```
Nit: ^^^ looks like this one slipped through the cracks and wasn't addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127686

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

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish added a comment.

I'm still going through PGOInstrumentation.cpp ...




Comment at: clang/test/CodeGen/memprof.cpp:15
+// # Collect memory profile:
+// $ clang++ -fuse-ld=lld -Wl,-no-pie -Wl,--no-rosegment -gmlt \
+//  -fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \

Just `-no-pie` is better (details: https://reviews.llvm.org/rG103b28902fd6)



Comment at: llvm/include/llvm/Analysis/MemoryProfileInfo.h:1
+//===- llvm/Analysis/MemoryProfileInfo.h - memory profile info ---*- C++ 
-*-==//
+//

Can you split out the memory profile info parts into a separate patch? Also I 
think the interface is simple enough to be able to unit test. Something set up 
like [1] would be great. Then we can call addCallstack with different 
annotations followed by buildAndAttachMIBMetaData followed by checking the 
metadata annotated on the call inst(s). What do you think?

[1] 
https://github.com/llvm/llvm-project/blob/3f8e4169c1c390fd086658c1e51983ee61bff9bc/llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp#L71



Comment at: llvm/include/llvm/Analysis/MemoryProfileInfo.h:40
+
+// Class to build a trie of call stack contexts for a particular profiled
+// allocation call, along with their associated allocation types.

Should this be a doxygen comment block with `///`?



Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:333
 
+  bool hasMemoryProfile() const override {
+return (Version & VARIANT_MASK_MEMPROF) != 0;

The RawInstrProfReader shouldn't have the memprof mask set. We have a separate  
raw binary format which is independent. So this should always return false. 
Also maybe add a comment to document the fact?



Comment at: llvm/lib/Analysis/MemoryProfileInfo.cpp:19
+
+#define DEBUG_TYPE "memory-profile-info"
+

We use MemoryProfile and MemProf interchangeably. Does it make sense to pick 
one and make it consistent throughout? Here for eg. the flags begin with 
"memprof-*" but the debug type is "memory-profile-*".



Comment at: llvm/lib/Analysis/MemoryProfileInfo.cpp:102
+assert(AllocStackId == StackId);
+Alloc->AllocTypes |= (uint8_t)AllocType;
+  } else {

nit: prefer static_cast here and elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128142

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

[PATCH] D127901: [LinkerWrapper] Add PTX output to CUDA fatbinary in LTO-mode

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D127901#3602771 , @jdoerfert wrote:

> Do we want/need PTX, I do not, but I don't mind having it. Someone will ask 
> for it eventually.

Fair enough.

> However, if we embed bitcode via LTO we can use the
> single-linked PTX image for the whole module and include it in the
> fatbinary. This allows us to do the following and have it execute even
> without the correct architecture specified.
> `clang foo.cu -foffload-lto -fgpu-rdc --offload-new-driver -lcudart`

Then we do need a knob controlling whether we do want to embed PTX or not. The 
default should be "off" IMO.
We currently have `--[no-]cuda-include-ptx=` we may reuse for that purpose.

This brings another question -- which GPU variant will we generate PTX for? 
One? All (if more than one is specified)? The ones specified by 
`--[no-]cuda-include-ptx=` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127901

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

[PATCH] D128022: [HIP] add -fhip-kernel-arg-name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+  }
+  if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+  getCodeGenOpts().HIPSaveKernelArgName)
 Fn->setMetadata("kernel_arg_name",

yaxunl wrote:
> yaxunl wrote:
> > tra wrote:
> > > tra wrote:
> > > > Should we consolidate both options into `-fkernel-arg-info` and make 
> > > > `-cl-kernel-arg-info` an alias to it?
> > > Also, this check is odd. For some reason only *arg name*  metadata is set 
> > > conditionally, but for whatever reason OpenCL sets other arg metadata 
> > > unconditionally.
> > > 
> > > Now I'm really curious what's so special about "kernel_arg_name" vs the 
> > > other arg metadata.
> > > Should we consolidate both options into `-fkernel-arg-info` and make 
> > > `-cl-kernel-arg-info` an alias to it?
> > 
> > -cl-kernel-arg-info is an OpenCL option defined in OpenCL spec, therefore 
> > is made OpenCL only option. It would be confusing to allow it with other 
> > languages.
> > Also, this check is odd. For some reason only *arg name*  metadata is set 
> > conditionally, but for whatever reason OpenCL sets other arg metadata 
> > unconditionally.
> > 
> > Now I'm really curious what's so special about "kernel_arg_name" vs the 
> > other arg metadata.
> 
> The other metadata are mandatory because they are necessary for OpenCL 
> runtime to set kernel argument.
> 
> The kernel argument name is emitted only with -cl-kernel-arg-info since it is 
> only used to support clGetKernelArgInfo which requires -cl-kernel-arg-info to 
> work.
>  It would be confusing to allow it with other languages.

On the other hand having two options that control exactly the same 
functionality also looks odd to me.

The way I see it is that an entity may have more than one name. OpenCL standard 
requires that that particular functionality must be enabled via 
`-cl-kernel-arg-info` and I'm not proposing to change that. The OpenCL standard 
has no say whether that flag may have a different name in addition to the 
standard-required one.

This is similar to how over time we've been transitioning what used to be 
CUDA-only options into their generic `-gpu` and `-offload` variants, only in 
this case OpenCL functionality becomes useful outside of OpenCL.



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

https://reviews.llvm.org/D128022

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

[PATCH] D126097: [clang-tidy] Adds the NSDateFormatter checker to clang-tidy

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126097

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

[PATCH] D126735: [clang-tidy] Silence modernize-redundant-void-arg in the case of vexing parses

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126735

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

[PATCH] D119165: [clang-tidy] Add processing lambda captures at bugprone-use-after-move check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119165

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D127036: [clang-tidy] Warn about arrays in `bugprone-undefined-memory-manipulation`

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


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

https://reviews.llvm.org/D127036

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

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


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

https://reviews.llvm.org/D127293

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

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto main:HEAD and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y updated this revision to Diff 439153.
vaibhav.y added a comment.

Fix formatting in Sarif.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,315 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest-matchers.h"
+#include "gtest/gtest.h"
+
+#include 
+
+using namespace clang;
+
+namespace {
+
+using LineCol = std::pair;
+
+static std::string serializeSarifDocument(llvm::json::Object &) {
+  std::string Output;
+  llvm::json::Value value(std::move(Doc));
+  llvm::raw_string_ostream OS{Output};
+  OS << llvm::formatv("{0}", value);
+  OS.flush();
+  return Output;
+}
+
+class SarifDocumentWriterTest : public ::testing::Test {
+protected:
+  SarifDocumentWriterTest()
+  : InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
+FileMgr(FileSystemOptions(), InMemoryFileSystem),
+DiagID(new DiagnosticIDs()), DiagOpts(new DiagnosticOptions()),
+Diags(DiagID, DiagOpts.get(), new IgnoringDiagConsumer()),
+SourceMgr(Diags, FileMgr) {}
+
+  IntrusiveRefCntPtr InMemoryFileSystem;
+  FileManager FileMgr;
+  IntrusiveRefCntPtr DiagID;
+  IntrusiveRefCntPtr DiagOpts;
+  DiagnosticsEngine Diags;
+  SourceManager SourceMgr;
+  LangOptions LangOpts;
+
+  FileID registerSource(llvm::StringRef Name, const char *SourceText,
+bool IsMainFile = false) {
+std::unique_ptr SourceBuf =
+llvm::MemoryBuffer::getMemBuffer(SourceText);
+const FileEntry *SourceFile =
+FileMgr.getVirtualFile(Name, SourceBuf->getBufferSize(), 0);
+SourceMgr.overrideFileContents(SourceFile, std::move(SourceBuf));
+FileID FID = SourceMgr.getOrCreateFileID(SourceFile, SrcMgr::C_User);
+if (IsMainFile)
+  SourceMgr.setMainFileID(FID);
+return FID;
+  }
+
+  FullSourceRange getFakeFullSourceRange(FileID FID, LineCol Begin,
+ LineCol End) {
+auto BeginLoc = SourceMgr.translateLineCol(FID, Begin.first, Begin.second);
+auto EndLoc = SourceMgr.translateLineCol(FID, End.first, End.second);
+return FullSourceRange{FullSourceLoc{BeginLoc, SourceMgr},
+   FullSourceLoc{EndLoc, SourceMgr}};
+  }
+};
+
+TEST_F(SarifDocumentWriterTest, createEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter Writer{LangOpts};
+
+  // WHEN:
+  const llvm::json::Object  = Writer.createDocument();
+  std::vector Keys(EmptyDoc.size());
+  std::transform(EmptyDoc.begin(), EmptyDoc.end(), Keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(Keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST_F(SarifDocumentWriterTest, documentWithARun) {
+  // GIVEN:
+  SarifDocumentWriter Writer{LangOpts};
+  const char *ShortName = "sariftest";
+  const char *LongName = "sarif writer test";
+
+  // WHEN:
+  Writer.createRun(ShortName, LongName);
+  Writer.endRun();
+  const llvm::json::Object  = Writer.createDocument();
+  const llvm::json::Array *Runs = Doc.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(Runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(Runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const llvm::json::Object *driver =
+  Runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Tests and docs have moved to subdirectories by module name.

Please rebase onto HEAD and:

- move 
`test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp`
 to 
`test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp`
- move 
`docs/clang-tidy/checks/cppcoreguidelines-avoid-const-or-ref-data-members.rst` 
to 
`docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst`
- make the target `test-clang-extra` to validate your tests still pass
- make the target `docs-clang-tools-html` to validate your documentation links 
are correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:61
+   "ignoring the result of 'empty()', did you mean 'clear()'? ");
+  Builder << FixItHint::CreateReplacement(ReplacementRange, "clear");
+}

I wonder if we should add a fixit hint that suggests `x = T{};` in the event 
`x.clear()` isn't available (e.g. `std::stack`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D128372#3602881 , @tschuett wrote:

> Just for reference:
> https://reviews.llvm.org/D128267

Ack. I still think this CL is useful, given that not every library will have 
`[[nodiscard]]`, and because it can suggest appropriate alternatives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128157: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Please rebase this and folder your changes into 
`checkers/cppcoreguidelines/virtual-class-destructor.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128157

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

[PATCH] D128372: Clang-Tidy Empty Check

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Just for reference:
https://reviews.llvm.org/D128267


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

In 
https://github.com/llvm/llvm-project/commit/6e566bc5523f743bc34a7e26f050f1f2b4d699a8,
 The directory structure of the documentation for clang-tidy checks was 
changed, however clangd wasn't updated.
Now all the links generated will point to old dead pages.
This updated clangd to use the new page structure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128379

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test


Index: clang-tools-extra/clangd/test/diagnostics-tidy.test
===
--- clang-tools-extra/clangd/test/diagnostics-tidy.test
+++ clang-tools-extra/clangd/test/diagnostics-tidy.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
 # CHECK-NEXT:"codeDescription": {
-# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html;
+# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html;
 # CHECK-NEXT:},
 # CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'?",
 # CHECK-NEXT:"range": {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -918,9 +918,18 @@
 // information to be worth linking.
 // https://clang.llvm.org/docs/DiagnosticsReference.html
 break;
-  case Diag::ClangTidy:
-return {("https://clang.llvm.org/extra/clang-tidy/checks/; + Name + 
".html")
+  case Diag::ClangTidy: {
+StringRef Module, Check;
+// This won't correctly get the module for clang-analyzer checks, but as we
+// don't link in the analyzer that shouldn't be an issue.
+// This would also need updating if anyone decides to create a module with 
a
+// '-' in the name.
+std::tie(Module, Check) = Name.split('-');
+assert(!Module.empty() && !Check.empty());
+return {("https://clang.llvm.org/extra/clang-tidy/checks/; + Module + "/" +
+ Check + ".html")
 .str()};
+  }
   case Diag::Clangd:
 if (Name == "unused-includes")
   return {"https://clangd.llvm.org/guides/include-cleaner"};


Index: clang-tools-extra/clangd/test/diagnostics-tidy.test
===
--- clang-tools-extra/clangd/test/diagnostics-tidy.test
+++ clang-tools-extra/clangd/test/diagnostics-tidy.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
 # CHECK-NEXT:"codeDescription": {
-# CHECK-NEXT:  "href": "https://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html;
+# CHECK-NEXT:  "href": "https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html;
 # CHECK-NEXT:},
 # CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'?",
 # CHECK-NEXT:"range": {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -918,9 +918,18 @@
 // information to be worth linking.
 // https://clang.llvm.org/docs/DiagnosticsReference.html
 break;
-  case Diag::ClangTidy:
-return {("https://clang.llvm.org/extra/clang-tidy/checks/; + Name + ".html")
+  case Diag::ClangTidy: {
+StringRef Module, Check;
+// This won't correctly get the module for clang-analyzer checks, but as we
+// don't link in the analyzer that shouldn't be an issue.
+// This would also need updating if anyone decides to create a module with a
+// '-' in the name.
+std::tie(Module, Check) = Name.split('-');
+assert(!Module.empty() && !Check.empty());
+return {("https://clang.llvm.org/extra/clang-tidy/checks/; + Module + "/" +
+ Check + ".html")
 .str()};
+  }
   case Diag::Clangd:
 if (Name == "unused-includes")
   return {"https://clangd.llvm.org/guides/include-cleaner"};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

A great start, thanks!




Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57
+auto Methods = MemberCall->getRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});
+
+if (Clear != Methods.end()) {

Note to other reviewers: I suggested this because we couldn't work out a better 
way to identify if .clear() exists (Abraham can best expand on the issues 
regarding how a matcher wasn't working).



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:75-78
+diag(NonMemberLoc, "ignoring the result of '%0'")
+<< llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+   ->getQualifiedNameAsString()
+<< FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

It looks as though this is unconditionally suggesting clear (even if it doesn't 
exist), and doesn't suggest it in the message.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h:18
+
+/// FIXME: Write a short description.
+///

Please fix this :)



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:123
+
+  Warns when `empty()` is used on a container and the result is ignored.
+

* s/container/range
* This should probably mention it suggests `clear()` if it's a member function.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:39
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, 
"Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion 
`_, "Yes"
-   `abseil-faster-strsplit-delimiter 
`_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, 
"Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions 
`_, "Yes"
-   `altera-id-dependent-backward-branch 
`_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   `android-cloexec-pipe2 `_, "Yes"
-   `android-cloexec-socket `_, "Yes"
-   `android-comparison-in-temp-failure-retry 
`_,
-   `boost-use-to-string `_, "Yes"
-   `bugprone-argument-comment `_, "Yes"
-   `bugprone-assert-side-effect `_,
-   `bugprone-bad-signal-to-kill-thread 
`_,
-   `bugprone-bool-pointer-implicit-conversion 
`_, "Yes"
-   `bugprone-branch-clone `_,
-   `bugprone-copy-constructor-init `_, 
"Yes"
-   `bugprone-dangling-handle `_,
-   `bugprone-dynamic-static-initializers 
`_,
-   `bugprone-easily-swappable-parameters 
`_,
-   `bugprone-exception-escape `_,
-   `bugprone-fold-init-type `_,
-   `bugprone-forward-declaration-namespace 
`_,
-   `bugprone-forwarding-reference-overload 
`_,
-   `bugprone-implicit-widening-of-multiplication-result 
`_, "Yes"
-   `bugprone-inaccurate-erase `_, "Yes"
-   `bugprone-incorrect-roundings `_,
-   `bugprone-infinite-loop `_,
-   `bugprone-integer-division `_,
-   `bugprone-lambda-function-name `_,
-   `bugprone-macro-parentheses `_, "Yes"
-   `bugprone-macro-repeated-side-effects 
`_,
-   `bugprone-misplaced-operator-in-strlen-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-pointer-arithmetic-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-widening-cast `_,
-   `bugprone-move-forwarding-reference 
`_, "Yes"
-   `bugprone-multiple-statement-macro 
`_,
-   `bugprone-no-escape `_,
-   `bugprone-not-null-terminated-result 
`_, "Yes"
-   `bugprone-parent-virtual-call `_, "Yes"
-   `bugprone-posix-return `_, "Yes"
-   `bugprone-redundant-branch-condition 
`_, "Yes"
-   `bugprone-reserved-identifier `_, "Yes"
-   `bugprone-shared-ptr-array-mismatch 
`_, "Yes"
-   `bugprone-signal-handler `_,
-   `bugprone-signed-char-misuse `_,
-   `bugprone-sizeof-container `_,
-   `bugprone-sizeof-expression `_,
-   `bugprone-spuriously-wake-up-functions 
`_,
-   

[clang-tools-extra] 165d693 - [clang-tidy][docs] Fix a couple of missed cases from 6e566bc5523

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-06-22T21:37:16+01:00
New Revision: 165d69337a7dbcb88dc054ac8fc3e43f623d4ba7

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

LOG: [clang-tidy][docs] Fix a couple of missed cases from 6e566bc5523

A few of the checks had documentation URLs that weren't updated in the 
aforementioned commit.

Added: 


Modified: 
clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h
clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.h
clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
clang-tools-extra/clang-tidy/hicpp/NoAssemblerCheck.h
clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h 
b/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
index 4b3b8fb603b9f..75d8fdddeb73c 100644
--- a/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
+++ b/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
@@ -19,7 +19,7 @@ namespace cert {
 /// This check warns for such modifications.
 ///
 /// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert/dcl58-cpp.html
 class DontModifyStdNamespaceCheck : public ClangTidyCheck {
 public:
   DontModifyStdNamespaceCheck(StringRef Name, ClangTidyContext *Context)

diff  --git a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h 
b/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
index 3ccf03be1b579..5b84cf4ec7cd7 100644
--- a/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
+++ b/clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
@@ -19,7 +19,7 @@ namespace cert {
 /// object.
 ///
 /// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert/dcl21-cpp.html
 class PostfixOperatorCheck : public ClangTidyCheck {
 public:
   PostfixOperatorCheck(StringRef Name, ClangTidyContext *Context)

diff  --git a/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h 
b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
index 9196194b37750..b899333bb28bc 100644
--- a/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
+++ b/clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
@@ -18,7 +18,7 @@ namespace concurrency {
 /// Checks that non-thread-safe functions are not used.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/threads-mt-unsafe.html
+/// http://clang.llvm.org/extra/clang-tidy/checks/threads/mt-unsafe.html
 class MtUnsafeCheck : public ClangTidyCheck {
 public:
   MtUnsafeCheck(StringRef Name, ClangTidyContext *Context);

diff  --git a/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h 
b/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
index a05bedf60b23e..3866fdc0b346a 100644
--- a/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
+++ b/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
@@ -19,7 +19,7 @@ namespace google {
 /// replaces them with equivalent names containing "suite".
 ///
 /// For the user-facing documentation see:
-/// 
https://clang.llvm.org/extra/clang-tidy/checks/google-upgrade-googletest-case.html
+/// 
https://clang.llvm.org/extra/clang-tidy/checks/google/upgrade-googletest-case.html
 class UpgradeGoogletestCaseCheck : public ClangTidyCheck {
 public:
   UpgradeGoogletestCaseCheck(StringRef Name, ClangTidyContext *Context)

diff  --git 
a/clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h 
b/clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h
index c1b86c953fc59..779bb469fdddc 100644
--- a/clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h
+++ b/clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h
@@ -33,7 +33,7 @@ namespace build {
 /// Corresponding cpplint.py check name: `build/namespaces`.
 ///
 /// For the user-facing documentation see:
-/// 
https://clang.llvm.org/extra/clang-tidy/checks/google-build-using-namespace.html
+/// 
https://clang.llvm.org/extra/clang-tidy/checks/google/build-using-namespace.html
 class UsingNamespaceDirectiveCheck : public ClangTidyCheck {
 public:
   UsingNamespaceDirectiveCheck(StringRef Name, ClangTidyContext *Context)

diff  

[PATCH] D128337: [clang-tidy] Extend spelling for CheckOptions

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 439139.
njames93 marked an inline comment as done.
njames93 added a comment.

Rebased and addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128337

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
  clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -15,12 +15,16 @@
 // CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
 
 // For this test we have to use names of the real checks because otherwise values are ignored.
+// Running with the old key: , value:  CheckOptions
 // RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+// Running with the new :  syntax
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/key-dict/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+
 // CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
-// CHECK-CHILD4-DAG: - key: llvm-qualified-auto.AddConstToQualified{{ *[[:space:]] *}}value: 'true'
-// CHECK-CHILD4-DAG: - key: modernize-loop-convert.MaxCopySize{{ *[[:space:]] *}}value: '20'
-// CHECK-CHILD4-DAG: - key: modernize-loop-convert.MinConfidence{{ *[[:space:]] *}}value: reasonable
-// CHECK-CHILD4-DAG: - key: modernize-use-using.IgnoreMacros{{ *[[:space:]] *}}value: 'false'
+// CHECK-CHILD4-DAG: llvm-qualified-auto.AddConstToQualified: 'true'
+// CHECK-CHILD4-DAG: modernize-loop-convert.MaxCopySize: '20'
+// CHECK-CHILD4-DAG: modernize-loop-convert.MinConfidence: reasonable
+// CHECK-CHILD4-DAG: modernize-use-using.IgnoreMacros: 'false'
 
 // RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
 // CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}44{{[/\\]}}.clang-tidy.
@@ -32,14 +36,21 @@
 // RUN: Checks: -llvm-qualified-auto, \
 // RUN: CheckOptions: [{key: modernize-loop-convert.MaxCopySize, value: 21}]}' \
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD5
+// Also test with the {Key: Value} Syntax specified on command line
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{InheritParentConfig: true, \
+// RUN: Checks: -llvm-qualified-auto, \
+// RUN: CheckOptions: {modernize-loop-convert.MaxCopySize: 21}}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD5
+
 // CHECK-CHILD5: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto,-llvm-qualified-auto
-// CHECK-CHILD5-DAG: - key: modernize-loop-convert.MaxCopySize{{ *[[:space:]] *}}value: '21'
-// CHECK-CHILD5-DAG: - key: modernize-loop-convert.MinConfidence{{ *[[:space:]] *}}value: reasonable
-// CHECK-CHILD5-DAG: - key: modernize-use-using.IgnoreMacros{{ *[[:space:]] *}}value: 'false'
+// CHECK-CHILD5-DAG: modernize-loop-convert.MaxCopySize: '21'
+// CHECK-CHILD5-DAG: modernize-loop-convert.MinConfidence: reasonable
+// CHECK-CHILD5-DAG: modernize-use-using.IgnoreMacros: 'false'
 
 // RUN: clang-tidy -dump-config \
 // RUN: --config='{InheritParentConfig: false, \
 // RUN: Checks: -llvm-qualified-auto}' \
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
-// CHECK-CHILD6-NOT: - key: modernize-use-using.IgnoreMacros
+// CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
@@ -0,0 +1,7 @@
+InheritParentConfig: true
+Checks: 'llvm-qualified-auto'
+CheckOptions:
+  modernize-loop-convert.MaxCopySize: '20'
+  llvm-qualified-auto.AddConstToQualified: 'true'
+  IgnoreMacros: 'false'
+
Index: clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
@@ 

[clang-tools-extra] b1cc59f - [clang-tidy][docs] Reorganise release notes

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-06-22T21:21:24+01:00
New Revision: b1cc59fd3a9bad2fbd654011613909ffe90329b0

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

LOG: [clang-tidy][docs] Reorganise release notes

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index ec7b44405310..b1a5d110a921 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,9 +110,6 @@ Improvements to clang-tidy
   from suppressing diagnostics associated with macro arguments. This fixes
   `Issue 55134 `_.
 
-- Invalid parameters are no longer treated as being implicitly unused for the
-  `-misc-unused-parameters` check. This fixes `Issue 56152 
`_.
-
 New checks
 ^^
 
@@ -182,13 +179,18 @@ Changes in existing checks
   ` when executing for C++ code
   that contain calls to advanced constructs, e.g. overloaded operators.
 
-- Fixed a false positive in :doc:`misc-redundant-expression
-  ` involving overloaded
-  comparison operators.
+- Fixed false positives in :doc:`misc-redundant-expression
+  `:
+
+  - Fixed a false positive involving overloaded comparison operators.
+
+  - Fixed a false positive involving assignments in
+conditions. This fixes `Issue 35853 
`_.
 
-- Fixed a false positive in :doc:`misc-redundant-expression
-  ` involving assignments in
-  conditions. This fixes `Issue 35853 
`_.
+- Fixed a false positive in :doc:`misc-unused-parameters
+  `
+  where invalid parameters were implicitly being treated as being unused. 
+  This fixes `Issue 56152 
`_.
 
 - Fixed a false positive in :doc:`modernize-deprecated-headers
   ` involving including



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

[PATCH] D128337: [clang-tidy] Extend spelling for CheckOptions

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D128337#3602694 , @aaron.ballman 
wrote:

> I like the changes -- this is a much nicer syntax for specifying 
> configuration options!
>
>> The only observable differences are support for the new syntax and 
>> -dump=config will emit using the new syntax.
>
> Do you expect the behavior of `-dump` to cause any problems for folks using 
> that option from a script? I can't think of any that aren't super contrived, 
> but maybe you've got more thoughts there.

I wouldn't expect people are using the dump-config option for any scripts, It's 
only really there for debugging.

> (Note, you should probably rebase your patch as it doesn't seem to apply 
> cleaning, so there's no precommit CI happening for it.)

That's your fault :) 
https://github.com/llvm/llvm-project/commit/bb297024fad2f6c3ccaaa6a5c3a270f73f15f3ac


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128337

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

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:2717
+__m128i Bytes =
+_mm_loadu_si128(reinterpret_cast(CurPtr));
+int Mask = _mm_movemask_epi8(Bytes);

aaron.ballman wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > This crashes when using `_mm_load_si128` which suprises me because 
> > > `CurPtr` is supposedly aligned on a 16 bytes boundary here. Any idea?
> > Wait, did you verify that `CurPtr` really is on a 16-byte boundary, or are 
> > you thinking it should be on such a boundary? (I don't see any alignment 
> > markings on the parameter, so I'd assume it's aligned as any other pointer.)
> Derp, I missed that the loop above is manually aligning the pointer.
> 
> I'm not certain what's going on here with your crash...
Just above, i do
```
while (C != '/' && ((intptr_t)CurPtr & 0x0F) != 0) {
if (!isASCII(C)) {
  CurPtr--;
  break;
}
```

Derp indeed (the `CurPtr--; break` is the culprit, fix shortly)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D119296: KCFI sanitizer

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen updated this revision to Diff 439137.
samitolvanen added a comment.

Fixed the debug output in InstCombine to use metadata as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/kcfi.c
  clang/test/Driver/fsanitize.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrControl.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/kcfi-bti.ll
  llvm/test/CodeGen/AArch64/kcfi.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/kcfi.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/Transforms/InstCombine/kcfi-operand-bundles.ll
  llvm/test/Transforms/TailCallElim/kcfi-bundle.ll
  llvm/test/Verifier/kcfi-operand-bundles.ll
  llvm/test/Verifier/metadata-function-kcfi-type.ll

Index: llvm/test/Verifier/metadata-function-kcfi-type.ll
===
--- /dev/null
+++ llvm/test/Verifier/metadata-function-kcfi-type.ll
@@ -0,0 +1,39 @@
+; RUN: not llvm-as %s -disable-output 2>&1 | FileCheck %s
+
+define void @a() {
+  unreachable
+}
+
+define void @b() !kcfi_type !0 {
+  unreachable
+}
+
+; CHECK: function must have a single !kcfi_type attachment
+define void @f0() !kcfi_type !0 !kcfi_type !0 {
+  unreachable
+}
+!0 = !{i32 10}
+
+; CHECK: !kcfi_type must have exactly one operand
+define void @f1() !kcfi_type !1 {
+  unreachable
+}
+!1 = !{!"string", i32 0}
+
+; CHECK: expected a constant operand for !kcfi_type
+define void @f2() !kcfi_type !2 {
+  unreachable
+}
+!2 = !{!"string"}
+
+; CHECK: expected a constant integer operand for !kcfi_type
+define void @f3() !kcfi_type !3 {
+  unreachable
+}
+!3 = !{ptr @f3}
+
+; CHECK: expected a 32-bit integer constant operand for !kcfi_type
+define void @f4() !kcfi_type !4 {
+  unreachable
+}
+!4 = !{i64 10}
Index: llvm/test/Verifier/kcfi-operand-bundles.ll
===
--- /dev/null
+++ llvm/test/Verifier/kcfi-operand-bundles.ll
@@ -0,0 +1,16 @@
+; RUN: not opt -verify < %s 2>&1 | FileCheck %s
+
+define void @test_kcfi_bundle(i64 %arg0, i32 %arg1, void()* %arg2) {
+; CHECK: Multiple kcfi operand bundles
+; CHECK-NEXT: call void %arg2() [ "kcfi"(i32 42), "kcfi"(i32 42) ]
+  call void %arg2() [ "kcfi"(i32 42), "kcfi"(i32 42) ]
+
+; CHECK: Kcfi bundle operand must be an i32 constant
+; CHECK-NEXT: call void %arg2() [ "kcfi"(i64 42) ]
+  call void %arg2() [ "kcfi"(i64 42) ]
+
+; CHECK-NOT: call
+  call void %arg2() [ "kcfi"(i32 42) ] ; OK
+  call void %arg2() [ "kcfi"(i32 42) ] ; OK
+  ret void
+}
Index: llvm/test/Transforms/TailCallElim/kcfi-bundle.ll
===
--- /dev/null
+++ llvm/test/Transforms/TailCallElim/kcfi-bundle.ll
@@ -0,0 +1,10 @@
+; RUN: opt < %s -tailcallelim -verify-dom-info -S | FileCheck %s
+; Check that the "kcfi" operand bundle doesn't prevent tail calls.
+
+define i64 @f_1(i64 %x, i64(i64)* %f_0) {
+; CHECK-LABEL: @f_1(
+entry:
+; CHECK: tail call i64 %f_0(i64 %x) [ "kcfi"(i32 42) ]
+  %tmp = call i64 %f_0(i64 %x) 

[PATCH] D127901: [LinkerWrapper] Add PTX output to CUDA fatbinary in LTO-mode

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Do we want JIT -> YES, but specalizing LLVM-IR JIT.
Do we want/need PTX, I do not, but I don't mind having it. Someone will ask for 
it eventually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127901

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D123319#3593644 , @aprantl wrote:

> In D123319#3517966 , @dblaikie 
> wrote:
>
>> In D123319#3506745 , @shafik wrote:
>>
>>> 
>>
>> What does the linkage name do for your use case? Which cases are missing 
>> linkage names/where do they go missing?
>>
>>> I am happy to consider other approaches as well to solving lookup for local 
>>> lambdas for D105564 . Let me know what 
>>> you think.
>>
>> Why does the return type help perform lookup? What kind of lookup?
>>
>> (again, my take is that "auto" return types probably shouldn't be described 
>> at all - we should just not describe functions where we don't know their 
>> return types because they're not very useful to know about (overload 
>> resolution, yes, but then you can't call them anyway) - but that's a broader 
>> argument to make/change to make)
>
> IIUC, the motivating problem is (@shafik please correct me if this isn't it) 
> this:
>
>   $ cat /tmp/lambda.cpp 
>   #include 
>   int main() {
> auto f = [](){ printf("hi from lambda\n"); return 1;} ;
> f();
> f();
> return f();
>   }
>   $ clang++ -g /tmp/lambda.cpp -o /tmp/a.out
>   $ lldb /tmp/a.out
>   (lldb) b 5
>   (lldb) r
>   (lldb) p f()
>   hi from lambda
>   (lldb) p auto val = f()
>   error: expression failed to parse:
>   error: :1:6: variable has incomplete type 'void'
>   auto val = f()
>^

OK. Thanks - I can reproduce this. Though it seems like the issue isn't about 
lambdas - but about member functions in general, lambdas or not:

  $ cat test.cpp
  #include 
  auto g = []() {
printf("hi from non-local lambda\n");
return 1;
  };
  struct t2 {
auto operator()() {
  printf("hi from non-local type\n");
  return 1;
}
auto func() {
  printf("hi from non-local named func\n");
  return 1;
}
  };
  int main() {
auto f = []() {
  printf("hi from lambda\n");
  return 1;
};
struct t1 {
  auto operator()() {
printf("hi from local type\n");
return 1;
  }
};
f();
g();
t1 v1;
v1();
t2 v2;
v2();
v2.func();
  }
  $ lldb ./a.out
  (lldb) p f()
  hi from lambda
  (int) $0 = 1
  (lldb) p g()
  hi from non-local lambda
  (int) $1 = 1
  (lldb) p v1()
  hi from local type
  (lldb) p v2()
  hi from non-local type
  (lldb) p v2.func()
  hi from non-local named func
  (lldb) p auto x = f()
  hi from lambda
  (lldb) p auto x = g()
  hi from non-local lambda
  (lldb) p auto x = v1()
  error: expression failed to parse:
  error: :1:6: variable has incomplete type 'void'
  auto x = v1()
   ^
  (lldb) p auto x = v2()
  error: expression failed to parse:
  error: :1:6: variable has incomplete type 'void'
  auto x = v2()
   ^
  (lldb) p auto x = v2.func()
  error: expression failed to parse:
  error: :1:6: variable has incomplete type 'void'
  auto x = v2.func()
   ^

(but a standalone auto function doesn't have the problem... because we don't 
produce declarations for those functions (even in a namespace, we just define 
them in the namespace)

So I think this is a problem with lldb's handling of auto return in general - 
not with lambdas in particular.

And I still have two related questions

1. Why is Clang being changed here, rather than fixing lldb to handle correct 
DWARF?
2. Alternatively: why are we emitting 'auto' return types at all (see previous 
comments/discussion) instead of treating these functions the same way we do for 
member function templates - omit them entirely (don't even emit declarations) 
if the return type isn't known.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123319

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

[PATCH] D128314: [Clang-tidy] Fixing a bug in clang-tidy infinite-loop checker

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 439132.
ziqingluo-90 retitled this revision from "[Clang-tidy] Fixing bugs in 
clang-tidy infinite-loop checker" to "[Clang-tidy] Fixing a bug in clang-tidy 
infinite-loop checker".
ziqingluo-90 edited the summary of this revision.
ziqingluo-90 added a comment.

Separate the change adding handling of noreturn attributes for ObjC nodes.  
Have run clang-format.


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

https://reviews.llvm.org/D128314

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop-noreturn.mm
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3829,6 +3829,25 @@
   InnerMatcher.matches(*ExprNode, Finder, Builder));
 }
 
+/// matches if ObjCMessageExpr's callee declaration matches
+///
+/// Given
+/// \code
+///   @interface I: NSObject
+///   +(void)foo;
+///   @end
+///   ...
+///   [I foo]
+/// \endcode
+/// The example above matches \code [I foo] \endcode with
+/// objcMessageExpr(objcMessageCallee(objcMethodDecl(hasName("foo"
+AST_MATCHER_P(ObjCMessageExpr, objcMessageCallee,
+  internal::Matcher, InnerMatcher) {
+  const ObjCMethodDecl *msgDecl = Node.getMethodDecl();
+  return (msgDecl != nullptr &&
+  InnerMatcher.matches(*msgDecl, Finder, Builder));
+}
+
 /// Matches if the call expression's callee's declaration matches the
 /// given matcher.
 ///
@@ -5070,6 +5089,17 @@
 /// \endcode
 AST_MATCHER(FunctionDecl, isNoReturn) { return Node.isNoReturn(); }
 
+/// matches a Decl if it has a  "no return" attribute of any kind
+AST_MATCHER(Decl, declHasNoReturnAttr) {
+  return Node.hasAttr() || Node.hasAttr() ||
+ Node.hasAttr();
+}
+
+/// matches a FunctionType if the type includes the GNU no return attribute
+AST_MATCHER(FunctionType, typeHasNoReturnAttr) {
+  return Node.getNoReturnAttr();
+}
+
 /// Matches the return type of a function declaration.
 ///
 /// Given:
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop-noreturn.mm
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop-noreturn.mm
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fblocks -fexceptions
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fblocks -fobjc-arc -fexceptions
+
+@interface I
++ (void)foo;
++ (void)bar;
++ (void)baz __attribute__((noreturn));
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+_Noreturn void term();
+
+void plainCFunction() {
+  int i = 0;
+  int j = 0;
+  int a[10];
+
+  while (i < 10) {
+// no warning, function term has C noreturn attribute
+term();
+  }
+  while (i < 10) {
+// no warning, class method baz has noreturn attribute
+[I baz];
+  }
+  while (i + j < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, j) are updated in the loop body [bugprone-infinite-loop]
+[I foo];
+  }
+  while (i + j < 10) {
+[I foo];
+[I baz]; // no warning, class method baz has noreturn attribute
+  }
+
+  void (^block)() = ^{
+  };
+  void __attribute__((noreturn)) (^block_nr)(void) = ^void __attribute__((noreturn)) (void) { throw "err"; };
+
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+block();
+  }
+  while (i < 10) {
+// no warning, the block has "noreturn" arribute
+block_nr();
+  }
+}
+
+@implementation I
++ (void)bar {
+}
+
++ (void)foo {
+  static int i = 0;
+
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+[I bar];
+  }
+}
+@end
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -21,10 +21,17 @@
 
 static internal::Matcher
 loopEndingStmt(internal::Matcher Internal) {
-  // FIXME: Cover noreturn ObjC methods (and blocks?).
+  internal::Matcher isNoReturnFunType =
+  ignoringParens(functionType(typeHasNoReturnAttr()));
+  internal::Matcher isNoReturnDecl =
+  anyOf(declHasNoReturnAttr(), functionDecl(hasType(isNoReturnFunType)),
+varDecl(hasType(blockPointerType(pointee(isNoReturnFunType);
+
   return stmt(anyOf(
   mapAnyOf(breakStmt, returnStmt, gotoStmt, cxxThrowExpr).with(Internal),
-  callExpr(Internal, 

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D113107#3602478 , @zahiraam wrote:

> Do we want to add an -fexcess-precision option?

Well, generally we try to offer the same options GCC does.  However, it looks 
like GCC's `-fexcess-precision` does not line up with the options we're 
thinking of offering: GCC offers only `-fexcess-precision=standard` 
(essentially what you're implementing) and `-fexcess-precision=fast` (which I 
think would require some sort of optimizer annotation to permit folding 
truncations across statement boundaries), with no option to get 
operation-by-operation fidelity (our current behavior).  And I think we're very 
unlikely to default to `-fexcess-precision=fast` the way that GCC apparently 
does.  I'm also not sure if anyone cares enough to enable this for anything 
other than `_Float16`; on GCC, `-fexcess-precision` is much broader, e.g. 
affecting `float` and `double` math on non-SSE x86 targets.  So I think we 
should hold off on adding this option until we know what behavior we actually 
want to expose via it.


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

https://reviews.llvm.org/D113107

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

[PATCH] D128373: [Sema] Check whether `__auto_type` has been deduced before merging

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: aaron.ballman.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

This fixes a bug in clang where it emits the following diagnostic when 
compiling the test case:

`argument to 'sizeof' in 'memset' call is the same pointer type 'S' as the 
destination`

The code that merges `__auto_type` with other types was committed in 
https://reviews.llvm.org/D122029.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128373

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Sema/warn-memset-bad-sizeof.c


Index: clang/test/Sema/warn-memset-bad-sizeof.c
===
--- /dev/null
+++ clang/test/Sema/warn-memset-bad-sizeof.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+void *memset(void *, int, unsigned long);
+
+typedef struct {
+  int a;
+} S;
+
+void test() {
+  S s;
+  __auto_type dstptr = 
+  memset(dstptr, 0, sizeof(s));
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10314,11 +10314,11 @@
 // Allow __auto_type to match anything; it merges to the type with more
 // information.
 if (const auto *AT = LHS->getAs()) {
-  if (AT->isGNUAutoType())
+  if (!AT->isDeduced() && AT->isGNUAutoType())
 return RHS;
 }
 if (const auto *AT = RHS->getAs()) {
-  if (AT->isGNUAutoType())
+  if (!AT->isDeduced() && AT->isGNUAutoType())
 return LHS;
 }
 return {};


Index: clang/test/Sema/warn-memset-bad-sizeof.c
===
--- /dev/null
+++ clang/test/Sema/warn-memset-bad-sizeof.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+void *memset(void *, int, unsigned long);
+
+typedef struct {
+  int a;
+} S;
+
+void test() {
+  S s;
+  __auto_type dstptr = 
+  memset(dstptr, 0, sizeof(s));
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10314,11 +10314,11 @@
 // Allow __auto_type to match anything; it merges to the type with more
 // information.
 if (const auto *AT = LHS->getAs()) {
-  if (AT->isGNUAutoType())
+  if (!AT->isDeduced() && AT->isGNUAutoType())
 return RHS;
 }
 if (const auto *AT = RHS->getAs()) {
-  if (AT->isGNUAutoType())
+  if (!AT->isDeduced() && AT->isGNUAutoType())
 return LHS;
 }
 return {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128372: Clang-Tidy Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added subscribers: carlosgalvezp, abrachet, phosek, mgorny.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang-tools-extra.

Adds a clang-tidy check for the incorrect use of `empty()` on a
container when the result of the call is ignored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+int test_member_empty() {
+  std::vector v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::vector v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+using std::empty;
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+using absl::empty;
+empty(w);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -36,328 +36,329 @@
 .. csv-table::
:header: "Name", "Offers fixes"
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, "Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion `_, "Yes"
-   `abseil-faster-strsplit-delimiter `_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, "Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions `_, "Yes"
-   `altera-id-dependent-backward-branch `_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 439130.

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

https://reviews.llvm.org/D113107

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
 
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main(void) {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex(void) {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
+
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86-PROM
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
@@ -85,7 +86,29 @@
   return a * b;
 }
 _Float16 _Complex mul_half_cc(_Float16 _Complex a, _Float16 _Complex b) {
-  // X86-LABEL: @mul_half_cc(
+  // CHECK: @mul_half_cc(
+
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: %[[AC:[^ ]+]] = fmul float
+  // X86-PROM: %[[BD:[^ ]+]] = fmul float
+  // X86-PROM: %[[AD:[^ ]+]] = fmul float
+  // X86-PROM: %[[BC:[^ ]+]] = fmul float
+  // X86-PROM: %[[RR:[^ ]+]] = fsub float
+  // X86-PROM: %[[RI:[^ ]+]] = fadd float
+  // X86-PROM: fcmp uno float %[[RR]]
+  // X86-PROM: fcmp uno float %[[RI]]
+  // X86-PROM: call <2 x float> @__mulsc3(
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: %[[RETR:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: %[[RETI:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: store half {{.*}}, ptr %[[RETR]]
+  // X86-PROM: store half {{.*}}, ptr %[[RETI]]
+  // X86-PROM: load 

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:2717
+__m128i Bytes =
+_mm_loadu_si128(reinterpret_cast(CurPtr));
+int Mask = _mm_movemask_epi8(Bytes);

aaron.ballman wrote:
> cor3ntin wrote:
> > This crashes when using `_mm_load_si128` which suprises me because `CurPtr` 
> > is supposedly aligned on a 16 bytes boundary here. Any idea?
> Wait, did you verify that `CurPtr` really is on a 16-byte boundary, or are 
> you thinking it should be on such a boundary? (I don't see any alignment 
> markings on the parameter, so I'd assume it's aligned as any other pointer.)
Derp, I missed that the loop above is manually aligning the pointer.

I'm not certain what's going on here with your crash...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155
+def err_export_inline_not_defined : Error<
+  "exported inline functions must be defined within the module purview"
+  " and before any private module fragment">;

iains wrote:
> ChuanqiXu wrote:
> > From my reading, 'exported' is not emphasized.
> it is here:
> https://eel.is/c++draft/module#private.frag-2.1
> ( I agree it is somewhat confusing, but the export makes the linkage 
> external, which the example treats differently from the fn_m() case which has 
> module linkage).
> 
> It is possible that we might need to pull together several pieces of the std 
> and maybe ask core for clarification?
> it is here:
> https://eel.is/c++draft/module#private.frag-2.1
> ( I agree it is somewhat confusing, but the export makes the linkage 
> external, which the example treats differently from the fn_m() case which has 
> module linkage).

hmm... my linkage comment is wrong - however the distinction between exported 
and odr-used seems to be made here (fn_m() and fn_e()).
> 
> It is possible that we might need to pull together several pieces of the std 
> and maybe ask core for clarification?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D128337: [clang-tidy] Extend spelling for CheckOptions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I like the changes -- this is a much nicer syntax for specifying configuration 
options!

> The only observable differences are support for the new syntax and 
> -dump=config will emit using the new syntax.

Do you expect the behavior of `-dump` to cause any problems for folks using 
that option from a script? I can't think of any that aren't super contrived, 
but maybe you've got more thoughts there.

(Note, you should probably rebase your patch as it doesn't seem to apply 
cleaning, so there's no precommit CI happening for it.)




Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:85
+template <>
+void yamlize(IO , ClangTidyOptions::OptionMap , bool,
+ EmptyContext ) {

I'm not super tickled with `IO Io` so if you want to use a more descriptive 
name, feel free. Mostly just changing it for coding style conformance.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113
 
+- .clang-tidy files can now use the more natural dictionary syntax for 
specifying `CheckOptions`
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128337

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 439128.

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

https://reviews.llvm.org/D113107

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
 
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main(void) {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex(void) {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
+
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86-PROM
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
@@ -85,7 +86,29 @@
   return a * b;
 }
 _Float16 _Complex mul_half_cc(_Float16 _Complex a, _Float16 _Complex b) {
-  // X86-LABEL: @mul_half_cc(
+  // CHECK: @mul_half_cc(
+
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: %[[AC:[^ ]+]] = fmul float
+  // X86-PROM: %[[BD:[^ ]+]] = fmul float
+  // X86-PROM: %[[AD:[^ ]+]] = fmul float
+  // X86-PROM: %[[BC:[^ ]+]] = fmul float
+  // X86-PROM: %[[RR:[^ ]+]] = fsub float
+  // X86-PROM: %[[RI:[^ ]+]] = fadd float
+  // X86-PROM: fcmp uno float %[[RR]]
+  // X86-PROM: fcmp uno float %[[RI]]
+  // X86-PROM: call <2 x float> @__mulsc3(
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: %[[RETR:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: %[[RETI:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: store half {{.*}}, ptr %[[RETR]]
+  // X86-PROM: store half {{.*}}, ptr %[[RETI]]
+  // X86-PROM: load 

[PATCH] D127812: [AArch64] Function multiversioning support added.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D127812#3602645 , @erichkeane 
wrote:

> In D127812#3601476 , @danielkiss 
> wrote:
>
>> Your understanding is correct. `target` attribute has two usage model. One 
>> is just redefine the to be used codegen options, this is used already widely 
>> for Arm and AArch64. The other use of the `target` attribute is the multi 
>> versioning and the rational for the `target_version` attribute is the easier 
>> distinction between the two usage mode, also not to break any code out there 
>> by changing the behaviour of an attribute.
>
> I don't think differentiating the uses here is a good idea.  I think it would 
> have been a GREAT idea about 10 years ago, but that ship has already sailed 
> once GCC started using it that way however.  We should be keeping the current 
> behavior, otherwise we're going to have a horrible mix of 
> target/target_version working inconsistently between platforms.

That largely is my concern as well. The existing behavior of `target` is just 
that -- the existing behavior. I think deviating from that existing behavior 
will be confusing in practice. Adding additional attributes doesn't improve 
that confusion because users then have to know to decide between two very 
similar attributes, which means they then need to educate themselves on the 
differences between them. If we're going to push them towards the documentation 
to correctly use the attribute anyway, that's basically the same situation 
they're in today with the confusing dual behavior of `target`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added a comment.

In D128328#3601080 , @ChuanqiXu wrote:

> It looks like we need to handle inline variable as well to match the 
> intention.

can you construct a test-case, where this would apply and which is not already 
diagnosed as incorrect?




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155
+def err_export_inline_not_defined : Error<
+  "exported inline functions must be defined within the module purview"
+  " and before any private module fragment">;

ChuanqiXu wrote:
> From my reading, 'exported' is not emphasized.
it is here:
https://eel.is/c++draft/module#private.frag-2.1
( I agree it is somewhat confusing, but the export makes the linkage external, 
which the example treats differently from the fn_m() case which has module 
linkage).

It is possible that we might need to pull together several pieces of the std 
and maybe ask core for clarification?



Comment at: clang/lib/Sema/SemaModule.cpp:895-905
+  if (auto *FD = dyn_cast(Child)) {
+// [dcl.inline]/7
+// If an inline function or variable that is attached to a named module
+// is declared in a definition domain, it shall be defined in that
+// domain.
+// So, if the current declaration does not have a definition, we must
+// check at the end of the TU (or when the PMF starts) to see that we

ChuanqiXu wrote:
> So we might need to move this to somewhere else.
depending on reading of the cases that can have this effect.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D127812: [AArch64] Function multiversioning support added.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D127812#3601476 , @danielkiss 
wrote:

> In D127812#3599530 , @aaron.ballman 
> wrote:
>
>> In D127812#3587223 , @ilinpv wrote:
>>
>>> In D127812#3585249 , @erichkeane 
>>> wrote:
>>>
 I'm concerned as to the design of this addition, I don't particularly 
 appreciate the reasons for making 'target_clones' different, nor the 
 purpose for adding a new attribute instead of using 'target' for what 
 seems like exactly that?  IF the new spelling is THAT necessary, we 
 perhaps don't need a whole new attribute for it either.
>>>
>>> Thank you for fair concern, "target_clones" for AArch64 has different 
>>> format, semantic, e.g. "default" is not required.  Therefore it diverges 
>>> with X86 in these parts.
>>
>> Is it *necessary* that it diverges like this? (Is there some published 
>> standards document you're trying to conform to, is there an implementation 
>> difficulty with not diverging, something else?)
>
> In ACLE there are a few rules/behaviour documented around what should be the 
> behaviour of the "default" for example. Making for example "default" optional 
> hopefully makes the adation of multi versioning seamless as possible. 
> Compilers won't support from day one these attributes therefore the goal was 
> to make the whole addition of a multi versioned function as less distributive 
> as possible while the code is still compiled with older compilers too.
>
>   // this is the original code
>   // mandating __attribute__ ((target ("default"))) would not work with the 
> today's compilers
>   void foo(){}
>   
>   // new backward compatible code
>   #ifdef __ARM_FEATURE_FUNCTION_MULTI_VERSIONING
>   __attribute__ ((target_version("fancy_feature")))
>   void foo(){}
>   #endif
>
> if the "default" is not mandated here, it felt not right to mandate it for 
> the `target_clones` either.
>
>>> "target" attribute has been already used and supported on AArch64 in a 
>>> different sense, like target("arm"), target("dotprod"), 
>>> target("branch-protection=bti"). The intention of creating new 
>>> "target_version" attribute is not to overlap with that. It also has 
>>> different format, mangling and semantic, e.g. treating function without 
>>> attribute as "default", and option to disable attribute droping function 
>>> multi versions. Do these explanations dispel your concern?
>>
>> Do I understand correctly that AArch64 was using an attribute named `target` 
>> which does not behave like the attribute with the same name in GCC and Clang 
>> on other architectures, and now you'd like to introduce a new attribute 
>> which does behave like `target` but under a different name? If I have that 
>> correct, I don't think that's reasonable -- there's far too much possibility 
>> for confusion with that situation already, and adding a new attribute only 
>> increases the confusion. I'm not certain where the technical debt came from, 
>> but we shouldn't increase the burden on users here; I think `target` and 
>> `target_clones` should be made to work consistently across architectures if 
>> at all possible.
>
> Your understanding is correct. `target` attribute has two usage model. One is 
> just redefine the to be used codegen options, this is used already widely for 
> Arm and AArch64. The other use of the `target` attribute is the multi 
> versioning and the rational for the `target_version` attribute is the easier 
> distinction between the two usage mode, also not to break any code out there 
> by changing the behaviour of an attribute.

I don't think differentiating the uses here is a good idea.  I think it would 
have been a GREAT idea about 10 years ago, but that ship has already sailed 
once GCC started using it that way however.  We should be keeping the current 
behavior, otherwise we're going to have a horrible mix of target/target_version 
working inconsistently between platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D128368: added clear check functionality

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

A great start, thanks! I understand this CL to have been sent out a few minutes 
in advance, so as a note to other reviewers: maybe check back in in ~20 minutes 
please?




Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:52-55
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});

Note to other reviewers: I suggested this because we couldn't work out a better 
way to identify if `.clear()` exists (Abraham can best expand on the issues 
regarding how a matcher wasn't working).



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:75-78
+diag(NonMemberLoc, "ignoring the result of '%0'")
+<< llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+   ->getQualifiedNameAsString()
+<< FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

It looks as though this is unconditionally suggesting `clear` (even if it 
doesn't exist), and doesn't suggest it in the message.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:5-9
 template 
 struct vector {
   bool empty();
   void clear();
 };

Please remove `clear` from `vector`, and add `vector_with_clear`. We need to 
make sure that this works both when `clear` is absent and present.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:16-21
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};

As above. Please also rename `absl::vector` to something like `absl::string` 
(and drop the template altogether).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128368

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

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:2717
+__m128i Bytes =
+_mm_loadu_si128(reinterpret_cast(CurPtr));
+int Mask = _mm_movemask_epi8(Bytes);

cor3ntin wrote:
> This crashes when using `_mm_load_si128` which suprises me because `CurPtr` 
> is supposedly aligned on a 16 bytes boundary here. Any idea?
Wait, did you verify that `CurPtr` really is on a 16-byte boundary, or are you 
thinking it should be on such a boundary? (I don't see any alignment markings 
on the parameter, so I'd assume it's aligned as any other pointer.)



Comment at: clang/lib/Lex/Lexer.cpp:2405-2406
 // Skip over characters in the fast loop.
-while (C != 0 &&// Potentially EOF.
-   C != '\n' && C != '\r')  // Newline or DOS-style newline.
+// Warn on invalid UTF-8 if the corresponding warning is enabled, emitting 
a
+// diagnostic only once per sequence that cannot be decoded.
+while ((!WarnOnInvalidUtf8 || isASCII(C)) && C != 0 && // Potentially EOF.

tahonermann wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > tahonermann wrote:
> > > > I think it would be helpful to include a link to [[ 
> > > > http://unicode.org/review/pr-121.html | Unicode PR issue 121 ]] here 
> > > > and note that the diagnostic follows policy option 1. Likewise for 
> > > > handling of '//' comment styles below. Alternatively, provide the link 
> > > > and a note in the release notes.
> > > +1 -- I hadn't realized there were other options. However, ultimately I 
> > > think this should be committed when WG21 has adopted the paper and then 
> > > it would be even better to have a comment citing which stable name and 
> > > paragraph number the code is implementing from P2295.
> > I don't think this applies as we are not inserting replacement characters, 
> > but I'm all for adding a comment.
> > I don't know if we should reference the c++ wording though, as I don;t 
> > think there is value in not applying that behavior in all language modes.
> Perhaps phrase it something like "diagnostics are issued consistent with the 
> replacement character insertion strategy of Unicode PR-121 policy option 1". 
> The goal is to make it clear that the diagnostic strategy is not out of thin 
> air; that it follows Unicode endorsed behavior.
Tom covered why I would prefer to see a comment, but as for the standards 
reference -- we already put in references to only one language for behavior we 
treat as an extension, so this would be more of the same. That ends up being 
helpful (especially if there's an explicit comment about it being an extension 
in other languages) because it tells the reader how the extension is roughly 
expected to behave as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D128022: [HIP] add -fhip-kernel-arg-name

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+  }
+  if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+  getCodeGenOpts().HIPSaveKernelArgName)
 Fn->setMetadata("kernel_arg_name",

tra wrote:
> tra wrote:
> > Should we consolidate both options into `-fkernel-arg-info` and make 
> > `-cl-kernel-arg-info` an alias to it?
> Also, this check is odd. For some reason only *arg name*  metadata is set 
> conditionally, but for whatever reason OpenCL sets other arg metadata 
> unconditionally.
> 
> Now I'm really curious what's so special about "kernel_arg_name" vs the other 
> arg metadata.
> Should we consolidate both options into `-fkernel-arg-info` and make 
> `-cl-kernel-arg-info` an alias to it?

-cl-kernel-arg-info is an OpenCL option defined in OpenCL spec, therefore is 
made OpenCL only option. It would be confusing to allow it with other languages.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+  }
+  if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+  getCodeGenOpts().HIPSaveKernelArgName)
 Fn->setMetadata("kernel_arg_name",

yaxunl wrote:
> tra wrote:
> > tra wrote:
> > > Should we consolidate both options into `-fkernel-arg-info` and make 
> > > `-cl-kernel-arg-info` an alias to it?
> > Also, this check is odd. For some reason only *arg name*  metadata is set 
> > conditionally, but for whatever reason OpenCL sets other arg metadata 
> > unconditionally.
> > 
> > Now I'm really curious what's so special about "kernel_arg_name" vs the 
> > other arg metadata.
> > Should we consolidate both options into `-fkernel-arg-info` and make 
> > `-cl-kernel-arg-info` an alias to it?
> 
> -cl-kernel-arg-info is an OpenCL option defined in OpenCL spec, therefore is 
> made OpenCL only option. It would be confusing to allow it with other 
> languages.
> Also, this check is odd. For some reason only *arg name*  metadata is set 
> conditionally, but for whatever reason OpenCL sets other arg metadata 
> unconditionally.
> 
> Now I'm really curious what's so special about "kernel_arg_name" vs the other 
> arg metadata.

The other metadata are mandatory because they are necessary for OpenCL runtime 
to set kernel argument.

The kernel argument name is emitted only with -cl-kernel-arg-info since it is 
only used to support clGetKernelArgInfo which requires -cl-kernel-arg-info to 
work.


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

https://reviews.llvm.org/D128022

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

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:2405-2406
 // Skip over characters in the fast loop.
-while (C != 0 &&// Potentially EOF.
-   C != '\n' && C != '\r')  // Newline or DOS-style newline.
+// Warn on invalid UTF-8 if the corresponding warning is enabled, emitting 
a
+// diagnostic only once per sequence that cannot be decoded.
+while ((!WarnOnInvalidUtf8 || isASCII(C)) && C != 0 && // Potentially EOF.

cor3ntin wrote:
> aaron.ballman wrote:
> > tahonermann wrote:
> > > I think it would be helpful to include a link to [[ 
> > > http://unicode.org/review/pr-121.html | Unicode PR issue 121 ]] here and 
> > > note that the diagnostic follows policy option 1. Likewise for handling 
> > > of '//' comment styles below. Alternatively, provide the link and a note 
> > > in the release notes.
> > +1 -- I hadn't realized there were other options. However, ultimately I 
> > think this should be committed when WG21 has adopted the paper and then it 
> > would be even better to have a comment citing which stable name and 
> > paragraph number the code is implementing from P2295.
> I don't think this applies as we are not inserting replacement characters, 
> but I'm all for adding a comment.
> I don't know if we should reference the c++ wording though, as I don;t think 
> there is value in not applying that behavior in all language modes.
Perhaps phrase it something like "diagnostics are issued consistent with the 
replacement character insertion strategy of Unicode PR-121 policy option 1". 
The goal is to make it clear that the diagnostic strategy is not out of thin 
air; that it follows Unicode endorsed behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D128363: [clang][dataflow] Implement functionality for flow condition variable substitution.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:163
+BoolValue ,
+llvm::DenseMap ) {
+  auto It = SubstitutionsCache.find();

Could you refactor it into a free function? It does not use 'this', seems like.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:225-229
+  if (ConstraintsIt != FlowConditionConstraints.end()) {
+auto  = *ConstraintsIt->second;
+return substituteBoolVal(Constraints, SubstitutionsCache);
+  }
+  return getBoolLiteralValue(true);

It is better to put special cases (early returns) first.



Comment at: 
clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:191
+  // Empty flow condition holds regardless of value of boolean X
+  EXPECT_TRUE(Context.flowConditionImplies(FC, FCIndependentOfX));
+}

Could you do the following: build explicitly a BoolValue that represents the 
expected result formula, and then ask the SAT solver to prove equivalence 
between the two? That would be a stronger test than merely checking one 
implication.

Same in the test below.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128363

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

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

[Amanieu d'Antras via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0a4450ecdaf: Rename GCCBuiltin into ClangBuiltin (authored 
by GuillaumeGomez, committed by Amanieu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127460

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/include/llvm/IR/IntrinsicsHexagon.td
  llvm/include/llvm/IR/IntrinsicsMips.td
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/include/llvm/IR/IntrinsicsSystemZ.td
  llvm/include/llvm/IR/IntrinsicsVE.td
  llvm/include/llvm/IR/IntrinsicsVEVL.gen.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/IntrinsicsXCore.td
  llvm/lib/IR/Function.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

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

[PATCH] D128368: added clear check functionality

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128368

Files:
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
@@ -6,76 +6,74 @@
 struct vector {
   bool empty();
   void clear();
-  // CHECK-MESSAGES: HERE
 };
 
-// template 
-// bool empty(T &&);
+template 
+bool empty(T &&);
 
 } // namespace std
 
-// namespace absl {
-// template 
-// struct vector {
-//   bool empty();
-//   void clear();
-//   // CHEscdcCK-MESSAGES: HERE
-// };
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
 
-// template 
-// bool empty(T &&);
-// } // namespace absl
+template 
+bool empty(T &&);
+} // namespace absl
 
 int test_member_empty() {
   std::vector v;
 
   v.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CvfdafHECK-FIXES: {{^  }}v.clear();{{$}}
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
 
-  // absl::vector w;
+  absl::vector w;
 
-  // w.empty();
+  w.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // ChafdafHECK-FIXES: {{^  }}w.clear();{{$}}
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
 }
 
-// int test_qualified_empty() {
-//   absl::vector v;
-
-//   std::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-// }
-
-// int test_unqualified_empty() {
-//   std::vector v;
-
-//   empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::vector w;
-
-//   empty(w);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}w.clear();{{$}}
-
-//   {
-// using std::empty;
-// empty(v);
-// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-// // CHECK-FIXES: {{^  }}  v.clear();{{$}}
-//   }
-
-//   {
-// using absl::empty;
-// empty(w);
-// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-// // CHECK-FIXES: {{^  }}  w.clear();{{$}}
-//   }
-// }
+int test_qualified_empty() {
+  absl::vector v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+using std::empty;
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+using absl::empty;
+empty(w);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 
 namespace clang {
@@ -25,67 +26,57 @@
 namespace bugprone {
 
 void 

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

[Guillaume Gomez via Phabricator via cfe-commits]

GuillaumeGomez updated this revision to Diff 439115.
GuillaumeGomez added a comment.

- Rename GCCBuiltin into ClangBuiltin
- Update #define name as well
- Rename `IsGCC` function parameter into `IsClang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127460

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/include/llvm/IR/IntrinsicsHexagon.td
  llvm/include/llvm/IR/IntrinsicsMips.td
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/include/llvm/IR/IntrinsicsSystemZ.td
  llvm/include/llvm/IR/IntrinsicsVE.td
  llvm/include/llvm/IR/IntrinsicsVEVL.gen.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/IntrinsicsXCore.td
  llvm/lib/IR/Function.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

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

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76
 
+class UncheckedOptionalAccessDiagnosis {
+public:

samestep wrote:
> sgatev wrote:
> > samestep wrote:
> > > samestep wrote:
> > > > sgatev wrote:
> > > > > Move this to a new UncheckedOptionalAccessDiagnosis.(h,cpp)?
> > > > OK! I'll do that next.
> > > Actually, @sgatev would that go under `FlowSensitive/Models/` or just 
> > > under `FlowSensitive/`?
> > I suggest keeping it under `FlowSensitive/Models/` for now. We can change 
> > that at a later point if there's a better alternative.
> Where should I put the `UncheckedOptionalAccessModelOptions` type and 
> `ignorableOptional` function that need to be shared between both the model 
> and the diagnosis?
Same question for the following other helper functions:

- `hasOptionalType`
- `isOptionalMemberCallWithName`
- `isOptionalOperatorCallWithName`

Given how much code is shared between the two, I'm really not sure whether it's 
the best idea to move this into a separate file in this patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127898

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

[PATCH] D128182: [NFC] Switch FloatModeKind enum class to use bitmask enums

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a tiny naming nit. Thank you for the cleanup!




Comment at: clang/include/clang/Basic/TargetInfo.h:895-896
   bool useObjCFPRetForRealType(FloatModeKind T) const {
-assert(T <= FloatModeKind::Last &&
-   "T value is larger than RealTypeUsesObjCFPRetMask can handle");
-return RealTypeUsesObjCFPRetMask & (1 << (int)T);
+int val = llvm::BitmaskEnumDetail::Underlying(T);
+return RealTypeUsesObjCFPRetMask & val;
   }

Alternatively, you can get rid of the local variable entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128182

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 439113.

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

https://reviews.llvm.org/D113107

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
 
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main(void) {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex(void) {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
+
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86-PROM
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
@@ -85,7 +86,29 @@
   return a * b;
 }
 _Float16 _Complex mul_half_cc(_Float16 _Complex a, _Float16 _Complex b) {
-  // X86-LABEL: @mul_half_cc(
+  // CHECK: @mul_half_cc(
+
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: %[[AC:[^ ]+]] = fmul float
+  // X86-PROM: %[[BD:[^ ]+]] = fmul float
+  // X86-PROM: %[[AD:[^ ]+]] = fmul float
+  // X86-PROM: %[[BC:[^ ]+]] = fmul float
+  // X86-PROM: %[[RR:[^ ]+]] = fsub float
+  // X86-PROM: %[[RI:[^ ]+]] = fadd float
+  // X86-PROM: fcmp uno float %[[RR]]
+  // X86-PROM: fcmp uno float %[[RI]]
+  // X86-PROM: call <2 x float> @__mulsc3(
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: %[[RETR:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: %[[RETI:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: store half {{.*}}, ptr %[[RETR]]
+  // X86-PROM: store half {{.*}}, ptr %[[RETI]]
+  // X86-PROM: load 

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a subscriber: abrahamcd.
cjdb added a comment.

I think this CL (which is quite large) might be worth just getting the 
functionality in, and deferring the in-tree usage to a second CL. That second 
CL probably should just lift the existing diagnostics so that they're in the 
message component if the SARIF flag is enabled: nothing fancy. I also expect 
this to be fairly non-intrusive with respect to the compiler, but I expect that 
the diagnostic engine will need some teaching here.

We're intending @abrahamcd to pilot proper true integration, by getting 
overload resolution failure diagnostics to be SARIF-operational.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D127686: [Offloading] Embed the target features in the OffloadBinary

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127686

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

[PATCH] D125723: [MSVC] Add initial support for MSVC pragma optimize

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125723

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

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76
 
+class UncheckedOptionalAccessDiagnosis {
+public:

sgatev wrote:
> samestep wrote:
> > samestep wrote:
> > > sgatev wrote:
> > > > Move this to a new UncheckedOptionalAccessDiagnosis.(h,cpp)?
> > > OK! I'll do that next.
> > Actually, @sgatev would that go under `FlowSensitive/Models/` or just under 
> > `FlowSensitive/`?
> I suggest keeping it under `FlowSensitive/Models/` for now. We can change 
> that at a later point if there's a better alternative.
Where should I put the `UncheckedOptionalAccessModelOptions` type and 
`ignorableOptional` function that need to be shared between both the model and 
the diagnosis?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127898

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 439110.

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

https://reviews.llvm.org/D113107

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
 
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main(void) {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex(void) {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
+
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86-PROM
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
@@ -85,7 +86,29 @@
   return a * b;
 }
 _Float16 _Complex mul_half_cc(_Float16 _Complex a, _Float16 _Complex b) {
-  // X86-LABEL: @mul_half_cc(
+  // CHECK: @mul_half_cc(
+
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: fpext half {{.*}} to float
+  // X86-PROM: %[[AC:[^ ]+]] = fmul float
+  // X86-PROM: %[[BD:[^ ]+]] = fmul float
+  // X86-PROM: %[[AD:[^ ]+]] = fmul float
+  // X86-PROM: %[[BC:[^ ]+]] = fmul float
+  // X86-PROM: %[[RR:[^ ]+]] = fsub float
+  // X86-PROM: %[[RI:[^ ]+]] = fadd float
+  // X86-PROM: fcmp uno float %[[RR]]
+  // X86-PROM: fcmp uno float %[[RI]]
+  // X86-PROM: call <2 x float> @__mulsc3(
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: fptrunc float {{.*}} to half
+  // X86-PROM: %[[RETR:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: %[[RETI:[^ ]+]] = getelementptr inbounds { half, half }
+  // X86-PROM: store half {{.*}}, ptr %[[RETR]]
+  // X86-PROM: store half {{.*}}, ptr %[[RETI]]
+  // X86-PROM: load 

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Thank you for this, it has been annoying me for a while.
I'll give a tentative LG, but I'm no expert in this area so see what everyone 
else says first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127807

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

[PATCH] D128022: [HIP] add -fhip-kernel-arg-name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+  }
+  if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+  getCodeGenOpts().HIPSaveKernelArgName)
 Fn->setMetadata("kernel_arg_name",

Should we consolidate both options into `-fkernel-arg-info` and make 
`-cl-kernel-arg-info` an alias to it?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1845-1846
+  }
+  if (getCodeGenOpts().EmitOpenCLArgMetadata ||
+  getCodeGenOpts().HIPSaveKernelArgName)
 Fn->setMetadata("kernel_arg_name",

tra wrote:
> Should we consolidate both options into `-fkernel-arg-info` and make 
> `-cl-kernel-arg-info` an alias to it?
Also, this check is odd. For some reason only *arg name*  metadata is set 
conditionally, but for whatever reason OpenCL sets other arg metadata 
unconditionally.

Now I'm really curious what's so special about "kernel_arg_name" vs the other 
arg metadata.


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

https://reviews.llvm.org/D128022

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

[PATCH] D128072: [clang-tidy] Organize test files into subdirectories by module (NFC)

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Just noticed something nice about this, cmake targets are generated for each 
module
Should speed up development as you now only need to run the lit tests for the 
module that you are working on `check-clang-extra-clang-tidy-checkers-misc` etc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128072

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D113107#3601873 , @rjmccall wrote:

> Supporting the lowering in the backend is sensible in order to support 
> `-fexcess-precision=16`, because I agree that the most reasonable IR output 
> in that configuration is to simply generate `half` operations.  But under 
> `-fexcess-precision=32`, I do not want the frontend to be in the business of 
> recognizing cases where promoted emission is unnecessary, because that is 
> just a lot of extra complexity for something that's already fairly 
> special-case logic; among other things, it will tend to hide bugs.
>
>> I don't understand, how can we check the missed cases if the promotion was 
>> done in FE?
>
> You simply test that you do not see any unpromoted operations coming out of 
> the frontend.  Any operation emitted on `half` (except conversions to and 
> from other types) is probably a missed opportunity to be doing promoted 
> emission.  For example, if we saw that `x += y + z` performed a `half` 
> operation for the final addition, it would suggest that we had a bug in the 
> emission of `+=`, not just because we were emitting a `half` operation but 
> because we were likely failing to propagate promoted emission into the RHS of 
> the operator, i.e. that we were introducing an unnecessary truncation there.
>
> Of course, knowing that we aren't doing operations on `half` doesn't mean we 
> aren't doing unnecessary truncations explicitly, so we still need to be 
> testing such cases.  But it's an easy way to see bugs when simply scanning 
> the IR generated for a program.

Incidentally this little example here is not working with this patch. I will 
add code to fix it. It is now generating an fadd half.
Do we want to add an -fexcess-precision option?


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

https://reviews.llvm.org/D113107

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

[PATCH] D128072: [clang-tidy] Organize test files into subdirectories by module (NFC)

[Richard via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG89a1d03e2b37: [clang-tidy] Organize test files into 
subdirectories by module (NFC) (authored by LegalizeAdulthood).

Changed prior to commit:
  https://reviews.llvm.org/D128072?vs=437963=439102#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128072

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/external-file.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/flags/internal-file.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/time/time.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/types/optional.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/KERNEL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/VHDL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/vERILOG.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/header-with-decl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-argument-comment/system-header-with-decl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-cxx.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/google-namespaces.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/gtest/gtest-typed-test.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/gtest/gtest.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/gtest/nosuite/gtest/gtest-typed-test.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/gtest/nosuite/gtest/gtest.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/resource/include/stdatomic.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/resource/include/stddef.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/stdio.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/stdlib.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/system/string.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-concat-nested-namespaces/modernize-concat-nested-namespaces.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/assert.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/complex.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/ctype.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/errno.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/fenv.h
  

[PATCH] D127142: [HIP] Link with clang_rt.builtins

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D127142#3601810 , @yaxunl wrote:

> In D127142#3600809 , @MaskRay wrote:
>
>> Magically deciding a default value for --unwindlib or --rtlib is not nice. 
>> You may emit a warning if the selected default happens to be incompatible 
>> with HIP.
>
> We build clang not just for compiling HIP programs. For C/C++ programs, we 
> want to use the default runtime and unwind library.

One choice is that ensure the user specify `-DCMAKE_DEFAULT_RTLIB=compiler-rt`.
(Since compiler-rt can be used with libgcc_s, 
-DCMAKE_DEFAULT_UNWINDLIB=libunwind is not strictly necessary.)

> We only want to change the default runtime and unwind library for HIP.

The correct place is `clang/lib/Driver/ToolChain.cpp 
ToolChain::GetUnwindLibType` (see `"platform"`).
HIP should have its derived `ToolChain` to override the member function.


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

https://reviews.llvm.org/D127142

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

[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.

[weiyi via Phabricator via cfe-commits]

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

@xazax.hun

> Since you always want this function to create a null pointer value, I think 
> it would be less error prone to ask for the location instead of an arbitrary 
> value. Currently, a confused caller could put a non-null value into a table.

Replaced with a getOrCreate factory to encapsulate pointer value creation.

> I think `getAsString` is considered expensive. Could you use `QualType` 
> directly as the key?

Done.

> I am wondering about the future plans regarding how pointers are represented.
> What will be the expected behavior when the analysis discovers that the 
> pointer has a null value? E.g.:
>
>   if (p == nullptr)
>   {
> 
>   }
>
> Would we expect `p` in this case to have the same singleton value in the then 
> block of the if statement?

This is an interesting idea.
IIUC, for non-boolean values, the core framework currently does not dissect the 
equality expression - p == nullptr would be represented by a symbolic boolean, 
but p and nullptr would not be entangled together. 
However, we are working on a pointer nullability analysis on top of the 
framework that should add information to interrelate two pointer values and 
their nullability information when we do a comparison between pointers.




Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:146
+  ///  `PointeeType`.
+  void setNullPointerVal(QualType PointeeType, PointerValue ) {
+assert(NullPointerVals.find(PointeeType.getAsString()) ==

xazax.hun wrote:
> Since you always want this function to create a null pointer value, I think 
> it would be less error prone to ask for the location instead of an arbitrary 
> value. Currently, a confused caller could put a non-null value into a table. 
> Since you always want this function to create a null pointer value, I think 
> it would be less error prone to ask for the location instead of an arbitrary 
> value. Currently, a confused caller could put a non-null value into a table. 





Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:149
+   NullPointerVals.end());
+NullPointerVals[PointeeType.getAsString()] = 
+  }

xazax.hun wrote:
> I think `getAsString` is considered expensive. Could you use `QualType` 
> directly as the key?
> I think `getAsString` is considered expensive. Could you use `QualType` 
> directly as the key?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128056

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