[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Liming Liu via Phabricator via cfe-commits
lime updated this revision to Diff 486764.
lime added a comment.

Just rebased, and the `check-profile` target passed on my local machine.


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

https://reviews.llvm.org/D140554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
  clang/test/SemaCXX/lambda-unevaluated.cpp


Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- clang/test/SemaCXX/lambda-unevaluated.cpp
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -61,9 +61,7 @@
 // Same.
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
-// FIXME: We instantiate the lambdas into the context of the function template,
-//  so we think they're dependent and can't evaluate a call to them.
-void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
+void use_g() { g<6>(&"hello"); } // expected-error {{ambiguous}}
 }
 
 namespace GH51416 {
Index: clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm 
-o - | FileCheck %s
+
+// CHECK-LABEL: define linkonce_odr void 
@"_ZN10Issue579601EIiEENS_1FILNS_3$_0v"()
+namespace Issue57960 {
+template
+class F {};
+
+template
+F<[]{}> E() {
+return {};
+}
+
+static auto f = E();
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13234,10 +13234,11 @@
   // context that isn't a DeclContext (such as a variable template), or when
   // substituting an unevaluated lambda inside of a function's parameter's type
   // - as parameter types are not instantiated from within a function's DC. We
-  // use isUnevaluatedContext() to distinguish the function parameter case.
+  // use evaluation contexts to distinguish the function parameter case.
   CXXRecordDecl::LambdaDependencyKind DependencyKind =
   CXXRecordDecl::LDK_Unknown;
-  if (getSema().isUnevaluatedContext() &&
+  if ((getSema().isUnevaluatedContext() ||
+   getSema().isConstantEvaluatedContext()) &&
   (getSema().CurContext->isFileContext() ||
!getSema().CurContext->getParent()->isDependentContext()))
 DependencyKind = CXXRecordDecl::LDK_NeverDependent;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9624,6 +9624,12 @@
 return ExprEvalContexts.back().isUnevaluated();
   }
 
+  bool isConstantEvaluatedContext() const {
+assert(!ExprEvalContexts.empty() &&
+   "Must be in an expression evaluation context");
+return ExprEvalContexts.back().isConstantEvaluated();
+  }
+
   bool isImmediateFunctionContext() const {
 assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -338,6 +338,8 @@
 - In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
   ``(c ? e1 : e2)`` is no longer considered noreturn.
   `Issue 59792 `_
+- Fix an issue that makes Clang crash on lambda template parameters. This fixes
+  `Issue 57960 `_
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- clang/test/SemaCXX/lambda-unevaluated.cpp
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -61,9 +61,7 @@
 // Same.
 template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
 template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
-// FIXME: We instantiate the lambdas into the context of the function template,
-//  so we think they're dependent and can't evaluate a call to them.
-void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
+void use_g() { g<6>(&"hello"); } // expected-error {{ambiguous}}
 }
 
 namespace GH51416 {
Index: clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm -o - | Fi

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy marked an inline comment as done and 2 inline comments as not done.
vedgy added inline comments.



Comment at: llvm/include/llvm/Support/Path.h:423
+/// tempDirUtf8 pointer previously passed to this function.
+void set_system_temp_directory_erased_on_reboot(const char *tempDirUtf8);
+

vedgy wrote:
> aaron.ballman wrote:
> > Err, I'm not super excited about this new API. For starters, it's not 
> > setting the system temp directory at all (it doesn't make any modifications 
> > to the host system); instead it overrides the system temp directory. But 
> > also, this is pretty fragile due to allowing the user to override the temp 
> > directory after the compiler has already queried for the system temp 
> > directory, so now you get files in two different places. Further, it's 
> > fragile because the caller is responsible for keeping that pointer valid 
> > for the lifetime of the program. Finally, we don't allow you to override 
> > any other system directory that you can query (like the home directory).
> >  it's not setting the system temp directory at all (it doesn't make any 
> > modifications to the host system); instead it overrides the system temp 
> > directory.
> Rename to `override_system_temp_directory_erased_on_reboot()`?
> 
> > this is pretty fragile due to allowing the user to override the temp 
> > directory after the compiler has already queried for the system temp 
> > directory, so now you get files in two different places.
> Unfortunately, I don't know how to prevent this. In practice calling 
> `clang_setTemporaryDirectory()` before `clang_createIndex()` works perfectly 
> well in KDevelop: the //preamble-*.pch// files are created later and never 
> end up in the default temporary directory ///tmp//. The same issue applies to 
> the current querying of the environment variable values repeatedly: the user 
> can set environment variables at any time.
> 
> >  it's fragile because the caller is responsible for keeping that pointer 
> > valid for the lifetime of the program.
> I'd love to duplicate the temporary directory path buffer in Path.cpp. The 
> API would become much easier to use. But then the buffer would have to be 
> destroyed when libclang is unloaded or on program shutdown, which is 
> forbidden by LLVM Coding Standards: //Do not use Static Constructors//. That 
> is, I think the following code in Path.cpp is not acceptable:
> ```
> static SmallVectorImpl &tempDirErasedOnRebootUtf8()
> {
>   static SmallVector value;
>   return value;
> }
> ```
> 
> > we don't allow you to override any other system directory that you can 
> > query (like the home directory).
> Well, one has to start somewhere :) Seriously, overriding directories should 
> be allowed only if it has a practical use case. Not just because overriding 
> others is allowed.
A copy of user-provided temporary directory path buffer can be stored in `class 
CIndexer`. But this API in //llvm/Support/Path.h// would stay fragile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

2023-01-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

I'm happy to review the implementation, but I would first appreciate some 
guidance from @sammccall or @kadircet about whether we should add these 
semantic token kinds in the first place.

They would be a non-standard extension to the LSP token kinds, so I think the 
use case for them should be fairly compelling.

It's true that there is an ambiguity between `<` and `>` as operators, vs. 
template arg/param list delimiters, but, at least in terms of user 
understanding of code, my sense is that the highlighting of the **preceding** 
token should be sufficient to disambiguate -- i.e. it would be some sort of 
type name in the template case, vs. a variable / literal / punctuation ending 
an expression in the operator case.

> This is needed for clients that would like to visualize matching opening and 
> closing angle brackets, which can be valuable in non-trivial template 
> declarations or instantiations.

For this use case, could an editor make use of the recently added operator 
tokens (https://reviews.llvm.org/D136594) instead, inferring that a `<` token 
which does not have an `operator` semantic token is a template delimiter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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


[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

2023-01-06 Thread Takuya Shimizu via Phabricator via cfe-commits
hazohelet updated this revision to Diff 486776.
hazohelet added a comment.

This update limits the warning suppression case to string literals only, and 
delete no longer necessary functions.


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

https://reviews.llvm.org/D140860

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/logical-op-parentheses.c
  clang/test/Sema/logical-op-parentheses.cpp

Index: clang/test/Sema/logical-op-parentheses.cpp
===
--- clang/test/Sema/logical-op-parentheses.cpp
+++ /dev/null
@@ -1,66 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wlogical-op-parentheses
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses
-// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wlogical-op-parentheses 2>&1 | FileCheck %s
-
-#ifdef SILENCE
-// expected-no-diagnostics
-#endif
-
-void logical_op_parentheses(unsigned i) {
-	constexpr bool t = 1;
-  (void)(i ||
- i && i);
-#ifndef SILENCE
-  // expected-warning@-2 {{'&&' within '||'}}
-  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
-
-  static_assert(t ||
- t && t);
-#ifndef SILENCE
-  // expected-warning@-2 {{'&&' within '||'}}
-  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
-
-  static_assert(t && 
- t || t);
-#ifndef SILENCE
-  // expected-warning@-3 {{'&&' within '||'}}
-  // expected-note@-4 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"("
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
-	
-	(void)(i || i && "w00t");
-  (void)("w00t" && i || i);
-  (void)("w00t" && t || t);
-  static_assert(t && t || 0);
-  static_assert(1 && t || t);
-  static_assert(0 || t && t);
-  static_assert(t || t && 1);
-
-  (void)(i || i && "w00t" || i);
-#ifndef SILENCE
-  // expected-warning@-2 {{'&&' within '||'}}
-  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
-
-  (void)(i || "w00t" && i || i);
-#ifndef SILENCE
-  // expected-warning@-2 {{'&&' within '||'}}
-  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
-
-  (void)(i && i || 0);
-  (void)(0 || i && i);
-}
-
Index: clang/test/Sema/logical-op-parentheses.c
===
--- clang/test/Sema/logical-op-parentheses.c
+++ clang/test/Sema/logical-op-parentheses.c
@@ -40,9 +40,33 @@
   (void)("w00t" && i || i);
   (void)("w00t" && t || t);
   (void)(t && t || 0);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
   (void)(1 && t || t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
   (void)(0 || t && t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
   (void)(t || t && 1);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
 
   (void)(i || i && "w00t" || i);
 #ifndef SILENCE
@@ -61,5 +85,17 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
 
   (void)(i && i || 0);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("

[clang] 3c7fe7d - [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-06 Thread Balázs Kéri via cfe-commits

Author: Balázs Kéri
Date: 2023-01-06T11:04:24+01:00
New Revision: 3c7fe7d09da1d3f4ba90e1fce3c480b55e9fd970

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

LOG: [clang][analyzer] Add stream related functions to 
StdLibraryFunctionsChecker.

Additional stream handling functions are added.
These are partially evaluated by StreamChecker, result of the addition is
check for more preconditions and construction of success and failure branches
with specific errno handling.

Reviewed By: Szelethus

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-POSIX.c
clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
clang/test/Analysis/std-c-library-functions-vs-stream-checker.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
index d8c3d7a4a6a66..2011a042e0e91 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
@@ -227,12 +227,12 @@ ProgramStateRef ErrnoChecker::checkRegionChanges(
   // If 'errno' is invalidated we can not know if it is checked or written 
into,
   // allow read and write without bug reports.
   if (llvm::is_contained(Regions, ErrnoRegion))
-return setErrnoStateIrrelevant(State);
+return clearErrnoState(State);
 
   // Always reset errno state when the system memory space is invalidated.
   // The ErrnoRegion is not always found in the list in this case.
   if (llvm::is_contained(Regions, ErrnoRegion->getMemorySpace()))
-return setErrnoStateIrrelevant(State);
+return clearErrnoState(State);
 
   return State;
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index 5f79c162f10df..4f5a5793a0b7c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -246,12 +246,16 @@ Optional getErrnoLoc(ProgramStateRef State) {
   return loc::MemRegionVal{ErrnoR};
 }
 
+ErrnoCheckState getErrnoState(ProgramStateRef State) {
+  return State->get();
+}
+
 ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState) {
   return State->set(EState);
 }
 
-ErrnoCheckState getErrnoState(ProgramStateRef State) {
-  return State->get();
+ProgramStateRef clearErrnoState(ProgramStateRef State) {
+  return setErrnoState(State, Irrelevant);
 }
 
 bool isErrno(const Decl *D) {
@@ -299,6 +303,19 @@ ProgramStateRef setErrnoForStdFailure(ProgramStateRef 
State, CheckerContext &C,
   return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
 }
 
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C,
+ const Expr *InvalE) {
+  const MemRegion *ErrnoR = State->get();
+  if (!ErrnoR)
+return State;
+  State = State->invalidateRegions(ErrnoR, InvalE, C.blockCount(),
+   C.getLocationContext(), false);
+  if (!State)
+return nullptr;
+  return setErrnoState(State, MustBeChecked);
+}
+
 const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
   return getErrnoNoteTag(
   C, (Twine("Assuming that function '") + Twine(Fn) +
@@ -307,6 +324,14 @@ const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, 
llvm::StringRef Fn) {
  .str());
 }
 
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+ llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+  C, (Twine("Function '") + Twine(Fn) +
+  Twine("' indicates failure only by setting of 'errno'"))
+ .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 7d87743f8c33f..975fe1b48556c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -67,6 +67,9 @@ ProgramStateRef setErrnoValue(ProgramStateRef State, 
CheckerContext &C,
 /// Set the errno check state, do not modify the errno value.
 ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState);
 
+/// Clear state of errno (make it irrelevant).
+ProgramStateRef clearErrnoState(ProgramStateRef State);
+
 /// Determine if a `Decl` node related to 'errno'.

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-06 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c7fe7d09da1: [clang][analyzer] Add stream related functions 
to StdLibraryFunctionsChecker. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-vs-stream-checker.c

Index: clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
===
--- clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
+++ clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
@@ -45,12 +45,13 @@
   clang_analyzer_eval(x <= 10); // \
  // stream-warning{{TRUE}} \
  // stdLib-warning{{TRUE}} \
- // both-warning{{TRUE}} \
+ // both-warning{{TRUE}}
 
   clang_analyzer_eval(x == 10); // \
   // stream-warning{{TRUE}} \
   // stream-warning{{FALSE}} \
-  // stdLib-warning{{UNKNOWN}} \
+  // stdLib-warning{{TRUE}} \
+  // stdLib-warning{{FALSE}} \
   // both-warning{{TRUE}} \
   // both-warning{{FALSE}}
 
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -49,3 +49,18 @@
   clang_analyzer_express(buf); // expected-warning {{}} // the message does not really matter \
// expected-note {{}}
 }
+
+int __test_case_note();
+
+int test_case_note_1(int y) {
+  int x = __test_case_note(); // expected-note{{Function returns 0}} \
+  // expected-note{{'x' initialized here}}
+  return y / x; // expected-warning{{Division by zero}} \
+// expected-note{{Division by zero}}
+}
+
+int test_case_note_2(int y) {
+  int x = __test_case_note(); // expected-note{{Function returns 1}}
+  return y / (x - 1); // expected-warning{{Division by zero}} \
+  // expected-note{{Division by zero}}
+}
Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -7,6 +7,12 @@
 // RUN:   -analyzer-config eagerly-assume=false \
 // RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
 
+// CHECK: Loaded summary for: FILE *fopen(const char *restrict pathname, const char *restrict mode)
+// CHECK: Loaded summary for: FILE *tmpfile(void)
+// CHECK: Loaded summary for: FILE *freopen(const char *restrict pathname, const char *restrict mode, FILE *restrict stream)
+// CHECK: Loaded summary for: int fclose(FILE *stream)
+// CHECK: Loaded summary for: int fseek(FILE *stream, long offset, int whence)
+// CHECK: Loaded summary for: int fileno(FILE *stream)
 // CHECK: Loaded summary for: long a64l(const char *str64)
 // CHECK: Loaded summary for: char *l64a(long value)
 // CHECK: Loaded summary for: int access(const char *pathname, int amode)
@@ -63,7 +69,6 @@
 // CHECK: Loaded summary for: void rewinddir(DIR *dir)
 // CHECK: Loaded summary for: void seekdir(DIR *dirp, long loc)
 // CHECK: Loaded summary for: int rand_r(unsigned int *seedp)
-// CHECK: Loaded summary for: int fileno(FILE *stream)
 // CHECK: Loaded summary for: int fseeko(FILE *stream, off_t offset, int whence)
 // CHECK: Loaded summary for: off_t ftello(FILE *stream)
 // CHECK: Loaded summary for: void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset)
@@ -121,6 +126,16 @@
 // CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
 // CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
+typedef struct {
+  int x;
+} FILE;
+FILE *fopen(const char *restrict pathname, const char *restrict mode);
+FILE *tmpfile(void);
+FILE *freopen(const char *restrict pathname, const char *restrict mode,
+  FILE *restrict stream);
+int fclose(FILE *stream);
+int fseek(FILE *stream, long offset, int whence);
+int fileno(FILE *stream);
 long a64l(const char *str64);
 char *l64a(long value);
 int access(const char *pathname, int amode);
@@ -181,9 +196,6 @@
 DIR *opendir(const char *name);
 DIR *fdopendir(int fd);
 int isatty(int fildes);
-typedef struct {
-  int x;
-} FILE;
 FILE *popen(const char *command, const char *type);
 int pclose(FILE *stream);
 int close(int fildes);
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.c

[PATCH] D140307: [clang-tidy] Match derived types in in modernize-loop-convert

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

LGTM, let's give @njames93 a few days in case he has some comments.




Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:258
+   hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+   hasMethod(cxxMethodDecl(hasName("end"),
+   isConst())) // 
hasDeclaration

ccotter wrote:
> ccotter wrote:
> > carlosgalvezp wrote:
> > > Replace tabs with spaces
> > Ah, I think this was before I figured out `arc diff` and was copy/pasting 
> > diff files around :/
> > 
> > Thanks for catching this.
> just to confirm, how did you notice the tabs? I couldnt find them when I 
> downloaded the phab raw diff for any of the versions in the history. I still 
> see a double right arrow `»` in the phab diff - is that indicating a tab or 
> something else?
> 
> As a sanity check, I ran `./clang/tools/clang-format/git-clang-format 
> --binary build/bin/clang-format` from the root of my project and the tool did 
> not produce any changes with the latest version of the changes I submitted.
You are right, it must be a bug in Phabricator!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140307

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


[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

On a second thought, using `getLocalOrGlobal` would still require all checks to 
have 2 private member variables (the raw string option and the parsed header 
option), together with the logic in the constructor to get the option an parse 
that. I would like to get rid of all that as well, ideally every check should 
simply call a function "bool isInHeaderFile(Location)" or "bool 
isInImplementationFile(Location)". Keeping those private variables to get a 
global option is also a bit inconsistent with the rest of checks and might 
cause confusion. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141000

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


[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

I found now what this global `User` option is about:

  /// Specifies the name or e-mail of the user running clang-tidy.
  ///
  /// This option is used, for example, to place the correct user name in TODO()
  /// comments in the relevant check.
  std::optional User;

So it seems to me it's an option used only in one check, and yet it's a global 
option. Based on that I think it's reasonable to have `HeaderFileExtensions` 
and `ImplementationFileExtensions` (which are shared among multiple checks) as 
global as well. It doesn't need to be a CLI option, config-only is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141000

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


[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

2023-01-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added subscribers: yozhu, lanza, smeenai.
smeenai added a comment.

I'm investigating a size increase we observed after this change for Meta's 
Android apps. This increases the size of compiler-rt by 1.6 KB, which is small 
by itself, but then compiler-rt is statically linked into each SO, and some of 
our apps have hundreds of SOs, so the increase adds up to a sizeable total. (We 
would ideally have far fewer SOs, but that's a pretty involved change.)

`-mno-fmv` doesn't help. What I've found is that `cpu_model.c` is getting 
pulled in from the compiler-rt archive into our SOs because of references to 
outlined atomics (`__aarch64_have_lse_atomics`), and then it has the static 
constructor `init_cpu_features`, which pulls in `init_cpu_features_resolver`. 
We're not actually using multi-versioning anywhere, but we're still paying the 
size cost for it as a result. Would we consider moving the newly added 
functions into their own file (or perhaps moving the outlined atomics functions 
into a different file), so that you can use outlined atomics without also 
paying the size cost of function multiversioning if you don't need it?

I also had a couple of general questions, since I think I'm missing something 
obvious:

- How come we need both `init_cpu_features` and `init_cpu_features_resolver`? 
It seems like we're codegenning calls to the latter where needed, so I'm not 
sure what the former is adding on top.
- From what I can see, we're codegenning calls to `init_cpu_features_resolver` 
without any arguments, but the actual definition of that function has two 
arguments. How does that work?


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] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

2023-01-06 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 486787.
TIFitis added a comment.

Fixed Windows build issues,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140292

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

Index: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -193,6 +193,62 @@
   LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD)
 };
 
+/// Values for bit flags used to specify the mapping type for
+/// offloading.
+enum class OpenMPOffloadMappingFlags : uint64_t {
+  /// No flags
+  OMP_MAP_NONE = 0x0,
+  /// Allocate memory on the device and move data from host to device.
+  OMP_MAP_TO = 0x01,
+  /// Allocate memory on the device and move data from device to host.
+  OMP_MAP_FROM = 0x02,
+  /// Always perform the requested mapping action on the element, even
+  /// if it was already mapped before.
+  OMP_MAP_ALWAYS = 0x04,
+  /// Delete the element from the device environment, ignoring the
+  /// current reference count associated with the element.
+  OMP_MAP_DELETE = 0x08,
+  /// The element being mapped is a pointer-pointee pair; both the
+  /// pointer and the pointee should be mapped.
+  OMP_MAP_PTR_AND_OBJ = 0x10,
+  /// This flags signals that the base address of an entry should be
+  /// passed to the target kernel as an argument.
+  OMP_MAP_TARGET_PARAM = 0x20,
+  /// Signal that the runtime library has to return the device pointer
+  /// in the current position for the data being mapped. Used when we have the
+  /// use_device_ptr or use_device_addr clause.
+  OMP_MAP_RETURN_PARAM = 0x40,
+  /// This flag signals that the reference being passed is a pointer to
+  /// private data.
+  OMP_MAP_PRIVATE = 0x80,
+  /// Pass the element to the device by value.
+  OMP_MAP_LITERAL = 0x100,
+  /// Implicit map
+  OMP_MAP_IMPLICIT = 0x200,
+  /// Close is a hint to the runtime to allocate memory close to
+  /// the target device.
+  OMP_MAP_CLOSE = 0x400,
+  /// 0x800 is reserved for compatibility with XLC.
+  /// Produce a runtime error if the data is not already allocated.
+  OMP_MAP_PRESENT = 0x1000,
+  // Increment and decrement a separate reference counter so that the data
+  // cannot be unmapped within the associated region.  Thus, this flag is
+  // intended to be used on 'target' and 'target data' directives because they
+  // are inherently structured.  It is not intended to be used on 'target
+  // enter data' and 'target exit data' directives because they are inherently
+  // dynamic.
+  // This is an OpenMP extension for the sake of OpenACC support.
+  OMP_MAP_OMPX_HOLD = 0x2000,
+  /// Signal that the runtime library should use args as an array of
+  /// descriptor_dim pointers and use args_size as dims. Used when we have
+  /// non-contiguous list items in target update directive
+  OMP_MAP_NON_CONTIG = 0x1000,
+  /// The 16 MSBs of the flags indicate whether the entry is member of some
+  /// struct/class.
+  OMP_MAP_MEMBER_OF = 0x,
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */ OMP_MAP_MEMBER_OF)
+};
+
 enum class AddressSpace : unsigned {
   Generic = 0,
   Global = 1,
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6796,67 +6796,13 @@
 // code for that information.
 class MappableExprsHandler {
 public:
-  /// Values for bit flags used to specify the mapping type for
-  /// offloading.
-  enum OpenMPOffloadMappingFlags : uint64_t {
-/// No flags
-OMP_MAP_NONE = 0x0,
-/// Allocate memory on the device and move data from host to device.
-OMP_MAP_TO = 0x01,
-/// Allocate memory on the device and move data from device to host.
-OMP_MAP_FROM = 0x02,
-/// Always perform the requested mapping action on the element, even
-/// if it was already mapped before.
-OMP_MAP_ALWAYS = 0x04,
-/// Delete the element from the device environment, ignoring the
-/// current reference count associated with the element.
-OMP_MAP_DELETE = 0x08,
-/// The element being mapped is a pointer-pointee pair; both the
-/// pointer and the pointee should be mapped.
-OMP_MAP_PTR_AND_OBJ = 0x10,
-/// This flags signals that the base address of an entry should be
-/// passed to the target kernel as an argument.
-OMP_MAP_TARGET_PARAM = 0x20,
-/// Signal that the runtime library has to return the device pointer
-/// in the current position for the data being mapped. Used when we have the
-/// use_device_ptr or use_device_addr clause.
-OMP_MAP_RETURN_PARAM = 0x40,
-/// This flag signals that the reference being passed is a pointe

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

2023-01-06 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D135750

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


[PATCH] D141118: [clang-tidy][NFC] Remove unused User argument in misc-misleading-bidirectional check

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision.
Herald added a subscriber: xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

It's not used anywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141118

Files:
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp


Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
===
--- clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -90,8 +90,7 @@
 class MisleadingBidirectionalCheck::MisleadingBidirectionalHandler
 : public CommentHandler {
 public:
-  MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check,
- std::optional User)
+  MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check)
   : Check(Check) {}
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override {
@@ -114,8 +113,7 @@
 MisleadingBidirectionalCheck::MisleadingBidirectionalCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  Handler(std::make_unique(
-  *this, Context->getOptions().User)) {}
+  Handler(std::make_unique(*this)) {}
 
 MisleadingBidirectionalCheck::~MisleadingBidirectionalCheck() = default;
 


Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
===
--- clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -90,8 +90,7 @@
 class MisleadingBidirectionalCheck::MisleadingBidirectionalHandler
 : public CommentHandler {
 public:
-  MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check,
- std::optional User)
+  MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check)
   : Check(Check) {}
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override {
@@ -114,8 +113,7 @@
 MisleadingBidirectionalCheck::MisleadingBidirectionalCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  Handler(std::make_unique(
-  *this, Context->getOptions().User)) {}
+  Handler(std::make_unique(*this)) {}
 
 MisleadingBidirectionalCheck::~MisleadingBidirectionalCheck() = default;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

2023-01-06 Thread Takuya Shimizu via Phabricator via cfe-commits
hazohelet updated this revision to Diff 486793.
hazohelet added a comment.

add up the former 2 commits into 1


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

https://reviews.llvm.org/D140860

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/logical-op-parentheses.c

Index: clang/test/Sema/logical-op-parentheses.c
===
--- clang/test/Sema/logical-op-parentheses.c
+++ clang/test/Sema/logical-op-parentheses.c
@@ -8,6 +8,7 @@
 #endif
 
 void logical_op_parentheses(unsigned i) {
+	const unsigned t = 1;
   (void)(i ||
  i && i);
 #ifndef SILENCE
@@ -17,8 +18,55 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
 
-  (void)(i || i && "w00t");
+  (void)(t ||
+ t && t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
+
+  (void)(t && 
+ t || t);
+#ifndef SILENCE
+  // expected-warning@-3 {{'&&' within '||'}}
+  // expected-note@-4 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
+	
+	(void)(i || i && "w00t");
   (void)("w00t" && i || i);
+  (void)("w00t" && t || t);
+  (void)(t && t || 0);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
+  (void)(1 && t || t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
+  (void)(0 || t && t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
+  (void)(t || t && 1);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
 
   (void)(i || i && "w00t" || i);
 #ifndef SILENCE
@@ -37,5 +85,17 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
 
   (void)(i && i || 0);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
   (void)(0 || i && i);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15206,38 +15206,21 @@
 Bop->getSourceRange());
 }
 
-/// Returns true if the given expression can be evaluated as a constant
-/// 'true'.
-static bool EvaluatesAsTrue(Sema &S, Expr *E) {
-  bool Res;
-  return !E->isValueDependent() &&
- E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && Res;
-}
-
-/// Returns true if the given expression can be evaluated as a constant
-/// 'false'.
-static bool EvaluatesAsFalse(Sema &S, Expr *E) {
-  bool Res;
-  return !E->isValueDependent() &&
- E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && !Res;
-}
-
 /// Look for '&&' in the left hand of a '||' expr.
 static void DiagnoseLogicalAndInLogicalOrLHS(Sema &S, SourceLocation OpLoc,
  Expr *LHSExpr, Expr *RHSExpr) {
   if (BinaryOperator *Bop = dyn_cast(LHSExpr)) {
 if (Bop->getOpcode() == BO_LAnd) {
-  // If it's "a && b || 0" don't warn since the precedence doesn't matter.
-  if (EvaluatesAsFalse(S, RHSExpr))
-return;
-  // If it's "1 && a || b" don't warn since the precedence doesn't matter.
-  if (!EvaluatesAsTrue(S, Bop->getLHS()))
+  // If it's 

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Friendly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140290

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


[clang] 5cf8532 - [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-06 Thread Balázs Kéri via cfe-commits

Author: Balázs Kéri
Date: 2023-01-06T12:22:21+01:00
New Revision: 5cf85323a0788ee5666099d6a34c55f70edbc934

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

LOG: [clang][analyzer] Extend StreamChecker with some new functions.

The stream handling functions `ftell`, `rewind`, `fgetpos`, `fsetpos`
are evaluated in the checker more exactly than before.
New tests are added to test behavior of the checker together with
StdLibraryFunctionsChecker. The option ModelPOSIX of that checker
affects if (most of) the stream functions are recognized, and checker
StdLibraryFunctionArgs generates warnings if constraints for arguments
are not satisfied. The state of `errno` is set by StdLibraryFunctionsChecker
too for every case in the stream functions.
StreamChecker works with the stream state only, does not set the errno state,
and is not dependent on other checkers.

Reviewed By: Szelethus

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

Added: 
clang/test/Analysis/stream-errno-note.c
clang/test/Analysis/stream-errno.c
clang/test/Analysis/stream-noopen.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/Inputs/system-header-simulator.h
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index b6516988c087..e02ec4d6e71d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -85,10 +86,10 @@ const StreamErrorState ErrorFError{false, false, true};
 /// Full state information about a stream pointer.
 struct StreamState {
   /// The last file operation called in the stream.
+  /// Can be nullptr.
   const FnDescription *LastOperation;
 
   /// State of a stream symbol.
-  /// FIXME: We need maybe an "escaped" state later.
   enum KindTy {
 Opened, /// Stream is opened.
 Closed, /// Closed stream (an invalid stream pointer after it was closed).
@@ -202,7 +203,7 @@ ProgramStateRef bindAndAssumeTrue(ProgramStateRef State, 
CheckerContext &C,
 ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
 CheckerContext &C, const CallExpr *CE) {
   State = State->BindExpr(CE, C.getLocationContext(),
-  C.getSValBuilder().makeIntVal(Value, false));
+  C.getSValBuilder().makeIntVal(Value, CE->getType()));
   return State;
 }
 
@@ -250,10 +251,14 @@ class StreamChecker : public Checker EofVal;
+
   void evalFopen(const FnDescription *Desc, const CallEvent &Call,
  CheckerContext &C) const;
 
@@ -304,6 +311,18 @@ class StreamChecker : public Checker OptInt =
+tryExpandAsInteger("EOF", C.getPreprocessor()))
+  EofVal = *OptInt;
+else
+  EofVal = -1;
+  }
+
   /// Searches for the ExplodedNode where the file descriptor was acquired for
   /// StreamSym.
   static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
@@ -427,8 +457,7 @@ class StreamChecker : public CheckerisOpened() &&
- "Previous create of error node for non-opened stream failed?");
+  assert(SS->isOpened() && "Stream is expected to be opened");
 }
 
 const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
@@ -458,6 +487,8 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const 
ExplodedNode *N,
 
 void StreamChecker::checkPreCall(const CallEvent &Call,
  CheckerContext &C) const {
+  initEof(C);
+
   const FnDescription *Desc = lookupFn(Call);
   if (!Desc || !Desc->PreFn)
 return;
@@ -575,6 +606,10 @@ void StreamChecker::evalFclose(const FnDescription *Desc, 
const CallEvent &Call,
   if (!SS)
 return;
 
+  auto *CE = dyn_cast_or_null(Call.getOriginExpr());
+  if (!CE)
+return;
+
   assertStreamStateOpened(SS);
 
   // Close the File Descriptor.
@@ -582,7 +617,16 @@ void StreamChecker::evalFclose(const FnDescription *Desc, 
const CallEvent &Call,
   // and can not be used any more.
   State = State->set(Sym, StreamState::getClosed(Desc));
 
-  C.addTransition(State);
+  // Return 0 on success, EOF on failure.
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef StateSuccess = State->BindExpr(
+

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-06 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5cf85323a078: [clang][analyzer] Extend StreamChecker with 
some new functions. (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D140395?vs=485928&id=486797#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140395

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-errno.c
  clang/test/Analysis/stream-error.c
  clang/test/Analysis/stream-noopen.c

Index: clang/test/Analysis/stream-noopen.c
===
--- /dev/null
+++ clang/test/Analysis/stream-noopen.c
@@ -0,0 +1,157 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+// enable only StdCLibraryFunctions checker
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_var.h"
+
+void clang_analyzer_eval(int);
+
+const char *WBuf = "123456789";
+char RBuf[10];
+
+void test_freopen(FILE *F) {
+  F = freopen("xxx", "w", F);
+  if (F) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+}
+
+void test_fread(FILE *F) {
+  size_t Ret = fread(RBuf, 1, 10, F);
+  if (Ret == 10) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fwrite(FILE *F) {
+  size_t Ret = fwrite(WBuf, 1, 10, F);
+  if (Ret == 10) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fclose(FILE *F) {
+  int Ret = fclose(F);
+  if (Ret == 0) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fseek(FILE *F) {
+  int Ret = fseek(F, SEEK_SET, 1);
+  if (Ret == 0) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(Ret == -1); // expected-warning {{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fgetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fsetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_ftell(FILE *F) {
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferro

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

2023-01-06 Thread Takuya Shimizu via Phabricator via cfe-commits
hazohelet added a comment.

> So, so long as a string literal still does the suppression, it's probably 
> fine.

I agree with you. I now also think that integer literals _should not_ do the 
warning suppression because programmers are sometimes unsure of the expansion 
result of predefined macros in the compiled environment.
I updated the differential. I am new to Phabricator and made some unnecessary 
operations. Sorry for bothering you.


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

https://reviews.llvm.org/D140860

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


[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

Thanks. I think this looks good!


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

https://reviews.llvm.org/D140554

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


[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-01-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:13
 
+#include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"

Do we actually need that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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


[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Liming Liu via Phabricator via cfe-commits
lime added a comment.

Could you please help to commit this? My name and email are `Liming Liu 
`


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

https://reviews.llvm.org/D140554

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


[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

2023-01-06 Thread Pavel Iliin via Phabricator via cfe-commits
ilinpv added a comment.

In D127812#4030881 , @smeenai wrote:

> We're not actually using multi-versioning anywhere, but we're still paying 
> the size cost for it as a result. Would we consider moving the newly added 
> functions into their own file (or perhaps moving the outlined atomics 
> functions into a different file), so that you can use outlined atomics 
> without also paying the size cost of function multiversioning if you don't 
> need it?

Function multiversioning expects compiler-rt has __aarch64_cpu_features, it 
will be broken if compiler-rt miss that ( 
clang/lib/Driver/ToolChains/Clang.cpp:7231 ). I believe function 
multiversioning will be used in Android as outline atomics already did.

> I also had a couple of general questions, since I think I'm missing something 
> obvious:
>
> - How come we need both `init_cpu_features` and `init_cpu_features_resolver`? 
> It seems like we're codegenning calls to the latter where needed, so I'm not 
> sure what the former is adding on top.
> - From what I can see, we're codegenning calls to 
> `init_cpu_features_resolver` without any arguments, but the actual definition 
> of that function has two arguments. How does that work?

hwcaps are ABI specified arguments of ifunc resolver. The constructor 
init_cpu_features calls getauxval to initialize hwcaps and then pass them 
explicitly to init_cpu_features_resolver. If resolver called before constructor 
we get init_cpu_features_resolver hwcap and hwcap2 as arguments from dynamic 
loader.


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] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

2023-01-06 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Possible naming hazard here. march=native means target the local processor 
architecture, zen2 or whatever, and we have the host CPU as an offloading 
target already. So what I'd expect this to do is host offloading with the 
openmp runtime compiled for the local variant of x86 or aarch64, not for it to 
have a guess at a GPU target.

What you think of offload-arch=GPU for pick a plausible GPU? That distinguishes 
it from other things we might want to offload to. Open question whether it 
should create a vgpu instance if it can't detect a physical card.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141105

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


[PATCH] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D141105#4031103 , @JonChesterfield 
wrote:

> Possible naming hazard here. march=native means target the local processor 
> architecture, zen2 or whatever, and we have the host CPU as an offloading 
> target already. So what I'd expect this to do is host offloading with the 
> openmp runtime compiled for the local variant of x86 or aarch64, not for it 
> to have a guess at a GPU target.
>
> What you think of offload-arch=GPU for pick a plausible GPU? That 
> distinguishes it from other things we might want to offload to. Open question 
> whether it should create a vgpu instance if it can't detect a physical card.

There is some prior art here, e.g. CUDA 

 and CMake 
, but we 
don't necessarily need to follow them. I'm in favor of `native` because it 
tracks with what the user "expects" `native` to do. It may be somewhat 
ambiguous given that we can offload to the host, but I think native in the 
future should just go with whatever we can detect no the user's system. So if 
someone has some future FPGA it'll detect that. We can still use `native` in 
the host way with this syntax, we just can't infer the triple. e.g. `clang 
foo.c -fopenmp -fopenmp-targets=x86_64-unknown-linux-gnu --offload-arch=native` 
will do what you except.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141105

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


[PATCH] D112932: Use llvm.is_fpclass to implement FP classification functions

2023-01-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: clang/include/clang/Basic/Builtins.def:482-484
+BUILTIN(__builtin_issubnormal, "i.", "FnctE")
+BUILTIN(__builtin_iszero,  "i.", "FnctE")
+BUILTIN(__builtin_issignaling, "i.", "FnctE")

Sorry, I should have clarified, all the new builtins should be added separately 
from the change of the implementation of the existing builtins


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112932

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


[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

2023-01-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In D127812#4031101 , @ilinpv wrote:

> In D127812#4030881 , @smeenai wrote:
>
>> We're not actually using multi-versioning anywhere, but we're still paying 
>> the size cost for it as a result. Would we consider moving the newly added 
>> functions into their own file (or perhaps moving the outlined atomics 
>> functions into a different file), so that you can use outlined atomics 
>> without also paying the size cost of function multiversioning if you don't 
>> need it?
>
> Function multiversioning expects compiler-rt has __aarch64_cpu_features, it 
> will be broken if compiler-rt miss that ( 
> clang/lib/Driver/ToolChains/Clang.cpp:7231 ). I believe function 
> multiversioning will be used in Android as outline atomics already did.

We can use `-mno-fmv` to avoid that dependency, right? We're interested in 
using that for our own code (where we don't make use of function 
multi-versioning), and want to prevent the compiler-rt support from being 
pulled in from the archive unnecessarily. It'd still be available for users who 
needed it.

>> I also had a couple of general questions, since I think I'm missing 
>> something obvious:
>>
>> - How come we need both `init_cpu_features` and 
>> `init_cpu_features_resolver`? It seems like we're codegenning calls to the 
>> latter where needed, so I'm not sure what the former is adding on top.
>> - From what I can see, we're codegenning calls to 
>> `init_cpu_features_resolver` without any arguments, but the actual 
>> definition of that function has two arguments. How does that work?
>
> hwcaps are ABI specified arguments of ifunc resolver. The constructor 
> init_cpu_features calls getauxval to initialize hwcaps and then pass them 
> explicitly to init_cpu_features_resolver. If resolver called before 
> constructor we get init_cpu_features_resolver hwcap and hwcap2 as arguments 
> from dynamic loader.

Got it, thanks.


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


[clang] 051cc46 - [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Erich Keane via cfe-commits

Author: Liming Liu
Date: 2023-01-06T05:56:25-08:00
New Revision: 051cc460ba9117c9f62c09235bdf6f5ae1248dd4

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

LOG: [C++20] Determine the dependency of unevaluated lambdas more accurately

During template instantiation, the instantiator will enter constant
evaluated
context before instantiate a template argument originated from an
expression,
and this impedes the instantiator from creating lambdas with independent
types.

This patch solves the problem via widening the condition that the
instantiator
marks lambdas as never dependent, and fixes the issue #57960

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

Added: 
clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/lambda-unevaluated.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b69067139ffa5..f2cfac238613c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -338,6 +338,8 @@ Bug Fixes
 - In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
   ``(c ? e1 : e2)`` is no longer considered noreturn.
   `Issue 59792 `_
+- Fix an issue that makes Clang crash on lambda template parameters. This fixes
+  `Issue 57960 `_
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bac06b8fbfbeb..3d9d96c904a0a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9624,6 +9624,12 @@ class Sema final {
 return ExprEvalContexts.back().isUnevaluated();
   }
 
+  bool isConstantEvaluatedContext() const {
+assert(!ExprEvalContexts.empty() &&
+   "Must be in an expression evaluation context");
+return ExprEvalContexts.back().isConstantEvaluated();
+  }
+
   bool isImmediateFunctionContext() const {
 assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2ebd936b359f7..075066472df0c 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13234,10 +13234,11 @@ 
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
   // context that isn't a DeclContext (such as a variable template), or when
   // substituting an unevaluated lambda inside of a function's parameter's type
   // - as parameter types are not instantiated from within a function's DC. We
-  // use isUnevaluatedContext() to distinguish the function parameter case.
+  // use evaluation contexts to distinguish the function parameter case.
   CXXRecordDecl::LambdaDependencyKind DependencyKind =
   CXXRecordDecl::LDK_Unknown;
-  if (getSema().isUnevaluatedContext() &&
+  if ((getSema().isUnevaluatedContext() ||
+   getSema().isConstantEvaluatedContext()) &&
   (getSema().CurContext->isFileContext() ||
!getSema().CurContext->getParent()->isDependentContext()))
 DependencyKind = CXXRecordDecl::LDK_NeverDependent;

diff  --git a/clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp 
b/clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
new file mode 100644
index 0..9cc6ebe93af48
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm 
-o - | FileCheck %s
+
+// CHECK-LABEL: define linkonce_odr void 
@"_ZN10Issue579601EIiEENS_1FILNS_3$_0v"()
+namespace Issue57960 {
+template
+class F {};
+
+template
+F<[]{}> E() {
+return {};
+}
+
+static auto f = E();
+}

diff  --git a/clang/test/SemaCXX/lambda-unevaluated.cpp 
b/clang/test/SemaCXX/lambda-unevaluated.cpp
index fb58b3e0f2021..6d8c3b88c2eaf 100644
--- a/clang/test/SemaCXX/lambda-unevaluated.cpp
+++ b/clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -61,9 +61,7 @@ void use_f() { f({}); } // expected-error {{ambiguous}}
 // Same.
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
-// FIXME: We instantiate the lambdas into the context of the function template,
-//  so we think they're dependent and can't evaluate a call to them.
-void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
+void use_g() { g<6>(&"hello"); } // expected-error {{ambiguous}}
 }
 
 namespace GH51416 {



___
cfe-commits mailin

[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG051cc460ba91: [C++20] Determine the dependency of 
unevaluated lambdas more accurately (authored by lime, committed by erichkeane).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
  clang/test/SemaCXX/lambda-unevaluated.cpp


Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- clang/test/SemaCXX/lambda-unevaluated.cpp
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -61,9 +61,7 @@
 // Same.
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
 template void g(const char (*)[([]{ return N; })()]) {} // 
expected-note {{candidate}}
-// FIXME: We instantiate the lambdas into the context of the function template,
-//  so we think they're dependent and can't evaluate a call to them.
-void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
+void use_g() { g<6>(&"hello"); } // expected-error {{ambiguous}}
 }
 
 namespace GH51416 {
Index: clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm 
-o - | FileCheck %s
+
+// CHECK-LABEL: define linkonce_odr void 
@"_ZN10Issue579601EIiEENS_1FILNS_3$_0v"()
+namespace Issue57960 {
+template
+class F {};
+
+template
+F<[]{}> E() {
+return {};
+}
+
+static auto f = E();
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13234,10 +13234,11 @@
   // context that isn't a DeclContext (such as a variable template), or when
   // substituting an unevaluated lambda inside of a function's parameter's type
   // - as parameter types are not instantiated from within a function's DC. We
-  // use isUnevaluatedContext() to distinguish the function parameter case.
+  // use evaluation contexts to distinguish the function parameter case.
   CXXRecordDecl::LambdaDependencyKind DependencyKind =
   CXXRecordDecl::LDK_Unknown;
-  if (getSema().isUnevaluatedContext() &&
+  if ((getSema().isUnevaluatedContext() ||
+   getSema().isConstantEvaluatedContext()) &&
   (getSema().CurContext->isFileContext() ||
!getSema().CurContext->getParent()->isDependentContext()))
 DependencyKind = CXXRecordDecl::LDK_NeverDependent;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9624,6 +9624,12 @@
 return ExprEvalContexts.back().isUnevaluated();
   }
 
+  bool isConstantEvaluatedContext() const {
+assert(!ExprEvalContexts.empty() &&
+   "Must be in an expression evaluation context");
+return ExprEvalContexts.back().isConstantEvaluated();
+  }
+
   bool isImmediateFunctionContext() const {
 assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -338,6 +338,8 @@
 - In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
   ``(c ? e1 : e2)`` is no longer considered noreturn.
   `Issue 59792 `_
+- Fix an issue that makes Clang crash on lambda template parameters. This fixes
+  `Issue 57960 `_
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- clang/test/SemaCXX/lambda-unevaluated.cpp
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -61,9 +61,7 @@
 // Same.
 template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
 template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
-// FIXME: We instantiate the lambdas into the context of the function template,
-//  so we think they're dependent and can't evaluate a call to them.
-void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
+void use_g() { g<6>(&"hello"); } // expected-error {{ambiguous}}
 }
 
 namespace GH51416 {
Index: clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-unevaluat

[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Done, though I think it is more than time for you to ask for commit permissions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140554

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


[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

2023-01-06 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 added a comment.

In D140795#4022493 , @awarzynski 
wrote:

> Hi @madanial , thanks for posting this!
>
>> This patch adds user option -funderscoring/-fnounderscoring which behaves 
>> similar to the gfortran option be enabling/disabling the 
>> ExternalNameConversionPass
>
> I don't quite understand what this option is for and it's hard to deduce from 
> the patch. Please, could you add a link to some documentation? And tests.
>
> -Andrzej

@awarzynski  Thanks for the review. @madanial will not be available for the 
next few weeks. I will try to address some review comments while he is away.

The purpose of this option is to control the trailing underscore being appended 
to external names (e.g. procedure names, common block names). The option in 
gfortran is documented in 
https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html.

However, I don't think the patch does what we want. Given a procedure name 
`foo`, the `-fno-underscoring` option will give `_QPfoo` instead of `foo`. We 
will look into it.

However, I don't think the patch does what we want. Given a procedure name 
`foo`, the `-fno-underscoring` option will give `_QPfoo` instead of `foo`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140795

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


[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

2023-01-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter created this revision.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Warn when a lambda specifies a default capture and captures
``this``. Offer FixIts to correct the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t
+
+struct Obj {
+  void lambdas_that_warn_default_capture_copy() {
+int local{};
+int local2{};
+
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };
+
+auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
+
+auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&  local, &local2, this]() { return (local+x) > 10; }
+
+auto explicit_this_capture_local_ref_with_comment = [=, & /* byref */ local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [& /* byref */ local, &local2, this]() { return (local+x) > 10; }
+
+auto implicit_this_capture = [=]() { return x > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { return x > 10; 

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

2023-01-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel abandoned this revision.
ymandel added a comment.

Abandoning in favor of  https://reviews.llvm.org/D140694.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123032

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


[clang] b62906b - [clang-format] fix template closer followed by >

2023-01-06 Thread via cfe-commits

Author: Backl1ght
Date: 2023-01-06T22:50:48+08:00
New Revision: b62906b0d10fb7a6cd2e2ceb0f350cec326540d7

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

LOG: [clang-format] fix template closer followed by >

fix https://github.com/llvm/llvm-project/issues/59785

Reviewed By: HazardyKnusperkeks, MyDeveloperDay, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 9dd92c9b08f14..e37ffbde85a26 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -166,10 +166,9 @@ class AnnotatingParser {
 // parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==
- 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
- -1))) {
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {
   return false;
 }
 Left->MatchingParen = CurrentToken;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index b2a1e2c57f8ca..242e3f85eafea 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10341,6 +10341,8 @@ TEST_F(FormatTest, UnderstandsTemplateParameters) {
   verifyFormat("bool_constant");
   verifyFormat("bool_constant");
 
+  verifyFormat("if (std::tuple_size_v > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index dba893dfdd50b..66903f1fe8981 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -339,6 +339,14 @@ TEST_F(TokenAnnotatorTest, UnderstandsTemplatesInMacros) {
   EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsGreaterAfterTemplateCloser) {
+  auto Tokens = annotate("if (std::tuple_size_v > 0)");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");



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


[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-06 Thread Zhikai Zeng via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb62906b0d10f: [clang-format] fix template closer followed by 
> (authored by Backl1ght).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140843

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -339,6 +339,14 @@
   EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsGreaterAfterTemplateCloser) {
+  auto Tokens = annotate("if (std::tuple_size_v > 0)");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10341,6 +10341,8 @@
   verifyFormat("bool_constant");
   verifyFormat("bool_constant");
 
+  verifyFormat("if (std::tuple_size_v > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -166,10 +166,9 @@
 // parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==
- 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
- -1))) {
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {
   return false;
 }
 Left->MatchingParen = CurrentToken;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -339,6 +339,14 @@
   EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsGreaterAfterTemplateCloser) {
+  auto Tokens = annotate("if (std::tuple_size_v > 0)");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10341,6 +10341,8 @@
   verifyFormat("bool_constant");
   verifyFormat("bool_constant");
 
+  verifyFormat("if (std::tuple_size_v > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -166,10 +166,9 @@
 // parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==
- CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
- -1))) {
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {
   return false;
 }
 Left->MatchingParen = CurrentToken;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2023-01-06 Thread Hansang Bae via Phabricator via cfe-commits
hbae added a comment.

Looks like GCC 7.5 cannot build LLVM after this change. Could you please take a 
look?

  In file included from 
/localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:11:0:
  
/localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:
 In member function ‘virtual llvm::Expected 
> 
llvm::omp::target::plugin::GenericDeviceTy::doJITPostProcessing(std::unique_ptr)
 const’:
  
/localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:389:12:
 error: could not convert ‘MB’ from ‘std::unique_ptr’ to 
‘llvm::Expected >’
   return MB;
  ^~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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


[PATCH] D141118: [clang-tidy][NFC] Remove unused User argument in misc-misleading-bidirectional check

2023-01-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko accepted this revision.
Eugene.Zelenko added a comment.
This revision is now accepted and ready to land.

Looks OK for me, but please wait for more active developers' opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141118

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


[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D139287#4031469 , @hbae wrote:

> Looks like GCC 7.5 cannot build LLVM after this change. Could you please take 
> a look?
>
>   In file included from 
> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:11:0:
>   
> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:
>  In member function ‘virtual 
> llvm::Expected > 
> llvm::omp::target::plugin::GenericDeviceTy::doJITPostProcessing(std::unique_ptr)
>  const’:
>   
> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:389:12:
>  error: could not convert ‘MB’ from ‘std::unique_ptr’ to 
> ‘llvm::Expected >’
>return MB;
>   ^~

Some older GCC's have problem with the implicit move on copy elision AFAIK. 
I'll add a `std::move` and let me know if that fixes it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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


[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

2023-01-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:115
+  Warns when lambda specify a capture default and capture ``this``. The check 
also
+  offers FixIts.
+

`fix-it`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst:6
+
+Default lambda captures in member functions can be misleading about
+whether data members are captured by value or reference. For example,

Please make first statement same as in Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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


[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2023-01-06 Thread Hansang Bae via Phabricator via cfe-commits
hbae added a comment.

In D139287#4031473 , @jhuber6 wrote:

> In D139287#4031469 , @hbae wrote:
>
>> Looks like GCC 7.5 cannot build LLVM after this change. Could you please 
>> take a look?
>>
>>   In file included from 
>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:11:0:
>>   
>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:
>>  In member function ‘virtual 
>> llvm::Expected > 
>> llvm::omp::target::plugin::GenericDeviceTy::doJITPostProcessing(std::unique_ptr)
>>  const’:
>>   
>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:389:12:
>>  error: could not convert ‘MB’ from ‘std::unique_ptr’ to 
>> ‘llvm::Expected >’
>>return MB;
>>   ^~
>
> Some older GCC's have problem with the implicit move on copy elision AFAIK. 
> I'll add a `std::move` and let me know if that fixes it.

Yes, that fixes it. We have two more places in JIT.cpp (line 126, 185).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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


[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D139287#4031503 , @hbae wrote:

> In D139287#4031473 , @jhuber6 wrote:
>
>> In D139287#4031469 , @hbae wrote:
>>
>>> Looks like GCC 7.5 cannot build LLVM after this change. Could you please 
>>> take a look?
>>>
>>>   In file included from 
>>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:11:0:
>>>   
>>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:
>>>  In member function ‘virtual 
>>> llvm::Expected > 
>>> llvm::omp::target::plugin::GenericDeviceTy::doJITPostProcessing(std::unique_ptr)
>>>  const’:
>>>   
>>> /localdisk/hbae/LLVM/llvm-base/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:389:12:
>>>  error: could not convert ‘MB’ from ‘std::unique_ptr’ 
>>> to ‘llvm::Expected >’
>>>return MB;
>>>   ^~
>>
>> Some older GCC's have problem with the implicit move on copy elision AFAIK. 
>> I'll add a `std::move` and let me know if that fixes it.
>
> Yes, that fixes it. We have two more places in JIT.cpp (line 126, 185).

Thanks for pointing them out, I'll fix them right away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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


[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

2023-01-06 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Can you link relevant LLVM reviews or documentation and perhaps add some more 
details into the description?

It's  a bit suboptimal though to emit this form of types as SPIR-V specific.  
Ideally Clang should worry as less as possible about target specifics. So it 
would be better if we emit OpenCL opaque types as opaque in LLVM IR and then 
let backends expand those to whatever they need them to be.  There isn't 
technically anything SPIR-V specific in ``target("spirv.Image", void, 1, 1, 0, 
0, 0, 0, 0)`` as this form  is just a better representation of the type with 
more HL information preserved that can either be used or discarded by the 
optimiser/backend.

My worry is that with the current approach more complexity is being added into 
the common parts of the compiler. At least it might be better to hide this 
alternative generation paths into the target implementation itself just like 
for example we ask target hooks to provide mapping to address spaces. Then we 
could just have a default path from which targets can derive from...




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2263
+llvm::Type *LlvmTy = ConvertType(DestTy);
+if (auto *PointerTy = dyn_cast(LlvmTy))
+  return CGF.CGM.getNullPointer(PointerTy, DestTy);

This case at least deserve a comment with explanation. Otherwise I am confused 
why we end up with a non-pointer type as destination in NullToPointer cast?



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:80
+
+  if (useSPIRVTargetExtType(CGM)) {
+switch (cast(T)->getKind()) {

Perhaps it's best to split into separate functions and reflect in naming what 
they correspond to.



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:100
+#define INTEL_SUBGROUP_AVC_TYPE(Name, Id)  
\
+case BuiltinType::OCLIntelSubgroupAVC##Id: 
\
+  return llvm::TargetExtType::get(Ctx, "spirv.Avc" #Id "INTEL");

Why does this need special handling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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


[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

2023-01-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 486873.
ccotter marked an inline comment as done.
ccotter added a comment.

- docs feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t
+
+struct Obj {
+  void lambdas_that_warn_default_capture_copy() {
+int local{};
+int local2{};
+
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };
+
+auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
+
+auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&  local, &local2, this]() { return (local+x) > 10; }
+
+auto explicit_this_capture_local_ref_with_comment = [=, & /* byref */ local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [& /* byref */ local, &local2, this]() { return (local+x) > 10; }
+
+auto implicit_this_capture = [=]() { return x > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { return x > 10; };
+
+auto implicit_this_capture_local = [=]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: lambdas that capture this should not specify a capture default [cppcoreguideline

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

2023-01-06 Thread Mario Emmenlauer via Phabricator via cfe-commits
emmenlau added a comment.
Herald added a project: All.

I can confirm that the problem in OpenSSL is resolved as of OpenSSL 1.1.1s and 
ClangCl 15.0.6.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

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


[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

You do raise a good point about duplication, Therefore does it make sense to 
also do the same with the IncludeStyle as lots of checks add new includes.
Though there is a precedent to do away with LLVM/Google style, as clang-format 
should be responsible for reordering inserted includes.

In D141000#4030877 , @carlosgalvezp 
wrote:

> I found now what this global `User` option is about:
>
>   /// Specifies the name or e-mail of the user running clang-tidy.
>   ///
>   /// This option is used, for example, to place the correct user name in 
> TODO()
>   /// comments in the relevant check.
>   std::optional User;
>
> So it seems to me it's an option used only in one check, and yet it's a 
> global option. Based on that I think it's reasonable to have 
> `HeaderFileExtensions` and `ImplementationFileExtensions` (which are shared 
> among multiple checks) as global as well. It doesn't need to be a CLI option, 
> config-only is fine.

Yeah, I think there is definitely precedent to move that field out of the 
Options. IMO the google-readability-todo check should instead read that field 
from the Check GlobalOptions, and its default value should be set in the 
`GoogleModule::getModuleOptions` virtual function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141000

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


[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

> Therefore does it make sense to also do the same with the IncludeStyle as 
> lots of checks add new includes.

Sure, that could also be done, though I think it'd be preferable to do it in a 
separate patch. Personally I'm a bit reluctant to having formatting options in 
clang-tidy at all, since that is responsibility of clang-format - then a 
project has duplicated settings in both tools, and it gets tricky when they get 
in conflict. If a project cares about formatting they would most likely have 
clang-format as a pre-commit check anyway to take care of formatting after 
clang-tidy has fixed the rest. But if people want to keep the option I won't 
oppose :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141000

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


[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

2023-01-06 Thread Fahad Nayyar via Phabricator via cfe-commits
fahadnayyar updated this revision to Diff 486897.
fahadnayyar marked an inline comment as done.
fahadnayyar added a comment.
Herald added a project: libunwind.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libunwind.

Fixed libcxx failure and added test case for expected warning for shift assigsn 
operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c
  libunwind/src/AddressSpace.hpp

Index: libunwind/src/AddressSpace.hpp
===
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -246,7 +246,7 @@
 inline int64_t LocalAddressSpace::getSLEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
-  int64_t result = 0;
+  uint64_t result = 0;
   int bit = 0;
   uint8_t byte;
   do {
@@ -260,7 +260,7 @@
   if ((byte & 0x40) != 0 && bit < 64)
 result |= (-1ULL) << bit;
   addr = (pint_t) p;
-  return result;
+  return (int64_t)result;
 }
 
 inline LocalAddressSpace::pint_t
Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,6 +13374,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13392,8 +13397,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13682,9 +13695,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, boo

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

2023-01-06 Thread Fahad Nayyar via Phabricator via cfe-commits
fahadnayyar marked 2 inline comments as done.
fahadnayyar added a comment.

In D139114#4023456 , @aaron.ballman 
wrote:

> It looks like this change breaks libc++ (see the precommit CI failures) by 
> making more changes than expected. I'm now seeing `implicit conversion 
> changes signedness` diagnostics where we didn't previously get them. Is that 
> expected and intentional? (I think it may be a fix: 
> https://godbolt.org/z/hTaaf8c5P so I'm adding the libc++ folks just in case 
> they disagree.)
>
> Also, these changes should come with an entry in the release notes.

I've made some change to suppress the warning in libcxx, please have a look and 
let me know if the change can break the semantics of the function in any way. 
Since gcc also give warning on this example, I guess we should include this 
behaviour in clang as well.




Comment at: clang/lib/Sema/SemaChecking.cpp:13400-13401
 
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&

aaron.ballman wrote:
> This comment isn't quite accurate, right? It's checking for any kind of 
> implicit conversion issue (such as changing signs even if the integer widths 
> stay the same).
Changed the comment!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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


[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

2023-01-06 Thread Pavel Iliin via Phabricator via cfe-commits
ilinpv added a comment.

In D127812#4031313 , @smeenai wrote:

> We can use `-mno-fmv` to avoid that dependency, right? We're interested in 
> using that for our own code (where we don't make use of function 
> multi-versioning), and want to prevent the compiler-rt support from being 
> pulled in from the archive unnecessarily. It'd still be available for users 
> who needed it.

Right, you will need to explicitly provide '-mno-fmv` then. Currently 
__aarch64_cpu_features stuff located in bultins 
(`libclang_rt.builtins-aarch64.a`). Did I understand you correctly that your 
apps linked agains libclang_rt.builtins-aarch64.a and if we move function 
multiversioning part to new library, lets say 
`libclang_rt.cpu_features-aarch64.a`, that will resolve your concern ? As a 
sidenote, builtins/cpu_model.c contains X86 CPU features used in function 
multiversioning on that target as well.


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] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 486901.
jhuber6 added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141105

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/openmp-system-arch.c

Index: clang/test/Driver/openmp-system-arch.c
===
--- /dev/null
+++ clang/test/Driver/openmp-system-arch.c
@@ -0,0 +1,50 @@
+// RUN: mkdir -p %t
+// RUN: cp %S/Inputs/amdgpu-arch/amdgpu_arch_fail %t/
+// RUN: cp %S/Inputs/amdgpu-arch/amdgpu_arch_gfx906 %t/
+// RUN: cp %S/Inputs/nvptx-arch/nvptx_arch_fail %t/
+// RUN: cp %S/Inputs/nvptx-arch/nvptx_arch_sm_70 %t/
+// RUN: echo '#!/bin/sh' > %t/amdgpu_arch_empty
+// RUN: chmod +x %t/amdgpu_arch_fail
+// RUN: chmod +x %t/amdgpu_arch_gfx906
+// RUN: chmod +x %t/amdgpu_arch_empty
+// RUN: echo '#!/bin/sh' > %t/nvptx_arch_empty
+// RUN: chmod +x %t/nvptx_arch_fail
+// RUN: chmod +x %t/nvptx_arch_sm_70
+// RUN: chmod +x %t/nvptx_arch_empty
+
+// case when nvptx-arch and amdgpu-arch return nothing or fails
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_fail --amdgpu-arch-tool=%t/amdgpu_arch_fail %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=NO-OUTPUT-ERROR
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_empty --amdgpu-arch-tool=%t/amdgpu_arch_empty %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=NO-OUTPUT-ERROR
+// NO-OUTPUT-ERROR: error: failed to deduce triple for target architecture 'native'; specify the triple using '-fopenmp-targets' and '-Xopenmp-target' instead.
+
+// case when amdgpu-arch succeeds.
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_fail --amdgpu-arch-tool=%t/amdgpu_arch_gfx906 %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=ARCH-GFX906
+// ARCH-GFX906: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx906"
+
+// case when nvptx-arch succeeds.
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_sm_70 --amdgpu-arch-tool=%t/amdgpu_arch_fail %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=ARCH-SM_70
+// ARCH-SM_70: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_70"
+
+// case when both nvptx-arch and amdgpu-arch succeed.
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_sm_70 --amdgpu-arch-tool=%t/amdgpu_arch_gfx906 %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=ARCH-SM_70-GFX906
+// ARCH-SM_70-GFX906: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx906"
+// ARCH-SM_70-GFX906: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_70"
+
+// case when both nvptx-arch and amdgpu-arch succeed with other archs.
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -fopenmp --offload-arch=native,sm_75,gfx1030 \
+// RUN: --nvptx-arch-tool=%t/nvptx_arch_sm_70 --amdgpu-arch-tool=%t/amdgpu_arch_gfx906 %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=ARCH-MULTIPLE
+// ARCH-MULTIPLE: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx1030"
+// ARCH-MULTIPLE: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx906"
+// ARCH-MULTIPLE: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_70"
+// ARCH-MULTIPLE: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_75"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -848,9 +848,30 @@
   HostTC->getTriple());
 
   // Attempt to deduce the offloading triple from the set of architectures.
-  // We can only correctly deduce NVPTX / AMDGPU triples currently.
-  llvm::DenseSet Archs =
-  getOffloadArchs(C, C.getArgs(), Action::OFK_OpenMP, nullptr);
+  // We can only correctly deduce NVPTX / AMDGPU triples currently. We need
+  // to temporarily create these toolchains so that we can access tools for
+  // inferring architectures.
+  llvm::DenseSet Archs;
+  if (NVPTXTriple) {
+auto TempTC = std::make_unique(
+*this, *NVPTXTriple, *HostTC, C.getInputArgs());
+for (StringRef Arch : getOffloadArchs(
+ C, C.getArgs(), Action::OFK_OpenMP, &*TempTC, true))
+  Archs.insert(Arch);
+  }
+  if (AMDTriple) {
+auto TempTC = std::make_unique(
+*this, *AMDTriple, *HostTC, C.getInputArgs());
+for (StringRef Arch : getOffloadArchs(
+ C, C.getA

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

2023-01-06 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

The risk now is that this might significantly regress/add new findings for this 
warning that may not be sufficiently bug-finding to be worth immediate cleanup, 
causing users to have to choose between extensive lower-value cleanup and 
disabling the warning entirely.

Have you/could you run this over a significant codebase to see what sort of new 
findings the modified warning finds, to see if they're high quality bug 
finding, or mostly noise/check for whether this starts to detect certain idioms 
we want to handle differently?

It might be hard to find a candidate codebase that isn't already warning-clean 
with GCC (at least Clang/LLVM wouldn't be a good candidate because of this) & 
maybe that's sufficient justification to not worry too much about this 
outcome...

@aaron.ballman curious what your take on this might be


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

https://reviews.llvm.org/D140860

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


[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision.
Herald added subscribers: arphaman, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Specify that the .clang-tidy file is in YAML format.
- Document the options that may be used in the .clang-tidy file,
- Add missing documentation for existing options (User).
- Fix output example of --dump-config (incorrect format of CheckOptions).
- Fix spurious newline after the dash that comes after every command-line 
option. This was inconsistent with single-line descriptions, which lacked a 
newline. The description is now aligned with the dash and the corresponding 
command-line option, more visually pleasing.

This will enable documenting upcoming global clang-tidy
configuration options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-ap

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486913.
carlosgalvezp added a comment.

Remove extra newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-apply-replacements.
---extra-arg=   - Additional argument to append to the compiler command line.
- Can be used several times.
---extra-arg-before=- Additional argument to prepend to the compiler command line.
- Can be used several times.
---fix  -
- Apply suggested fixes. Without -fix-errors
+--extra-arg=   - Additional argument to append to the compiler command line
+--extra-arg-before=- Additional argument to prepend to the compiler command line
+--fix  - Apply suggested fixes. Without -fix-errors
  

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486915.
carlosgalvezp added a comment.

Remove wrong dash in key/value options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-apply-replacements.
---extra-arg=   - Additional argument to append to the compiler command line.
- Can be used several times.
---extra-arg-before=- Additional argument to prepend to the compiler command line.
- Can be used several times.
---fix  -
- Apply suggested fixes. Without -fix-errors
+--extra-arg=   - Additional argument to append to the compiler command line
+--extra-arg-before=- Additional argument to prepend to the compiler command line
+--fix  - Apply suggested fixes. Without

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486916.
carlosgalvezp added a comment.

Fix incorrect dash in key/value pairs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-apply-replacements.
---extra-arg=   - Additional argument to append to the compiler command line.
- Can be used several times.
---extra-arg-before=- Additional argument to prepend to the compiler command line.
- Can be used several times.
---fix  -
- Apply suggested fixes. Without -fix-errors
+--extra-arg=   - Additional argument to append to the compiler command line
+--extra-arg-before=- Additional argument to prepend to the compiler command line
+--fix  - Apply suggested fixes. Without -

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486921.
carlosgalvezp added a comment.

Remove colon after the config options, for consistency with
the command-line options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-apply-replacements.
---extra-arg=   - Additional argument to append to the compiler command line.
- Can be used several times.
---extra-arg-before=- Additional argument to prepend to the compiler command line.
- Can be used several times.
---fix  -
- Apply suggested fixes. Without -fix-errors
+--extra-arg=   - Additional argument to append to the compiler command line
+--extra-arg-before=- Additional argument to prepend to the compiler command line
+--fix

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486922.
carlosgalvezp added a comment.

Fix alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -122,8 +122,7 @@
 
   clang-tidy options:
 
---checks=  -
- Comma-separated list of globs with optional '-'
+--checks=  - Comma-separated list of globs with optional '-'
  prefix. Globs are processed in order of
  appearance in the list. Globs without '-'
  prefix add checks with matching names to the
@@ -132,21 +131,18 @@
  checks. This option's value is appended to the
  value of the 'Checks' option in .clang-tidy
  file, if any.
---config=  -
- Specifies a configuration in YAML/JSON format:
+--config=  - Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: {x, y}}"
+ CheckOptions: {x: y}}"
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
-Specify the path of .clang-tidy or custom config file:
+--config-file= - Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
-This option internally works exactly the same way as
+ This option internally works exactly the same way as
   --config option after reading specified config file.
-Use either --config-file or --config, not both.
---dump-config  -
- Dumps configuration in the YAML format to
+ Use either --config-file or --config, not both.
+--dump-config  - Dumps configuration in the YAML format to
  stdout. This option can be used along with a
  file name (and '--' if the file is outside of a
  project with configured compilation database).
@@ -154,38 +150,29 @@
  printed.
  Use along with -checks=* to include
  configuration of all checks.
---enable-check-profile -
- Enable per-check timing profiles, and print a
+--enable-check-profile - Enable per-check timing profiles, and print a
  report to stderr.
---explain-config   -
- For each enabled check explains, where it is
+--explain-config   - For each enabled check explains, where it is
  enabled, i.e. in clang-tidy binary, command
  line or a specific configuration file.
---export-fixes=  -
- YAML file to store suggested fixes in. The
+--export-fixes=  - YAML file to store suggested fixes in. The
  stored fixes can be applied to the input source
  code with clang-apply-replacements.
---extra-arg=   - Additional argument to append to the compiler command line.
- Can be used several times.
---extra-arg-before=- Additional argument to prepend to the compiler command line.
- Can be used several times.
---fix  -
- Apply suggested fixes. Without -fix-errors
+--extra-arg=   - Additional argument to append to the compiler command line
+--extra-arg-before=- Additional argument to prepend to the compiler command line
+--fix  - Apply suggested fixes. Without -fix-errors
 

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko accepted this revision.
Eugene.Zelenko added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:142
   e.g. 
--config-file=/some/path/myTidyConfigFile
-This option internally works exactly the 
same way as
+ This option internally works exactly the 
same way as
   --config option after reading specified 
config file.

Is this merge artifact? Same below.



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:215
+--use-color- Use colors in diagnostics. If not set, 
colors
+ will be used if the terminal connected to
+ standard output supports colors.

Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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


[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

2023-01-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In D127812#4031849 , @ilinpv wrote:

> In D127812#4031313 , @smeenai wrote:
>
>> We can use `-mno-fmv` to avoid that dependency, right? We're interested in 
>> using that for our own code (where we don't make use of function 
>> multi-versioning), and want to prevent the compiler-rt support from being 
>> pulled in from the archive unnecessarily. It'd still be available for users 
>> who needed it.
>
> Right, you will need to explicitly provide '-mno-fmv` then. Currently 
> __aarch64_cpu_features cpu_model.c stuff located in bultins 
> (`libclang_rt.builtins-aarch64.a`). Did I understand you correctly that your 
> apps linked agains libclang_rt.builtins-aarch64.a and if we move function 
> multiversioning part to new library, lets say 
> `libclang_rt.cpu_features-aarch64.a`, that will resolve your concern ? As a 
> sidenote, builtins/cpu_model.c contains X86 CPU features used in function 
> multiversioning on that target as well.

It doesn't need to be in a separate library. It just needs to be in a different 
source file than the outlined atomics, so that it only gets included in the 
link if it's explicitly referenced and not just because outlined atomics are 
used.

You're right that it conceptually makes sense for this to be in `cpu_model.c` 
though. An alternative would be providing an option for compiler-rt to be built 
without the multiversioning support, e.g. if it's built with `-mno-fmv` itself. 
Does that seem reasonable?


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] D140694: [clang][dataflow] Only model struct fields that are used in the function being analyzed.

2023-01-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 486926.
ymandel added a comment.

fix use-after-free bug in test. Possible cause of buildbot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140694

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -16,6 +16,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"
@@ -39,17 +40,22 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  llvm::StringRef TargetFun = "target") {
   using ast_matchers::hasName;
+  llvm::SmallVector ASTBuildArgs = {
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+  auto AI =
+  AnalysisInputs(Code, hasName(TargetFun),
+   [&Options](ASTContext &C, Environment &) {
+ return NoopAnalysis(C, Options);
+   })
+  .withASTBuildArgs(ASTBuildArgs);
+  if (Options.BuiltinTransferOpts &&
+  Options.BuiltinTransferOpts->ContextSensitiveOpts)
+AI = std::move(AI).withContextSensitivity();
   ASSERT_THAT_ERROR(
   checkDataflow(
-  AnalysisInputs(Code, hasName(TargetFun),
-   [Options](ASTContext &C, Environment &) {
- return NoopAnalysis(C, Options);
-   })
-  .withASTBuildArgs(
-  {"-fsyntax-only", "-fno-delayed-template-parsing",
-   "-std=" +
-   std::string(LangStandard::getLangStandardForKind(Std)
-   .getName())}),
+  std::move(AI),
   /*VerifyResults=*/
   [&Match](const llvm::StringMap>
&Results,
@@ -151,6 +157,7 @@
 
 void target() {
   A Foo;
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -198,6 +205,7 @@
 
 void target() {
   A Foo = Gen();
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -238,11 +246,13 @@
 TEST(TransferTest, ClassVarDecl) {
   std::string Code = R"(
 class A {
+ public:
   int Bar;
 };
 
 void target() {
   A Foo;
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -336,6 +346,10 @@
 
 void target() {
   A &Foo = getA();
+  (void)Foo.Bar.FooRef;
+  (void)Foo.Bar.FooPtr;
+  (void)Foo.Bar.BazRef;
+  (void)Foo.Bar.BazPtr;
   // [[p]]
 }
   )";
@@ -478,6 +492,10 @@
 
 void target() {
   A *Foo = getA();
+  (void)Foo->Bar->FooRef;
+  (void)Foo->Bar->FooPtr;
+  (void)Foo->Bar->BazRef;
+  (void)Foo->Bar->BazPtr;
   // [[p]]
 }
   )";
@@ -891,7 +909,7 @@
 };
 
 void target(A Foo) {
-  (void)0;
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -1052,6 +1070,9 @@
   int APrivate;
 public:
   int APublic;
+
+private:
+  friend void target();
 };
 
 class B : public A {
@@ -1060,10 +1081,20 @@
   int BProtected;
 private:
   int BPrivate;
+
+private:
+  friend void target();
 };
 
 void target() {
   B Foo;
+  (void)Foo.ADefault;
+  (void)Foo.AProtected;
+  (void)Foo.APrivate;
+  (void)Foo.APublic;
+  (void)Foo.BDefault;
+  (void)Foo.BProtected;
+  (void)Foo.BPrivate;
   // [[p]]
 }
   )";
@@ -1202,6 +1233,7 @@
 
 void target() {
   B Foo;
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -1525,7 +1557,11 @@
   int Bar;
 
   void target() {
-(void)0; // [[p]]
+A a;
+// Mention the fields to ensure they're included in the analysis.
+(void)a.Foo;
+(void)a.Bar;
+// [[p]]
   }
 };
   )";
@@ -1777,6 +1813,7 @@
 
 void target() {
   A Foo = A();
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ -1814,6 +1851,7 @@
 
 void target() {
   A Foo = A();
+  (void)Foo.Bar;
   // [[p]]
 }
   )";
@@ 

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp marked 2 inline comments as done.
carlosgalvezp added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:142
   e.g. 
--config-file=/some/path/myTidyConfigFile
-This option internally works exactly the 
same way as
+ This option internally works exactly the 
same way as
   --config option after reading specified 
config file.

Eugene.Zelenko wrote:
> Is this merge artifact? Same below.
I honestly have no idea, it seems some artifact of Phabricator that is 
unrelated to the code which is just fine. It's not a tab either, checked with 
my editor. I have seen it in patches from other people as well, leading to 
confusion...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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


[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

2023-01-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko accepted this revision.
Eugene.Zelenko added a comment.
This revision is now accepted and ready to land.

But will be good idea to wait for other reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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


[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

2023-01-06 Thread Fahad Nayyar via Phabricator via cfe-commits
fahadnayyar updated this revision to Diff 486930.
fahadnayyar marked an inline comment as done.
fahadnayyar added a comment.

Added summary in clang release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c
  libunwind/src/AddressSpace.hpp

Index: libunwind/src/AddressSpace.hpp
===
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -246,7 +246,7 @@
 inline int64_t LocalAddressSpace::getSLEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
-  int64_t result = 0;
+  uint64_t result = 0;
   int bit = 0;
   uint8_t byte;
   do {
@@ -260,7 +260,7 @@
   if ((byte & 0x40) != 0 && bit < 64)
 result |= (-1ULL) << bit;
   addr = (pint_t) p;
-  return result;
+  return (int64_t)result;
 }
 
 inline LocalAddressSpace::pint_t
Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,6 +13374,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13392,8 +13397,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13682,9 +13695,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   

[clang] 7d0d34f - Re-land "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"

2023-01-06 Thread via cfe-commits

Author: ziqingluo-90
Date: 2023-01-06T10:33:21-08:00
New Revision: 7d0d34fbb153ef044b254d41ae91fc57efbdc77d

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

LOG: Re-land "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher 
that skips callable declarations"

This reverts commit 22df4549a3718dcd8b387ba8246978349e4be50c.

After a quick investigation, realizing that the Sanitizer test
failures caused by this patch is not likely to block other
contributors. I re-land this patch before taking a closer look at
those tests so that it won't block the [-Wunsafe-buffer-usage]
development.

Added: 


Modified: 
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Removed: 




diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a699d27c1544c..80a54c3ad38b7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -8,12 +8,112 @@
 
 #include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/SmallVector.h"
 
 using namespace llvm;
 using namespace clang;
 using namespace ast_matchers;
 
+namespace clang::ast_matchers {
+// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
+// except for those belonging to a 
diff erent callable of "n".
+class MatchDescendantVisitor
+: public RecursiveASTVisitor {
+public:
+  typedef RecursiveASTVisitor VisitorBase;
+
+  // Creates an AST visitor that matches `Matcher` on all
+  // descendants of a given node "n" except for the ones
+  // belonging to a 
diff erent callable of "n".
+  MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
+ internal::ASTMatchFinder *Finder,
+ internal::BoundNodesTreeBuilder *Builder,
+ internal::ASTMatchFinder::BindKind Bind)
+  : Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind),
+Matches(false) {}
+
+  // Returns true if a match is found in a subtree of `DynNode`, which belongs
+  // to the same callable of `DynNode`.
+  bool findMatch(const DynTypedNode &DynNode) {
+Matches = false;
+if (const Stmt *StmtNode = DynNode.get()) {
+  TraverseStmt(const_cast(StmtNode));
+  *Builder = ResultBindings;
+  return Matches;
+}
+return false;
+  }
+
+  // The following are overriding methods from the base visitor class.
+  // They are public only to allow CRTP to work. They are *not *part
+  // of the public API of this class.
+
+  // For the matchers so far used in safe buffers, we only need to match
+  // `Stmt`s.  To override more as needed.
+
+  bool TraverseDecl(Decl *Node) {
+if (!Node)
+  return true;
+if (!match(*Node))
+  return false;
+// To skip callables:
+if (isa(Node))
+  return true;
+// Traverse descendants
+return VisitorBase::TraverseDecl(Node);
+  }
+
+  bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) {
+if (!Node)
+  return true;
+if (!match(*Node))
+  return false;
+// To skip callables:
+if (isa(Node))
+  return true;
+return VisitorBase::TraverseStmt(Node);
+  }
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const {
+// TODO: let's ignore implicit code for now
+return false;
+  }
+
+private:
+  // Sets 'Matched' to true if 'Matcher' matches 'Node'
+  //
+  // Returns 'true' if traversal should continue after this function
+  // returns, i.e. if no match is found or 'Bind' is 'BK_All'.
+  template  bool match(const T &Node) {
+internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
+
+if (Matcher->matches(DynTypedNode::create(Node), Finder,
+ &RecursiveBuilder)) {
+  ResultBindings.addMatch(RecursiveBuilder);
+  Matches = true;
+  if (Bind != internal::ASTMatchFinder::BK_All)
+return false; // Abort as soon as a match is found.
+}
+return true;
+  }
+
+  const internal::DynTypedMatcher *const Matcher;
+  internal::ASTMatchFinder *const Finder;
+  internal::BoundNodesTreeBuilder *const Builder;
+  internal::BoundNodesTreeBuilder ResultBindings;
+  const internal::ASTMatchFinder::BindKind Bind;
+  bool Matches;
+};
+
+AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) 
{
+  MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
+ Builder, ASTMatchFinder::BK_All);
+  return Visitor.findMatch(DynTypedNode::create(Node));
+}
+} // namespace clang::ast_matchers
+
 namespace {
 // Because the analysis revolves around varia

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

2023-01-06 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13402
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())

Why is this restricted to integers? It looks like `CheckImplicitConversion` 
also handles other types? Does calling it unconditionally cause any issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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


[PATCH] D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty

2023-01-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

LGTM, thanks for fixing!

Please be sure to have your commit message have `Fixes #59517` instead of a 
link to the issue, as this will close the bug upon merging.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp:586-590
+  {
+std::vector v;
+return std::empty(v);
+// no-warning
+  }

Please add additional cases similar to what's above (e.g. a case with 
`absl::empty`, an ADL-invoked case, etc.).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141107

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


[PATCH] D136566: [clang] Instantiate concepts with sugared template arguments

2023-01-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a subscriber: aaron.ballman.
erichkeane added a comment.

See @aaron.ballman 's comment here: 
https://github.com/llvm/llvm-project/issues/59271

Since the release branch is pending, if we don't have a solution or a revert by 
the 13th, Aaron or I will revert this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136566

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


[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

2023-01-06 Thread Fahad Nayyar via Phabricator via cfe-commits
fahadnayyar added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13402
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())

arichardson wrote:
> Why is this restricted to integers? It looks like `CheckImplicitConversion` 
> also handles other types? Does calling it unconditionally cause any issues?
@arichardson since `AnalyzeCompoundAssignment` already does some custom checks 
for certain data types like floats, I added the change only for integers as I 
am trying to fix the behaviour only for integral operands in this patch. 
Otherwise we see more than one (required) warning for floats operands and also 
for shift assign operator.

This patch targets only integral changes like test 3-5 in `conversion-64-32.c`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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


[PATCH] D131858: [clang] Track the templated entity in type substitution.

2023-01-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Note that the Clang 16 branch is coming up approximately on January 24, and 
this needs to be reverted or have the issue reported by @steven_wu fixed by 
then.  @mizvekov : If you or someone else don't have a solution/revert to this 
by January 13th, @aaron.ballman and I will begin the process to revert this 
ourselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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


[PATCH] D136565: [clang] Instantiate alias templates with sugar

2023-01-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Note that the Clang 16 branch is coming up approximately on January 24, and 
this needs to be reverted or perf-regression fixed by then.  @mizvekov : If you 
or someone else don't have a solution/revert to this by January 13th, 
@aaron.ballman and I will begin the process to revert this ourselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136565

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


[clang] 50d4a1f - [-Wunsafe-buffer-usage] Safe-buffers re-architecture to introduce Fixable gadgets

2023-01-06 Thread via cfe-commits

Author: MalavikaSamak
Date: 2023-01-06T11:45:23-08:00
New Revision: 50d4a1f70e111cd41b1a94d95fd06b5691aa2643

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

LOG: [-Wunsafe-buffer-usage] Safe-buffers re-architecture to introduce Fixable 
gadgets

Re-architecture of safe-buffers gadgets to re-classify them as warning and 
fixable
gadgets. The warning gadgets identify unsafe operations on buffer variables and
emit suitable warnings. While the fixable gadgets consider all operations on
variables identified by the warning gadgets and emit necessary fixits.

Differential Revision: https://reviews.llvm.org/D140062?id=486625

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index f24e925966094..d10d95e5b1ba7 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -14,22 +14,22 @@
 
 /// Unsafe gadgets correspond to unsafe code patterns that warrant
 /// an immediate warning.
-#ifndef UNSAFE_GADGET
-#define UNSAFE_GADGET(name) GADGET(name)
+#ifndef WARNING_GADGET
+#define WARNING_GADGET(name) GADGET(name)
 #endif
 
 /// Safe gadgets correspond to code patterns that aren't unsafe but need to be
 /// properly recognized in order to emit correct warnings and fixes over unsafe
 /// gadgets.
-#ifndef SAFE_GADGET
-#define SAFE_GADGET(name) GADGET(name)
+#ifndef FIXABLE_GADGET
+#define FIXABLE_GADGET(name) GADGET(name)
 #endif
 
-UNSAFE_GADGET(Increment)
-UNSAFE_GADGET(Decrement)
-UNSAFE_GADGET(ArraySubscript)
-UNSAFE_GADGET(PointerArithmetic)
+WARNING_GADGET(Increment)
+WARNING_GADGET(Decrement)
+WARNING_GADGET(ArraySubscript)
+WARNING_GADGET(PointerArithmetic)
 
-#undef SAFE_GADGET
-#undef UNSAFE_GADGET
+#undef FIXABLE_GADGET
+#undef WARNING_GADGET
 #undef GADGET

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 80a54c3ad38b7..d5423d81ee309 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -137,9 +137,9 @@ namespace {
 /// rigid AST structure that constitutes an operation on a pointer-type object.
 /// Discovery of a gadget in the code corresponds to claiming that we 
understand
 /// what this part of code is doing well enough to potentially improve it.
-/// Gadgets can be unsafe (immediately deserving a warning) or safe (not
-/// deserving a warning per se, but affecting our decision-making process
-/// nonetheless).
+/// Gadgets can be warning (immediately deserving a warning) or fixable (not
+/// always deserving a warning per se, but requires our attention to identify
+/// it warrants a fixit).
 class Gadget {
 public:
   enum class Kind {
@@ -156,7 +156,7 @@ class Gadget {
 
   Kind getKind() const { return K; }
 
-  virtual bool isSafe() const = 0;
+  virtual bool isWarningGadget() const = 0;
   virtual const Stmt *getBaseStmt() const = 0;
 
   /// Returns the list of pointer-type variables on which this gadget performs
@@ -164,53 +164,55 @@ class Gadget {
   /// of all DeclRefExprs in the gadget's AST!
   virtual DeclUseList getClaimedVarUseSites() const = 0;
 
-  /// Returns a fixit that would fix the current gadget according to
-  /// the current strategy. Returns None if the fix cannot be produced;
-  /// returns an empty list if no fixes are necessary.
-  virtual std::optional getFixits(const Strategy &) const {
-return std::nullopt;
-  }
-
   virtual ~Gadget() = default;
 
 private:
   Kind K;
 };
 
-using GadgetList = std::vector>;
 
-/// Unsafe gadgets correspond to unsafe code patterns that warrants
+/// Warning gadgets correspond to unsafe code patterns that warrants
 /// an immediate warning.
-class UnsafeGadget : public Gadget {
+class WarningGadget : public Gadget {
 public:
-  UnsafeGadget(Kind K) : Gadget(K) {}
+  WarningGadget(Kind K) : Gadget(K) {}
 
-  static bool classof(const Gadget *G) { return G->isSafe(); }
-  bool isSafe() const final { return false; }
+  static bool classof(const Gadget *G) { return G->isWarningGadget(); }
+  bool isWarningGadget() const final { return true; }
 };
 
-/// Safe gadgets correspond to code patterns that aren't unsafe but need to be
-/// properly recognized in order to emit correct warnings and fixes over unsafe
-/// gadgets. For example, if a raw pointer-type variable is replaced by
-/// a safe C++ container, every use of such variable may need to be
+/// Fixable gadgets correspond to code patterns that aren't always unsafe but 
need to be
+/// properly recognized in order 

[PATCH] D140062: [-Wunsafe-buffer-usage] Safe-buffers re-architecture to introduce Fixable gadgets

2023-01-06 Thread Malavika Samak 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 rG50d4a1f70e11: [-Wunsafe-buffer-usage] Safe-buffers 
re-architecture to introduce Fixable… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D140062?vs=486625&id=486952#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140062

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
  clang/lib/Analysis/UnsafeBufferUsage.cpp

Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -137,9 +137,9 @@
 /// rigid AST structure that constitutes an operation on a pointer-type object.
 /// Discovery of a gadget in the code corresponds to claiming that we understand
 /// what this part of code is doing well enough to potentially improve it.
-/// Gadgets can be unsafe (immediately deserving a warning) or safe (not
-/// deserving a warning per se, but affecting our decision-making process
-/// nonetheless).
+/// Gadgets can be warning (immediately deserving a warning) or fixable (not
+/// always deserving a warning per se, but requires our attention to identify
+/// it warrants a fixit).
 class Gadget {
 public:
   enum class Kind {
@@ -156,7 +156,7 @@
 
   Kind getKind() const { return K; }
 
-  virtual bool isSafe() const = 0;
+  virtual bool isWarningGadget() const = 0;
   virtual const Stmt *getBaseStmt() const = 0;
 
   /// Returns the list of pointer-type variables on which this gadget performs
@@ -164,53 +164,55 @@
   /// of all DeclRefExprs in the gadget's AST!
   virtual DeclUseList getClaimedVarUseSites() const = 0;
 
-  /// Returns a fixit that would fix the current gadget according to
-  /// the current strategy. Returns None if the fix cannot be produced;
-  /// returns an empty list if no fixes are necessary.
-  virtual std::optional getFixits(const Strategy &) const {
-return std::nullopt;
-  }
-
   virtual ~Gadget() = default;
 
 private:
   Kind K;
 };
 
-using GadgetList = std::vector>;
 
-/// Unsafe gadgets correspond to unsafe code patterns that warrants
+/// Warning gadgets correspond to unsafe code patterns that warrants
 /// an immediate warning.
-class UnsafeGadget : public Gadget {
+class WarningGadget : public Gadget {
 public:
-  UnsafeGadget(Kind K) : Gadget(K) {}
+  WarningGadget(Kind K) : Gadget(K) {}
 
-  static bool classof(const Gadget *G) { return G->isSafe(); }
-  bool isSafe() const final { return false; }
+  static bool classof(const Gadget *G) { return G->isWarningGadget(); }
+  bool isWarningGadget() const final { return true; }
 };
 
-/// Safe gadgets correspond to code patterns that aren't unsafe but need to be
-/// properly recognized in order to emit correct warnings and fixes over unsafe
-/// gadgets. For example, if a raw pointer-type variable is replaced by
-/// a safe C++ container, every use of such variable may need to be
+/// Fixable gadgets correspond to code patterns that aren't always unsafe but need to be
+/// properly recognized in order to emit fixes. For example, if a raw pointer-type
+/// variable is replaced by a safe C++ container, every use of such variable must be
 /// carefully considered and possibly updated.
-class SafeGadget : public Gadget {
+class FixableGadget : public Gadget {
 public:
-  SafeGadget(Kind K) : Gadget(K) {}
+  FixableGadget(Kind K) : Gadget(K) {}
+
+  static bool classof(const Gadget *G) { return !G->isWarningGadget(); }
+  bool isWarningGadget() const final { return false; }
+
+  /// Returns a fixit that would fix the current gadget according to
+  /// the current strategy. Returns None if the fix cannot be produced;
+  /// returns an empty list if no fixes are necessary.
+  virtual Optional getFixits(const Strategy &) const {
+return None;
+  }
 
-  static bool classof(const Gadget *G) { return !G->isSafe(); }
-  bool isSafe() const final { return true; }
 };
 
+using FixableGadgetList = std::vector>;
+using WarningGadgetList = std::vector>;
+
 /// An increment of a pointer-type value is unsafe as it may run the pointer
 /// out of bounds.
-class IncrementGadget : public UnsafeGadget {
+class IncrementGadget : public WarningGadget {
   static constexpr const char *const OpTag = "op";
   const UnaryOperator *Op;
 
 public:
   IncrementGadget(const MatchFinder::MatchResult &Result)
-  : UnsafeGadget(Kind::Increment),
+  : WarningGadget(Kind::Increment),
 Op(Result.Nodes.getNodeAs(OpTag)) {}
 
   static bool classof(const Gadget *G) {
@@ -239,13 +241,13 @@
 
 /// A decrement of a pointer-type value is unsafe as it may run the pointer
 /// out of bounds.
-class DecrementGadget : public UnsafeGadget {
+class Decr

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

2023-01-06 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment.

> Sorry for missing these. I think I addressed all of them now.

Thanks, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

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


[clang] 87fe37d - [-Wunsafe-buffer-usage] Changing the use of None with std::nullopt to address a warning.

2023-01-06 Thread via cfe-commits

Author: MalavikaSamak
Date: 2023-01-06T12:31:14-08:00
New Revision: 87fe37d9f8b668f537006a2bf6921cf2ad3fb2a2

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

LOG: [-Wunsafe-buffer-usage] Changing the use of None with std::nullopt to 
address a warning.

Added: 


Modified: 
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d5423d81ee30..d941cf5fe74f 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -196,7 +196,7 @@ class FixableGadget : public Gadget {
   /// the current strategy. Returns None if the fix cannot be produced;
   /// returns an empty list if no fixes are necessary.
   virtual Optional getFixits(const Strategy &) const {
-return None;
+return std::nullopt;
   }
 
 };



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


[clang] 6d140b9 - [Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations

2023-01-06 Thread via cfe-commits

Author: ziqingluo-90
Date: 2023-01-06T12:32:35-08:00
New Revision: 6d140b952805bd9277fba666520ce46c19f2c637

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

LOG: [Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that 
skips callable declarations

The original patch does include a `new` statement without a matching
`delete`, causing Sanitizer warnings in
https://lab.llvm.org/buildbot/#/builders/5/builds/30522/steps/13/logs/stdio.

This commit is a fix to it.

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

Added: 


Modified: 
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d941cf5fe74f..f03fe7505ad4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -26,7 +26,7 @@ class MatchDescendantVisitor
   // Creates an AST visitor that matches `Matcher` on all
   // descendants of a given node "n" except for the ones
   // belonging to a 
diff erent callable of "n".
-  MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
+  MatchDescendantVisitor(const internal::DynTypedMatcher &Matcher,
  internal::ASTMatchFinder *Finder,
  internal::BoundNodesTreeBuilder *Builder,
  internal::ASTMatchFinder::BindKind Bind)
@@ -89,7 +89,7 @@ class MatchDescendantVisitor
   template  bool match(const T &Node) {
 internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
 
-if (Matcher->matches(DynTypedNode::create(Node), Finder,
+if (Matcher.matches(DynTypedNode::create(Node), Finder,
  &RecursiveBuilder)) {
   ResultBindings.addMatch(RecursiveBuilder);
   Matches = true;
@@ -99,7 +99,7 @@ class MatchDescendantVisitor
 return true;
   }
 
-  const internal::DynTypedMatcher *const Matcher;
+  const internal::DynTypedMatcher & Matcher;
   internal::ASTMatchFinder *const Finder;
   internal::BoundNodesTreeBuilder *const Builder;
   internal::BoundNodesTreeBuilder ResultBindings;
@@ -108,7 +108,7 @@ class MatchDescendantVisitor
 };
 
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) 
{
-  MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
+  MatchDescendantVisitor Visitor(DynTypedMatcher(innerMatcher), Finder,
  Builder, ASTMatchFinder::BK_All);
   return Visitor.findMatch(DynTypedNode::create(Node));
 }



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


[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

"default capture"?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

carlosgalvezp wrote:
> "default capture"?
I find the check name a bit unintuitive. If you are up for a rename (you can 
use `rename_check.py`), I would consider renaming to something like 
`cppcoreguidelines-avoid-default-capture-when-capturing-this`

Like, what should be avoided is not "capturing this", it's using a default 
capture.

Would be good to get other reviewers opinion before spending time on renaming.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

carlosgalvezp wrote:
> carlosgalvezp wrote:
> > "default capture"?
> I find the check name a bit unintuitive. If you are up for a rename (you can 
> use `rename_check.py`), I would consider renaming to something like 
> `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> 
> Like, what should be avoided is not "capturing this", it's using a default 
> capture.
> 
> Would be good to get other reviewers opinion before spending time on renaming.
Maybe put it within quotes so clarify it's a C++ keyword? Either backticks 
`this` or single quotes 'this' would work I think, unless we have some other 
convention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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


[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:1
+// RUN: %check_clang_tidy -std=c++11-or-later %s 
cppcoreguidelines-avoid-capture-this-with-capture-default %t
+

I believe this is the default, does it make sense to remove it or do we need to 
be explicit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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


[PATCH] D140584: [Clang] Refactor "Designators" into a unified implementation [NFC]

2023-01-06 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment.

BTW, the naming of the enums sucks. Feel free to offer alternatives. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140584

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


[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, MaskRay, ggeorgakoudis, 
tianshilei1992.
Herald added subscribers: StephenFan, guansong, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

JIT support for OpenMP offloading was introduced in D139287 
. This patch
adds a simple flag that enables this mode. It simply requires enabling
`-foffload-lto` mode and `--embed-bitcode` in the linker wrapper. This
option implies LTO if it is not enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141158

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-jit.c


Index: clang/test/Driver/openmp-offload-jit.c
===
--- /dev/null
+++ clang/test/Driver/openmp-offload-jit.c
@@ -0,0 +1,42 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check that we enable LTO-mode properly with '-fopenmp-target-jit' and that 
it
+// still enabled LTO-mode if `-fno-offload-lto` is on.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases 
-fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases 
-fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -foffload-lto 
-fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases 
-fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases 
-fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -foffload-lto -fopenmp-target-jit 
%s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases 
-fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fno-offload-lto 
-fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+//
+//  PHASES-JIT: 0: input, "[[INPUT:.+]]", c, (host-openmp)
+// PHASES-JIT-NEXT: 1: preprocessor, {0}, cpp-output, (host-openmp)
+// PHASES-JIT-NEXT: 2: compiler, {1}, ir, (host-openmp)
+// PHASES-JIT-NEXT: 3: input, "[[INPUT]]", c, (device-openmp)
+// PHASES-JIT-NEXT: 4: preprocessor, {3}, cpp-output, (device-openmp)
+// PHASES-JIT-NEXT: 5: compiler, {4}, ir, (device-openmp)
+// PHASES-JIT-NEXT: 6: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, 
"device-openmp ([[TARGET:.+]])" {5}, ir
+// PHASES-JIT-NEXT: 7: backend, {6}, lto-bc, (device-openmp)
+// PHASES-JIT-NEXT: 8: offload, "device-openmp ([[TARGET]])" {7}, lto-bc
+// PHASES-JIT-NEXT: 9: clang-offload-packager, {8}, image, (device-openmp)
+// PHASES-JIT-NEXT: 10: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, 
"device-openmp (x86_64-unknown-linux-gnu)" {9}, ir
+// PHASES-JIT-NEXT: 11: backend, {10}, assembler, (host-openmp)
+// PHASES-JIT-NEXT: 12: assembler, {11}, object, (host-openmp)
+// PHASES-JIT-NEXT: 13: clang-linker-wrapper, {12}, image, (host-openmp)
+
+// Check that we add the `--embed-bitcode` flag to the linker wrapper.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LINKER %s
+// LINKER: clang-linker-wrapper"{{.*}}"--embed-bitcode"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8428,6 +8428,10 @@
   }
   Args.ClaimAllArgs(options::OPT_Xoffload_linker);
 
+  // Embed bitcode instead of an object in JIT mode.
+  if (const Arg *A = Args.getLastArg(options::OPT_fopenmp_target_jit))
+CmdArgs.push_back("--embed-bitcode");
+
   // Forward `-mllvm` arguments to the LLVM invocations if present.
   for (Arg *A : Args.filtered(options::OPT_mllvm)) {
 CmdArgs.push_back("-mllvm");
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -721,6 +721,13 @@
 
   OffloadLTOMode = parseLTOMode(*this, Args, options::OPT_foffload_lto_EQ,
 options::OPT_fno_offload_lto);
+
+  // Enable `-foffload-lto=full` if `-fopenmp-target-jit` is on.
+  if (OffloadLTOMode == LTOK_None || OffloadLTOMode == LTOK_Unknown)
+OffloadLTOMode = Args.hasFlag(options::OPT_fopenmp_target_jit,
+  options::OPT_fno_openmp_target_jit, false)

[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:730
+ ? LTOK_Full
+ : OffloadLTOMode;
 }

Should we overwrite unconditionally or warn/error if the combination is 
nonsensical?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141158

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


[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

2023-01-06 Thread Christian Kandeler via Phabricator via cfe-commits
ckandeler added a comment.

In D139926#4030782 , @nridge wrote:

> It's true that there is an ambiguity between `<` and `>` as operators, vs. 
> template arg/param list delimiters, but, at least in terms of user 
> understanding of code, my sense is that the highlighting of the **preceding** 
> token should be sufficient to disambiguate -- i.e. it would be some sort of 
> type name in the template case, vs. a variable / literal / punctuation ending 
> an expression in the operator case.

We used to do this sort of heuristic in our old libclang-based implementation, 
and it turned out to be rather messy, with a surprising amount of exceptions 
having to be added.

>> This is needed for clients that would like to visualize matching opening and 
>> closing angle brackets, which can be valuable in non-trivial template 
>> declarations or instantiations.
>
> For this use case, could an editor make use of the recently added operator 
> tokens (https://reviews.llvm.org/D136594) instead, inferring that a `<` token 
> which does not have an `operator` semantic token is a template delimiter?

I have a suspicion that this will lead to false positives for invalid code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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


[clang] 42b1d08 - Revert "[Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"

2023-01-06 Thread via cfe-commits

Author: ziqingluo-90
Date: 2023-01-06T13:37:13-08:00
New Revision: 42b1d08b955d6cfa0b78ceca62605bddc905461a

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

LOG: Revert "[Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher 
that skips callable declarations"

This reverts commit 6d140b952805bd9277fba666520ce46c19f2c637.

This commit may causes `test/SemaCXX/warn-unsafe-buffer-usage.cpp` failure.

Added: 


Modified: 
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index f03fe7505ad4a..d941cf5fe74f5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -26,7 +26,7 @@ class MatchDescendantVisitor
   // Creates an AST visitor that matches `Matcher` on all
   // descendants of a given node "n" except for the ones
   // belonging to a 
diff erent callable of "n".
-  MatchDescendantVisitor(const internal::DynTypedMatcher &Matcher,
+  MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
  internal::ASTMatchFinder *Finder,
  internal::BoundNodesTreeBuilder *Builder,
  internal::ASTMatchFinder::BindKind Bind)
@@ -89,7 +89,7 @@ class MatchDescendantVisitor
   template  bool match(const T &Node) {
 internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
 
-if (Matcher.matches(DynTypedNode::create(Node), Finder,
+if (Matcher->matches(DynTypedNode::create(Node), Finder,
  &RecursiveBuilder)) {
   ResultBindings.addMatch(RecursiveBuilder);
   Matches = true;
@@ -99,7 +99,7 @@ class MatchDescendantVisitor
 return true;
   }
 
-  const internal::DynTypedMatcher & Matcher;
+  const internal::DynTypedMatcher *const Matcher;
   internal::ASTMatchFinder *const Finder;
   internal::BoundNodesTreeBuilder *const Builder;
   internal::BoundNodesTreeBuilder ResultBindings;
@@ -108,7 +108,7 @@ class MatchDescendantVisitor
 };
 
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) 
{
-  MatchDescendantVisitor Visitor(DynTypedMatcher(innerMatcher), Finder,
+  MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
  Builder, ASTMatchFinder::BK_All);
   return Visitor.findMatch(DynTypedNode::create(Node));
 }



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


[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:730
+ ? LTOK_Full
+ : OffloadLTOMode;
 }

jdoerfert wrote:
> Should we overwrite unconditionally or warn/error if the combination is 
> nonsensical?
Yeah, we might need something like that. Like if the user specifies `thin` then 
the monolithic linking won't work. Probably will need a check for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141158

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


[PATCH] D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address

2023-01-06 Thread Adrian Dole via Phabricator via cfe-commits
adriandole updated this revision to Diff 486980.
adriandole added a comment.

Add test and release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139570

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/SemaCXX/return-stack-addr-2.cpp


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 
-Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,7 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 -Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,7 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140817: [Driver] move NetBSD header search path management to the driver

2023-01-06 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka added a comment.

Hi @brad,
thank you for fixing the broken tests.

There is still a problem with `netbsd.c` test: 
https://lab.llvm.org/buildbot/#/builders/60/builds/10135/steps/9/logs/FAIL__Clang__netbsd_c
Here:

  // DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
  // DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" 
"{{.*}}/usr/include"

The first's `{{^}}` breaks the test. It can be removed from the first line or 
from the both of them.
The tests are getting passed without this part on the win-x-cross builders. 
They should work on Linux/BSD hosts with these changes.
Would you check and update the tests accordingly?

The following changes are working on my cross builders

  // DRIVER-PASS-INCLUDES-SAME: "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
  // DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" 
"{{.*}}/usr/include"

and

  // DRIVER-PASS-INCLUDES-SAME: "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
  // DRIVER-PASS-INCLUDES-SAME: "-internal-externc-isystem" "{{.*}}/usr/include"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140817

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


[PATCH] D140694: [clang][dataflow] Only model struct fields that are used in the function being analyzed.

2023-01-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.

I think in these cases you can recommit without waiting for a new round of 
reviews, but I hit accept just in case :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140694

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


[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 486991.
jhuber6 added a comment.

Adding an error print and some tests for it. We error if the user specified any 
`-foffload-lto` optoins that don't result in full LT.O


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141158

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-jit.c

Index: clang/test/Driver/openmp-offload-jit.c
===
--- /dev/null
+++ clang/test/Driver/openmp-offload-jit.c
@@ -0,0 +1,54 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check that we enable LTO-mode properly with '-fopenmp-target-jit' and that it
+// still enabled LTO-mode if `-fno-offload-lto` is on.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -foffload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -foffload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fno-offload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+//
+//  PHASES-JIT: 0: input, "[[INPUT:.+]]", c, (host-openmp)
+// PHASES-JIT-NEXT: 1: preprocessor, {0}, cpp-output, (host-openmp)
+// PHASES-JIT-NEXT: 2: compiler, {1}, ir, (host-openmp)
+// PHASES-JIT-NEXT: 3: input, "[[INPUT]]", c, (device-openmp)
+// PHASES-JIT-NEXT: 4: preprocessor, {3}, cpp-output, (device-openmp)
+// PHASES-JIT-NEXT: 5: compiler, {4}, ir, (device-openmp)
+// PHASES-JIT-NEXT: 6: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp ([[TARGET:.+]])" {5}, ir
+// PHASES-JIT-NEXT: 7: backend, {6}, lto-bc, (device-openmp)
+// PHASES-JIT-NEXT: 8: offload, "device-openmp ([[TARGET]])" {7}, lto-bc
+// PHASES-JIT-NEXT: 9: clang-offload-packager, {8}, image, (device-openmp)
+// PHASES-JIT-NEXT: 10: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (x86_64-unknown-linux-gnu)" {9}, ir
+// PHASES-JIT-NEXT: 11: backend, {10}, assembler, (host-openmp)
+// PHASES-JIT-NEXT: 12: assembler, {11}, object, (host-openmp)
+// PHASES-JIT-NEXT: 13: clang-linker-wrapper, {12}, image, (host-openmp)
+
+// Check that we add the `--embed-bitcode` flag to the linker wrapper.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LINKER %s
+// LINKER: clang-linker-wrapper"{{.*}}"--embed-bitcode"
+
+// Check for incompatible combinations
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fno-offload-lto \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=NO-LTO %s
+// NO-LTO: error: The combination of '-fno-offload-lto' and '-fopenmp-target-jit' is incompatible
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -foffload-lto=thin \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN-LTO %s
+// THIN-LTO: error: The combination of '-foffload-lto=' and '-fopenmp-target-jit' is incompatible
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8428,6 +8428,10 @@
   }
   Args.ClaimAllArgs(options::OPT_Xoffload_linker);
 
+  // Embed bitcode instead of an object in JIT mode.
+  if (const Arg *A = Args.getLastArg(options::OPT_fopenmp_target_jit))
+CmdArgs.push_back("--embed-bitcode");
+
   // Forward `-mllvm` arguments to the LLVM invocations if present.
   for (Arg *A : Args.filtered(options::OPT_mllvm)) {
 CmdArgs.push_back("-mllvm");
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -721,6 +721,17 @@
 
   OffloadLTOMode = parseLTOMode(*this, Args, options::OPT_foffload_lto_E

[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141158

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


[clang] 5be422b - [Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations

2023-01-06 Thread via cfe-commits

Author: ziqingluo-90
Date: 2023-01-06T14:38:12-08:00
New Revision: 5be422b49288d7b9394312ad5ccba1fef42116e6

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

LOG: [Fix][-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that 
skips callable declarations

The original patch does include a `new` statement without a matching
`delete`, causing Sanitizer warnings in
https://lab.llvm.org/buildbot/#/builders/5/builds/30522/steps/13/logs/stdio.

This commit is a fix to it.

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

Added: 


Modified: 
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d941cf5fe74f5..7dd9e918b0c2e 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -108,8 +108,9 @@ class MatchDescendantVisitor
 };
 
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) 
{
-  MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
- Builder, ASTMatchFinder::BK_All);
+  const DynTypedMatcher &DTM = static_cast(innerMatcher);
+  
+  MatchDescendantVisitor Visitor(&DTM, Finder, Builder, 
ASTMatchFinder::BK_All);  
   return Visitor.findMatch(DynTypedNode::create(Node));
 }
 } // namespace clang::ast_matchers



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


[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-06 Thread Arthur Laurent via Phabricator via cfe-commits
Arthapz added a comment.

Hi, just wanted to say that i added support of these patch to XMake and it work 
pretty well :)

  > xmake b -vD 
  
  [  0%]: generating.module.deps src/main.cpp
  checking for clang-scan-deps ... /usr/bin/clang-scan-deps
  checking for flags (-std=c++20) ... ok
  > clang "-std=c++20"
  checking for flags (-fmodules) ... ok
  > clang "-fmodules"
  /usr/bin/clang-scan-deps --format=p1689 -- /usr/bin/clang -x c++ -c 
src/main.cpp -o build/.objs/dependence/linux/x86_64/release/src/main.cpp.o 
-Qunused-arguments -m64 -std=c++20 -fmodules -fno-implicit-module-maps
  [  0%]: generating.module.deps src/foo.mpp
  /usr/bin/clang-scan-deps --format=p1689 -- /usr/bin/clang -x c++ -c 
src/foo.mpp -o build/.objs/dependence/linux/x86_64/release/src/foo.mpp.o 
-Qunused-arguments -m64 -std=c++20 -fmodules -fno-implicit-module-maps
  [  0%]: generating.module.deps src/zoo.mpp
  /usr/bin/clang-scan-deps --format=p1689 -- /usr/bin/clang -x c++ -c 
src/zoo.mpp -o build/.objs/dependence/linux/x86_64/release/src/zoo.mpp.o 
-Qunused-arguments -m64 -std=c++20 -fmodules -fno-implicit-module-maps
  [  0%]: generating.module.deps src/cat.mpp
  /usr/bin/clang-scan-deps --format=p1689 -- /usr/bin/clang -x c++ -c 
src/cat.mpp -o build/.objs/dependence/linux/x86_64/release/src/cat.mpp.o 
-Qunused-arguments -m64 -std=c++20 -fmodules -fno-implicit-module-maps
  [  0%]: generating.module.deps src/bar.mpp
  /usr/bin/clang-scan-deps --format=p1689 -- /usr/bin/clang -x c++ -c 
src/bar.mpp -o build/.objs/dependence/linux/x86_64/release/src/bar.mpp.o 
-Qunused-arguments -m64 -std=c++20 -fmodules -fno-implicit-module-maps
  checking for flags (clang_modules_cache_path) ... ok
  > clang "-fmodules-cache-path=/dev/shm/.xmake1000/230107"
  [ 10%]: compiling.module.release zoo
  /usr/bin/clang -c -x c++-module --precompile -Qunused-arguments -m64 
-std=c++20 -fmodules -fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/zoo.pcm 
src/zoo.mpp
  /usr/bin/clang -c -Qunused-arguments -m64 -std=c++20 -fmodules 
-fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 -o build/.objs/dependence/linux/x86_64/release/src/zoo.mpp.o 
build/.gens/dependence/linux/x86_64/release/rules/modules/cache/zoo.pcm
  checking for flags (-MMD -MF) ... ok
  > clang "-MMD" "-MF" "/dev/null"
  [ 10%]: compiling.module.release cat
  /usr/bin/clang -c -x c++-module --precompile -Qunused-arguments -m64 
-std=c++20 -fmodules -fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/cat.pcm 
src/cat.mpp
  /usr/bin/clang -c -Qunused-arguments -m64 -std=c++20 -fmodules 
-fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 -o build/.objs/dependence/linux/x86_64/release/src/cat.mpp.o 
build/.gens/dependence/linux/x86_64/release/rules/modules/cache/cat.pcm
  checking for flags (-fdiagnostics-color=always) ... ok
  > clang "-fdiagnostics-color=always"
  checking for flags (clang_module_file) ... ok
  > clang 
"-fmodule-file=/dev/shm/.xmake1000/230107/_11EA40624C464D10876A6DA3D0E41320.pcm"
  [ 30%]: compiling.module.release bar
  /usr/bin/clang -c -x c++-module --precompile -Qunused-arguments -m64 
-std=c++20 -fmodules -fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 
-fmodule-file=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/zoo.pcm
 -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/bar.pcm 
src/bar.mpp
  /usr/bin/clang -c -Qunused-arguments -m64 -std=c++20 -fmodules 
-fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 
-fmodule-file=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/zoo.pcm
 -o build/.objs/dependence/linux/x86_64/release/src/bar.mpp.o 
build/.gens/dependence/linux/x86_64/release/rules/modules/cache/bar.pcm
  [ 40%]: compiling.module.release foo
  /usr/bin/clang -c -x c++-module --precompile -Qunused-arguments -m64 
-std=c++20 -fmodules -fno-implicit-module-maps 
-fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache
 
-fmodule-file=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/bar.pcm
 
-fmodule-file=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/zoo.pcm
 
-fmodule-file=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/cat.pcm
 -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/foo.pcm 
src/foo.mpp
  /usr/bin/clang -c -Qunused-arguments -m64 -std=c++20 -fmodules 
-fno-implicit-module-maps 
-fmodules-cac

[PATCH] D140925: [CMake] Use Clang to infer the target triple

2023-01-06 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: runtimes/CMakeLists.txt:168
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_TRIPLE} 
-print-target-triple)
+  execute_process(COMMAND ${CXX_TARGET_TRIPLE}

tamas wrote:
> I think there is one downside to this: the `normalize` function will not, in 
> fact, normalize alternative spellings for equivalent architectures (and 
> likely other components, I haven't checked that):
> ```
> + clang -print-target-triple -target amd64-unknown-linux-musl
> amd64-unknown-linux-musl
> + clang -print-target-triple -target x86_64-unknown-linux-musl
> x86_64-unknown-linux-musl
> ```
> So the issue where the distribution build can install runtimes in paths that 
> won't be found by the driver still remains 
> (https://github.com/llvm/llvm-project/issues/57781). One way to fix this 
> could be adding a new switch that does a more opinionated normalization (for 
> example, always picks `x86_64` in the above). Sort of a reverse for 
> `Triple::parseArch` etc. where it always returns one certain spelling when 
> there are multiple common alternatives. Alternatively, `normalize` could be 
> changed to do this, but that might subtle breaking consequences for users 
> which are hard to foresee.
I think this is an orthogonal issue to the one that this change is trying to 
address. Clang currently uses normalized triples to for its include directories 
(libc++ headers) and runtime libraries. This change ensures that the triple 
spelling used by CMake and Clang matches, but it doesn't change the 
normalization logic.

From Clang's perspective, `amd64-unknown-linux-musl` and 
`x86_64-unknown-linux-musl` are two different triples and so it'd use different 
search directories. We could consider changing the normalization rules to 
normalize these to the same triple, but that should be done separately (and 
would likely require further discussion to understand the potential 
consequences of such a change).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140925

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


[PATCH] D140584: [Clang] Refactor "Designators" into a unified implementation [NFC]

2023-01-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/AST/Designator.h:88
+  /// An array designator, e.g., "[42] = 0" and "[42 ... 50] = 1".
+  template  struct ArrayDesignatorInfo {
+/// Location of the first and last index expression within the designated

Can we move the templating out from here to the whole `Designator` and 
`Designation` classes? It shouldn't be possible to mix the two kinds in the 
same `Designation`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140584

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


[PATCH] D141172: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

2023-01-06 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
samitolvanen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

When -fpatchable-function-entry is used to emit prefix nops
before functions, KCFI assumes all indirectly called functions
have the same number of prefix nops, because the nops are emitted
between the KCFI type hash and the function entry. However, as
patchable-function-prefix is a function attribute set by Clang,
functions later synthesized by LLVM don't inherit this attribute
and end up not having prefix nops. One of these functions
is asan.module_ctor, which the Linux kernel ends up calling
indirectly when KASAN is enabled.

In order to avoid tripping KCFI, save the expected prefix offset
to a module flag, and use it when we're setting KCFI type for the
relevant synthesized functions.

Link: https://github.com/ClangBuiltLinux/linux/issues/1742


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141172

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/kcfi.c
  llvm/lib/Transforms/Utils/ModuleUtils.cpp
  llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll


Index: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when 
kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr 
@asan.module_ctor }
+
+; CHECK: define internal void @asan.module_ctor()
+; CHECK-SAME: #[[#ATTR:]]
+; CHECK-SAME: !kcfi_type
+
+; CHECK: attributes #[[#ATTR]] = { {{.*}} "patchable-function-prefix"="3" }
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-offset", i32 3}
Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,6 +161,14 @@
   MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
Type::getInt32Ty(Ctx),
static_cast(xxHash64(MangledType));
+  // If the module was compiled with -fpatchable-function-entry, ensure
+  // we use the same patchable-function-prefix.
+  if (auto *MD = mdconst::extract_or_null(
+  M.getModuleFlag("kcfi-offset"))) {
+unsigned Offset = MD->getZExtValue();
+if (Offset)
+  F.addFnAttr("patchable-function-prefix", std::to_string(Offset));
+  }
 }
 
 FunctionCallee
Index: clang/test/CodeGen/kcfi.c
===
--- clang/test/CodeGen/kcfi.c
+++ clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi 
-fpatchable-function-entry-offset=3 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OFFSET
 #if !__has_feature(kcfi)
 #error Missing kcfi?
 #endif
@@ -54,5 +55,6 @@
 }
 
 // CHECK-DAG: ![[#]] = !{i32 4, !"kcfi", i32 1}
+// OFFSET-DAG: ![[#]] = !{i32 4, !"kcfi-offset", i32 3}
 // CHECK-DAG: ![[#TYPE]] = !{i32 [[#HASH]]}
 // CHECK-DAG: ![[#TYPE2]] = !{i32 [[#%d,HASH2:]]}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -758,8 +758,14 @@
   CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
-  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
 getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
+// KCFI assumes patchable-function-prefix is the same for all indirectly
+// called functions. Store the expected offset for code generation.
+if (CodeGenOpts.PatchableFunctionEntryOffset)
+  getModule().addModuleFlag(llvm::Module::Override, "kcfi-offset",
+CodeGenOpts.PatchableFunctionEntryOffset);
+  }
 
   if (CodeGenOpts.CFProtectionReturn &&
   Target.checkCFProtectionReturnSupported(getDiags())) {


Index: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.modul

[PATCH] D141172: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

2023-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Transforms/Utils/ModuleUtils.cpp:169
+unsigned Offset = MD->getZExtValue();
+if (Offset)
+  F.addFnAttr("patchable-function-prefix", std::to_string(Offset));

`if (unsigned Offset = MD->getZExtValue())`



Comment at: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll:1
+;; Test that we set patchable-function-prefix for asan.module_ctor when 
kcfi-offset is defined.
+

`test/Transforms/KCFI/kcfi-patchable-function-prefix.ll` or a new file is 
perhaps a better place for this test. It's mainly about kcfi's requirement and 
less about asan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141172

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


[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

2023-01-06 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D141098#4030426 , @rymiel wrote:

> The LLVM Coding Standard apparently doesn't mention line endings..?

Line endings probably should never be specified in coding standards, but the 
default should match the majority, and in the case of LLVM style, it should be 
`\n` IMO.

> A quick grep does show a bunch of \r\n results, primarily in tests.

It's possible that some/most of those `\r\n` get in by accident like in 
https://github.com/llvm/llvm-project/blob/617277e7cbdaea6881425c7a1a5b1cf4b1d4b507/clang/unittests/Format/FormatTest.cpp?
 And for tests, clang-format can be disabled like in D128706 
.

I think we should combine `DeriveLineEnding` and `UseCRLF`, with the default 
being LF for LLVM. If you all agree, I will abandon this patch and implement a 
new option and deprecate the current ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141098

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


[clang] f5f746f - [OpenMP] Introduce '-f[no-]openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2023-01-06T20:01:41-06:00
New Revision: f5f746f1efd45a6e9ed8ca9be26e8c01c5aa00b1

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

LOG: [OpenMP] Introduce '-f[no-]openmp-target-jit' flag to control JIT for 
offloading

JIT support for OpenMP offloading was introduced in D139287. This patch
adds a simple flag that enables this mode. It simply requires enabling
`-foffload-lto` mode and `--embed-bitcode` in the linker wrapper. This
option implies LTO if it is not enabled.

Reviewed By: jdoerfert

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

Added: 
clang/test/Driver/openmp-offload-jit.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 4e86a5ec46b99..ed4f7b8affeb3 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -126,6 +126,8 @@ def err_drv_invalid_unwindlib_name : Error<
   "invalid unwind library name in argument '%0'">;
 def err_drv_incompatible_unwindlib : Error<
   "--rtlib=libgcc requires --unwindlib=libgcc">;
+def err_drv_incompatible_options : Error<
+  "The combination of '%0' and '%1' is incompatible">;
 def err_drv_invalid_stdlib_name : Error<
   "invalid library name in argument '%0'">;
 def err_drv_invalid_output_with_multiple_archs : Error<

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b2f74add94ee4..d4afa4d98f5b5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2652,6 +2652,11 @@ def fopenmp_offload_mandatory : Flag<["-"], 
"fopenmp-offload-mandatory">, Group<
   Flags<[CC1Option, NoArgumentUnused]>,
   HelpText<"Do not create a host fallback if offloading to the device fails.">,
   MarshallingInfoFlag>;
+def fopenmp_target_jit : Flag<["-"], "fopenmp-target-jit">, Group,
+  Flags<[CoreOption, NoArgumentUnused]>,
+  HelpText<"Emit code that can be JIT compiled for OpenMP offloading. Implies 
-foffload-lto=full">;
+def fno_openmp_target_jit : Flag<["-"], "fno-openmp-target-jit">, 
Group,
+  Flags<[CoreOption, NoArgumentUnused, HelpHidden]>;
 def fopenmp_target_new_runtime : Flag<["-"], "fopenmp-target-new-runtime">,
   Group, Flags<[CC1Option, HelpHidden]>;
 def fno_openmp_target_new_runtime : Flag<["-"], 
"fno-openmp-target-new-runtime">,

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 61f835054c977..1a4fceb80a9ad 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -721,6 +721,17 @@ void Driver::setLTOMode(const llvm::opt::ArgList &Args) {
 
   OffloadLTOMode = parseLTOMode(*this, Args, options::OPT_foffload_lto_EQ,
 options::OPT_fno_offload_lto);
+
+  // Try to enable `-foffload-lto=full` if `-fopenmp-target-jit` is on.
+  if (Args.hasFlag(options::OPT_fopenmp_target_jit,
+   options::OPT_fno_openmp_target_jit, false)) {
+if (Arg *A = Args.getLastArg(options::OPT_foffload_lto_EQ,
+ options::OPT_fno_offload_lto))
+  if (OffloadLTOMode != LTOK_Full)
+Diag(diag::err_drv_incompatible_options)
+<< A->getSpelling() << "-fopenmp-target-jit";
+OffloadLTOMode = LTOK_Full;
+  }
 }
 
 /// Compute the desired OpenMP runtime from the flags provided.

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f16c799887995..e407d65f678db 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8428,6 +8428,10 @@ void LinkerWrapper::ConstructJob(Compilation &C, const 
JobAction &JA,
   }
   Args.ClaimAllArgs(options::OPT_Xoffload_linker);
 
+  // Embed bitcode instead of an object in JIT mode.
+  if (const Arg *A = Args.getLastArg(options::OPT_fopenmp_target_jit))
+CmdArgs.push_back("--embed-bitcode");
+
   // Forward `-mllvm` arguments to the LLVM invocations if present.
   for (Arg *A : Args.filtered(options::OPT_mllvm)) {
 CmdArgs.push_back("-mllvm");

diff  --git a/clang/test/Driver/openmp-offload-jit.c 
b/clang/test/Driver/openmp-offload-jit.c
new file mode 100644
index 0..93c38393025db
--- /dev/null
+++ b/clang/test/Driver/openmp-offload-jit.c
@@ -0,0 +1,54 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check that we enable LTO-mode properly with '-fopenmp-target-jit' and that 
it
+// still enabled LTO-mode if `-fno-offload-lto` is on.
+// RUN: %clang -### -

[PATCH] D141158: [OpenMP] Introduce '-f[no-]openmp-target-jit' flag to control JIT for offloading

2023-01-06 Thread Joseph Huber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5f746f1efd4: [OpenMP] Introduce 
'-f[no-]openmp-target-jit' flag to control JIT for offloading 
(authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141158

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-jit.c

Index: clang/test/Driver/openmp-offload-jit.c
===
--- /dev/null
+++ clang/test/Driver/openmp-offload-jit.c
@@ -0,0 +1,54 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check that we enable LTO-mode properly with '-fopenmp-target-jit' and that it
+// still enabled LTO-mode if `-fno-offload-lto` is on.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -foffload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -foffload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-phases -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -fno-offload-lto -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=PHASES-JIT %s
+//
+//  PHASES-JIT: 0: input, "[[INPUT:.+]]", c, (host-openmp)
+// PHASES-JIT-NEXT: 1: preprocessor, {0}, cpp-output, (host-openmp)
+// PHASES-JIT-NEXT: 2: compiler, {1}, ir, (host-openmp)
+// PHASES-JIT-NEXT: 3: input, "[[INPUT]]", c, (device-openmp)
+// PHASES-JIT-NEXT: 4: preprocessor, {3}, cpp-output, (device-openmp)
+// PHASES-JIT-NEXT: 5: compiler, {4}, ir, (device-openmp)
+// PHASES-JIT-NEXT: 6: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp ([[TARGET:.+]])" {5}, ir
+// PHASES-JIT-NEXT: 7: backend, {6}, lto-bc, (device-openmp)
+// PHASES-JIT-NEXT: 8: offload, "device-openmp ([[TARGET]])" {7}, lto-bc
+// PHASES-JIT-NEXT: 9: clang-offload-packager, {8}, image, (device-openmp)
+// PHASES-JIT-NEXT: 10: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (x86_64-unknown-linux-gnu)" {9}, ir
+// PHASES-JIT-NEXT: 11: backend, {10}, assembler, (host-openmp)
+// PHASES-JIT-NEXT: 12: assembler, {11}, object, (host-openmp)
+// PHASES-JIT-NEXT: 13: clang-linker-wrapper, {12}, image, (host-openmp)
+
+// Check that we add the `--embed-bitcode` flag to the linker wrapper.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LINKER %s
+// LINKER: clang-linker-wrapper"{{.*}}"--embed-bitcode"
+
+// Check for incompatible combinations
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fno-offload-lto \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=NO-LTO %s
+// NO-LTO: error: The combination of '-fno-offload-lto' and '-fopenmp-target-jit' is incompatible
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -foffload-lto=thin \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -fopenmp-target-jit %s 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN-LTO %s
+// THIN-LTO: error: The combination of '-foffload-lto=' and '-fopenmp-target-jit' is incompatible
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8428,6 +8428,10 @@
   }
   Args.ClaimAllArgs(options::OPT_Xoffload_linker);
 
+  // Embed bitcode instead of an object in JIT mode.
+  if (const Arg *A = Args.getLastArg(options::OPT_fopenmp_target_jit))
+CmdArgs.push_back("--embed-bitcode");
+
   // Forward `-mllvm` arguments to the LLVM invocations if present.
   for (Arg *A : Args.filtered(options::OPT_mllvm)) {
 CmdArgs.push_back("-mllvm");
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -721,6 +721,17 @@
 
   OffloadLTOMode = parseLTOMode(*this, Args, options::OPT_foffload_l

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-06 Thread Ben Boeckel via Phabricator via cfe-commits
ben.boeckel added a comment.

In D139168#4030425 , @ChuanqiXu wrote:

> BTW, if you are interested, it should be possible to use clang-scan-deps to 
> get the make-style format information.

I want a *single* depfile for *anything* scanned by the `clang-scan-deps` 
invocation, not one per TU scanned. Also note that the `-MT` is the output of 
`clang-scan-deps`; in this case, the `.ddi` file that contains P1689 
 content. I think it'd be *very* weird to have 
`clang-scan-deps -p1689-output=out.ddi -- clang -MF blah.d -MT out.ddi`.


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

https://reviews.llvm.org/D139168

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


[clang] cf97ee7 - [Clang] Fix mispelled option passed to the linker wrapper

2023-01-06 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2023-01-06T20:02:23-06:00
New Revision: cf97ee75f080579d2bbf2a559ec5d46ee00d828d

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

LOG: [Clang] Fix mispelled option passed to the linker wrapper

Summary:
This option was spelled wrong and caused errors if used in combination
with the linking job. Fix it.

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index e407d65f678d..3cbffef3064d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8395,7 +8395,7 @@ void LinkerWrapper::ConstructJob(Compilation &C, const 
JobAction &JA,
   }
 
   for (const auto &A : Args.getAllArgValues(options::OPT_Xcuda_ptxas))
-CmdArgs.push_back(Args.MakeArgString("--ptxas-args=" + A));
+CmdArgs.push_back(Args.MakeArgString("--ptxas-arg=" + A));
 
   // Forward remarks passes to the LLVM backend in the wrapper.
   if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))



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


[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

2023-01-06 Thread Theodore Luo Wang via Phabricator via cfe-commits
theo-lw created this revision.
theo-lw added a reviewer: aaron.ballman.
Herald added a project: All.
theo-lw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This changes the diagnostic for _Alignas so it no longer tells people to place 
_Alignas after the "struct" keyword.

Fixes https://github.com/llvm/llvm-project/issues/58637


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141177

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/c1x-alignas.c


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5291,18 +5291,29 @@
   // __attribute__((aligned)) struct A;
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
-DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }
+};
+
 if (TypeSpecType == DeclSpec::TST_class ||
 TypeSpecType == DeclSpec::TST_struct ||
 TypeSpecType == DeclSpec::TST_interface ||
 TypeSpecType == DeclSpec::TST_union ||
 TypeSpecType == DeclSpec::TST_enum) {
-  for (const ParsedAttr &AL : DS.getAttributes())
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-  for (const ParsedAttr &AL : DeclAttrs)
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }
 }
   }
 


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5291,18 +5291,29 @@
   // __attribute__((aligned)) struct A;
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
-DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }
+};
+
 if (TypeSpecType == DeclSpec::TST_class ||
 TypeSpecType == DeclSpec::TST_struct ||
 TypeSpecType == DeclSpec::TST_interface ||
 TypeSpecType == DeclSpec::TST_union ||
 TypeSpecType == DeclSpec::TST_enum) {
-  for (const ParsedAttr &AL : DS.getAttributes())
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-  for (const ParsedAttr &AL : DeclAttrs)
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

2023-01-06 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

> I think we should combine `DeriveLineEnding` and `UseCRLF`

Do you mean a single `LineEnding` options which is an enum with `LF`, `CRLF` 
and `Derive`, or similar?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141098

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


  1   2   >