date:20230109

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

2023-01-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674
+else if (const auto *ND = dyn_cast(Context)) {
+  if (ND->isInlineNamespace())
+Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");

tom-anders wrote:
> kadircet wrote:
> > tom-anders wrote:
> > > kadircet wrote:
> > > > since we know that the `Context` is a `NamespaceDecl` it should be safe 
> > > > to use `printQualifiedName` always. any reason for the extra branching 
> > > > here (apart from minimizing the change to behaviour)? if not I think we 
> > > > can get rid of the special casing.
> > > Unfortunately, this fails CompletionTest.EnclosingScopeComesFirst and 
> > > CompletionTest.NoDuplicatedQueryScopes, I think because of anonymous 
> > > namespaces (where printNameSpaceScope would return an empty string, but 
> > > (printQualifiedName(*ND) + "::" does not). 
> > i see. taking a closer look at this `getQueryScopes` is used for two things:
> > - The scopes to query with fuzzyfind requests, hence this should use the 
> > same "serialization" as symbolcollector (which only strips anon namespaces 
> > today, but initially it were to strip both anon & inline namespaces. it got 
> > changed inside clang without clangd tests catching it).
> > - The shortening of the fully qualified name in `CodeCompletionBuilder`. 
> > Not having inline namespaces spelled in the available namespaces implies 
> > getting wrong qualifiers (such as the bug you're fixing).
> > 
> > so considering the requirements here:
> > - when querying index, we actually want to hide inline namespaces (as 
> > `ns::hidden::Foo` should be a viable alternative even if only `ns::` is 
> > accessible). so we should actually fix `printQualifiedName` to set 
> > `SuppressInlineNamespace` in printing policy to restore the old behaviour 
> > (and keep using `printNamespaceScope` here).
> > - inside `CodeCompletionBuilder`, we shouldn't use the same scopes we use 
> > during index queries. we should use the visible namespaces while preserving 
> > inline namespace information and only ignoring the anonymous namespaces.
> > 
> > hence can we have 2 separate scopes in `CodeCompleteFlow` instead?
> > One called `QueryScopes`, which has the behavior we have today (fixing 
> > printQualifiedName is a separate issues).
> > Other called `AccessibleScopes`, which has accessible namespaces spelled 
> > **with** inline namespaces, so that we can get proper qualification during 
> > code-complete.
> > 
> > does that make sense?
> tbh I'm a bit confused - I understand your requirements, but am not sure I 
> understand your proposed solution. Can you expand a bit further? Looking at 
> the code, there are already both `QueryScopes` and `AccessibleScopes` 
> variables/fields in various classes, I'm not really sure at which places you 
> want to make changes.
sorry for the long and confusing answer :D

I was talking about `CodeCompleteFlow` class specifically, inside 
`CodeComplete.cpp`. Currently it only has `QueryScopes`, derived from the 
visible contexts reported by Sema. Unfortunately it loses some granularity to 
fetch more symbols from index hence it should not be used when picking the 
required qualifier.
My suggestion is to add a new field in `CodeCompleteFlow` called 
`AccessibleScope`, which is derived at the same stage as `QueryScopes`, while 
preserving inline namespaces, and used when creating `CodeCompletionBuilder` in 
`CodeCompleteFlow::toCodeCompletion` (instead of `QueryScopes`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140915

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

[clang] 570bf97 - [clang][analyzer] Remove report of null stream from StreamChecker.

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


Author: Balázs Kéri
Date: 2023-01-09T09:49:08+01:00
New Revision: 570bf972f5adf05438c7e08d693bf4b96bfd510a

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

LOG: [clang][analyzer] Remove report of null stream from StreamChecker.

The case of NULL stream passed to stream functions was reported by 
StreamChecker.
The same condition is checked already by StdLibraryFunctionsChecker and it is
enough to check at one place. The StreamChecker stops now analysis if a passed 
NULL
stream is encountered but generates no report.
This change removes a dependency between StdCLibraryFunctionArgs checker and
StreamChecker. There is now no more specific message reported by StreamChecker,
the previous weak-dependency is not needed. And StreamChecker can be used
without StdCLibraryFunctions checker or its ModelPOSIX option.

Reviewed By: Szelethus

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

Added: 
clang/test/Analysis/stream-stdlibraryfunctionargs.c

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
clang/test/Analysis/stream-noopen.c
clang/test/Analysis/stream-note.c
clang/test/Analysis/stream.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 3dd2c7c1606c7..094b3a69c2302 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -588,7 +588,7 @@ def StdCLibraryFunctionArgsChecker : 
Checker<"StdCLibraryFunctionArgs">,
"such as whether the parameter of isalpha is in the range [0, 255] "
"or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
-  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, 
StreamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
   Documentation;
 
 } // end "alpha.unix"

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index e02ec4d6e71d6..4af6a69edd6a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -209,7 +209,6 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 
 class StreamChecker : public Checker 
{
-  BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
   BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
   BugType BT_UseAfterOpenFailed{this, "Invalid stream",
 "Stream handling error"};
@@ -338,7 +337,7 @@ class StreamChecker : public Checker(
-  BT_FileNull, "Stream pointer might be NULL.", N);
-  if (StreamE)
-bugreporter::trackExpressionValue(N, StreamE, *R);
-  C.emitReport(std::move(R));
-}
+// Stream argument is NULL, stop analysis on this path.
+// This case should occur only if StdLibraryFunctionsChecker (or ModelPOSIX
+// option of it) is not turned on, otherwise that checker ensures non-null
+// argument.
+C.generateSink(StateNull, C.getPredecessor());
 return nullptr;
   }
 

diff  --git 
a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c 
b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index f00e10a0f056f..4e5c66a8a0a40 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -18,9 +18,9 @@
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.NonNullParamChecker
-// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
 // CHECK-NEXT: alpha.unix.StdCLibraryFunctionArgs
+// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.Errno
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.TrustReturnsNonnull

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c 
b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
index 1dead7327604f..3d2d5a6aae2b2 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -6,7 +6,6 @@
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -verify
 
@@ -18,7 +17,6 @@
 // RUN:

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-09 Thread Balázs Kéri 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 rG570bf972f5ad: [clang][analyzer] Remove report of null stream 
from StreamChecker. (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D137790?vs=474559&id=487314#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
  clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
  clang/test/Analysis/stream-noopen.c
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream-stdlibraryfunctionargs.c
  clang/test/Analysis/stream.c

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -2,81 +2,6 @@
 
 #include "Inputs/system-header-simulator.h"
 
-void check_fread(void) {
-  FILE *fp = tmpfile();
-  fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fwrite(void) {
-  FILE *fp = tmpfile();
-  fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fseek(void) {
-  FILE *fp = tmpfile();
-  fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_ftell(void) {
-  FILE *fp = tmpfile();
-  ftell(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_rewind(void) {
-  FILE *fp = tmpfile();
-  rewind(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fgetpos(void) {
-  FILE *fp = tmpfile();
-  fpos_t pos;
-  fgetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fsetpos(void) {
-  FILE *fp = tmpfile();
-  fpos_t pos;
-  fsetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_clearerr(void) {
-  FILE *fp = tmpfile();
-  clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_feof(void) {
-  FILE *fp = tmpfile();
-  feof(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_ferror(void) {
-  FILE *fp = tmpfile();
-  ferror(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fileno(void) {
-  FILE *fp = tmpfile();
-  fileno(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void f_open(void) {
-  FILE *p = fopen("foo", "r");
-  char buf[1024];
-  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
-}
-
 void f_seek(void) {
   FILE *p = fopen("foo", "r");
   if (!p)
@@ -161,7 +86,7 @@
 }
 
 void check_freopen_1(void) {
-  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // expected-warning {{Stream pointer might be NULL}}
+  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // Not reported by the stream checker.
   f1 = freopen(0, "w", (FILE *)0x123456);  // Do not report this as error.
 }
 
Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===
--- /dev/null
+++ clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -0,0 +1,142 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.StdCLibraryFunctionArgs,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=any %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.StdCLibraryFunctionArgs,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+
+#include "Inputs/system-header-simulator.h"
+
+extern void clang_analyzer_eval(int);
+
+void *buf;
+size_t size;
+size_t n;
+
+void test_fopen(void) {
+  FILE *fp = fopen("path", "r");
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+  // stdargs-note{{The 1st argument should not be NULL}}
+}
+
+void test_tmpfile(void) {
+  FILE *fp = tmpfile();
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+  // stdargs-note{{The 1st argument should not be NULL}}
+}
+
+void test_fclose(void) {
+  FILE *fp = tmpfile();
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

2023-01-09 Thread Yueh-Ting (eop) Chen via Phabricator via cfe-commits

eopXD updated this revision to Diff 487317.
eopXD edited the summary of this revision.
eopXD added a comment.

Update test cases. Don't add target feature `zfh` and `zvfh` for intrinsics 
that does not have a floating-point type variant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

Files:
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vand.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcompress.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcpop.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfabs.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfclass.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfirst.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmerge.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfncvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfneg.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrec7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredosum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredusum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsqrt7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsgnj.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1down.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1up.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsqrt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwred

[PATCH] D139458: [clangd] Full support for #import insertions

2023-01-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139458

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

[PATCH] D141218: [clangd] Include the correct header for typeid()

2023-01-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

thanks for catching this!




Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:1531
 
+TEST(IncludeFixerTest, Typeid) {
+  Annotations Test(R"cpp(

nit: i'd actually drop the test, as this is too much of a detail to test 
against.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141218

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

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

2023-01-09 Thread Sander de Smalen via Phabricator via cfe-commits

sdesmalen added a comment.

Using metadata seems sensible, but did you also identify any downsides? I could 
imagine that we'd need to manually propagate metadata to any nodes after we do 
a combine (which can't be blindly copied?), e.g. add + mul -> mla, this new 
intrinsic would also need the metadata.

For intrinsics that don't have a directly corresponding (unpredicated) LLVM IR 
instruction, is there still a way to use this information in SelectionDAG?

> the select instruction itself has strict rules relating to poison that 
> hampered the intent of this change

For my understanding, can you elaborate what these strict rules regarding 
poison are that hamper such a change, and what it was that you tried?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141240

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

[PATCH] D139935: [NFC] [Doc] Fix example for AnnotateTypeDocs

2023-01-09 Thread Martin Böhme via Phabricator via cfe-commits

mboehme added a comment.

Sorry for the delay.

Just to confirm, yes this should of course be `annotate_type`. Thanks for 
catching my mistake.

@aaron.ballman Thanks for reviewing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139935

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

[PATCH] D141230: [clang-format-diff.py] give clang-format-diff a job pool (10x speed)

2023-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits

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

You've lost the first part of the patch in this latest diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141230

___
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-09 Thread MyDeveloperDay via Phabricator via cfe-commits

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

I kind of agree, it would be nice not to have to use dos2unix and it be set by 
default.


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

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

2023-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D140843#4029641 , @owenpan wrote:

> In D140843#4028142 , 
> @MyDeveloperDay wrote:
>
>> I'd like one of @owenpan, @HazardyKnusperkeks  or @rymiel to comment before 
>> commit, just to get another view.
>
> Perhaps clang-format should never insert a space in `>>`.

I seem to remember some "quirkiness" with nested templates that need converting

`Foo>`

into

`Foo >`

but I think that was sometime back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140843

___
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-09 Thread Christian Kandeler via Phabricator via cfe-commits

ckandeler added a comment.

In D139926#4033152 , @nridge wrote:

> In D139926#4032473 , @ckandeler 
> wrote:
>
>> 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.
>
> To clarify, I'm not suggesting that any client-side logic use this heuristic, 
> only that it's probably good enough for a human reader to disambiguate 
> without needing to assign angle brackets and comparison operators different 
> colors.

Note that we use this information to *animate* the matching tokens, i.e. when 
the cursor is on one of them, both it and its counterpart get a special 
highlighting. That's why it's so important that the language server guarantees 
they always come in pairs.


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] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent LE GARREC via Phabricator via cfe-commits

bansan added a comment.
Herald added a subscriber: StephenFan.

I just made a test:

  #include 
  #include 
  
  int64_t f(int x) {
return 1024 * 1024 * 1024 * x;
  }
  
  void g(int x) {
std::vector b;
b.reserve(1024 * 1024 * 1024 * x);
  }
  
  int main() {
f(1024);
g(1024);
  }

The fixed code:

  #include 
  #include 
  #include 
  
  int64_t f(int x) {
return static_cast(1024 * 1024 * 1024 * x);
  }
  
  void g(int x) {
std::vector b;
b.reserve(static_cast(1024 * 1024 * 1024 * x));
  }
  
  int main() {
f(1024);
g(1024);
  }

For the first test, I still think that the auto fix should be `return 
static_cast(1024) * 1024 * 1024 * x;` to have the good result.

For the second test, the type is the internal type of `std::vector`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141134: [NFC] Only expose getXXXSize functions in TypeSize

2023-01-09 Thread Guillaume Chatelet via Phabricator via cfe-commits

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

- Fix missing clang change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141134

Files:
  clang/lib/CodeGen/CGDecl.cpp
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/Loads.cpp
  llvm/lib/CodeGen/Analysis.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
  llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/utils/TableGen/CodeGenDAGPatterns.cpp

Index: llvm/utils/TableGen/CodeGenDAGPatterns.cpp
===
--- llvm/utils/TableGen/CodeGenDAGPatterns.cpp
+++ llvm/utils/TableGen/CodeGenDAGPatterns.cpp
@@ -764,8 +764,8 @@
 namespace {
 struct TypeSizeComparator {
   bool operator()(const TypeSize &LHS, const TypeSize &RHS) const {
-return std::make_tuple(LHS.isScalable(), LHS.getKnownMinValue()) <
-   std::make_tuple(RHS.isScalable(), RHS.getKnownMinValue());
+return std::make_tuple(LHS.isScalable(), LHS.getKnownMinSize()) <
+   std::make_tuple(RHS.isScalable(), RHS.getKnownMinSize());
   }
 };
 } // end anonymous namespace
Index: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
===
--- llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -543,7 +543,7 @@
   if (!isAligned(I->getAlign(), Off))
 return false;
 
-  NeededDerefBytes = std::max(NeededDerefBytes, Off + Size.getFixedValue());
+  NeededDerefBytes = std::max(NeededDerefBytes, Off + Size.getFixedSize());
   NeededAlign = std::max(NeededAlign, I->getAlign());
 }
 
Index: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
===
--- llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -990,7 +990,7 @@
 unsigned RISCVTTIImpl::getEstimatedVLFor(VectorType *Ty) {
   if (isa(Ty)) {
 const unsigned EltSize = DL.getTypeSizeInBits(Ty->getElementType());
-const unsigned MinSize = DL.getTypeSizeInBits(Ty).getKnownMinValue();
+const unsigned MinSize = DL.getTypeSizeInBits(Ty).getKnownMinSize();
 const unsigned VectorBits = *getVScaleForTuning() * RISCV::RVVBitsPerBlock;
 return RISCVTargetLowering::computeVLMAX(VectorBits, EltSize, MinSize);
   }
@@ -1455,7 +1455,7 @@
   TypeSize Size = DL.getTypeSizeInBits(Ty);
   if (Ty->isVectorTy()) {
 if (Size.isScalable() && ST->hasVInstructions())
-  return divideCeil(Size.getKnownMinValue(), RISCV::RVVBitsPerBlock);
+  return divideCeil(Size.getKnownMinSize(), RISCV::RVVBitsPerBlock);
 
 if (ST->useRVVForFixedLengthVectors())
   return divideCeil(Size, ST->getRealMinVLen());
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -138,7 +138,7 @@
   if (VT.getVectorMinNumElements() < MinElts)
 return;
 
-  unsigned Size = VT.getSizeInBits().getKnownMinValue();
+  unsigned Size = VT.getSizeInBits().getKnownMinSize();
   const TargetRegisterClass *RC;
   if (Size <= RISCV::RVVBitsPerBlock)
 RC = &RISCV::VRRegClass;
@@ -1589,7 +1589,7 @@
 
 RISCVII::VLMUL RISCVTargetLowering::getLMUL(MVT VT) {
   assert(VT.isScalableVector() && "Expecting a scalable vector type");
-  unsigned KnownSize = VT.getSizeInBits().getKnownMinValue();
+  unsigned KnownSize = VT.getSizeInBits().getKnownMinSize();
   if (VT.getVectorElementType() == MVT::i1)
 KnownSize *= 8;
 
@@ -5443,7 +5443,7 @@
 // Optimize for constant AVL
 if (isa(AVL)) {
   unsigned EltSize = VT.getScalarSizeInBits();
-  unsigned MinSize = VT.getSizeInBits().getKnownMinValue();
+  unsigned MinSize = VT.getSizeInBits().getKnownMinSize();
 
   unsigned VectorBitsMax = Subtarget.getRealMaxVLen();
   unsigned MaxVLMAX =
@@ -6419,7 +6419,7 @@
 return DAG.getNode(ISD::TRUNCATE, DL, VecVT, Op2);
   }
   unsigned EltSize = VecVT.getScalarSizeInBits();
-  unsigned MinSize = VecVT.getSizeInBits().getKnownMinValue();
+  unsigned MinSize = VecVT.getSizeInBits().getKnownMinSize();
   unsigned VectorBitsMax = Subtarget.getRealMaxVLen();
   unsigned MaxVLMAX =
 RISCVTargetLowering::computeVLMAX(VectorBitsMax, EltSize, MinSize);
Index: llvm/lib

[PATCH] D141134: [NFC] Only expose getXXXSize functions in TypeSize

2023-01-09 Thread Guillaume Chatelet via Phabricator via cfe-commits

gchatelet added a comment.

I've created https://reviews.llvm.org/D141267 to get a feel of what it looks 
like to only keep the Size version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141134

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

Thank you for taking a look at this patch! @bansan

> For the first test, I still think that the auto fix should be return 
> static_cast(1024) * 1024 * 1024 * x; to have the good result.

I'd say I'm not the author of this checker. I think this behavior change needs 
a discussion somewhere, for example, you could raise an issue on GitHub. I just 
want to fix the wrong fix-hint and I think this is out of what this patch 
should do. Sorry!

> For the second test, the type is the internal type of std::vector.

Good catch, thanks! I didn't realize this problem. Will take a look at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

___
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-09 Thread Arthur Laurent via Phabricator via cfe-commits

Arthapz added a comment.

> BTW, for header units, it is still under discussion that how should build 
> system and compiler interact about header units. It is still unclear whether 
> or not the header units should be transparent to build systems (and other 
> tools).

On XMake we built our support around MSVC then extended support to GCC and 
clang, so we handle header units separately from named modules (but in a 
similar way), i don't have problem to add some logic in XMake to support them 
for clang


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

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Viktoriia Bakalova via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141271

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,28 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, TemplateInstantiations) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots, 
testing::Not(testing::ElementsAre(named("get";
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,15 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template 
+bool isImplicitTemplateSpecialization(const NamedDecl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,11 +374,21 @@
   for (Decl *D : DG) {
 if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation(
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (const NamedDecl *ND = dyn_cast(D))
+  if (isImplicitTemplateInstantiation(ND))
+continue;
+// FIXME: Filter out certain Obj-C as well.
 Out->Roots.push_back(D);
   }
   return ASTConsumer::HandleTopLevelDecl(DG);
 }
+
+  private:
+  bool isImplicitTemplateInstantiation(const NamedDecl *D) {   
+return isImplicitTemplateSpecialization(D) ||
+ isImplicitTemplateSpecialization(D) ||
+ isImplicitTemplateSpecialization(D);
+  }
   };
 
   return std::make_unique(this);


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,28 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, TemplateInstantiations) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots, testing::Not(testing::ElementsAre(named("get";
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,15 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template 
+bool isImplicitTemplateSpecialization(const NamedDecl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique

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

2023-01-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 487363.
carlosgalvezp marked an inline comment as done.
carlosgalvezp added a comment.

Fix formatting


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 s

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

2023-01-09 Thread Pierre van Houtryve via Phabricator via cfe-commits

Pierre-vh added a comment.

Ping? it's a small NFC and if it's not desired I don't mind abandoning it; I'd 
just like to remove this diff from the queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139608

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

> For the second test, the type is the internal type of std::vector.

There are two approaches to fixing this issue. One is fixing with the **FULLY** 
qualified type name, in the above case that is, `std::vector>::size_type`. Another one is fixing with the desugared type 
name, that is, `unsigned long`. I personally don't have a strong opinion on 
which one is better.

WDYT, @bansan?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

> There are two approaches to fixing this issue. One is fixing with the FULLY 
> qualified type name, in the above case that is, std::vector std::allocator>::size_type. Another one is fixing with the desugared 
> type name, that is, unsigned long. I personally don't have a strong opinion 
> on which one is better.

IMHO, the first approach can cause redundancy in some cases, like this one. The 
second one will turn `int64_t` into `long`, which I don't have a good feeling 
about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

___
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-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 487370.
carlosgalvezp added a comment.

Trim first character instead, to keep the code visually
pleasing.


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  - Appl

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

2023-01-09 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 added a comment.

The newly added test fails when `tools/clang/include/clang/Config/config.h` is 
configured with `#define CLANG_DEFAULT_OPENMP_RUNTIME "libgomp"`. Please fix. 
Thanks!


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] D140968: [clang-tidy] Add check for passing the result of `std::string::c_str` to `strlen`

2023-01-09 Thread Alex Coster via Phabricator via cfe-commits

acoster added a comment.

In D140968#4029072 , @njames93 wrote:

> I don't see the appear of this check as its a situation that I doubt ever 
> appears in code bases. If there are open source code bases where this is a 
> known problem can you please provide links to them as well as running the 
> run_clang_tidy script over them to verify the changes are good.

After a look at some projects, it seems like the pattern does arise, but often 
it's intended (eg a string buffer is set to a large size, a legacy function 
taking a char* writes to it, and then the string is resized to 
strlen(foo.c_str())). As such, I'll drop this patch, as it can introduce bugs 
in these cases.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140968

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

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:377
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (const NamedDecl *ND = dyn_cast(D))
+  if (isImplicitTemplateInstantiation(ND))

as we already do a dyn_cast inside the `isImplicitTemplateInstantiation`, we 
can get rid of this NamedDecl cast by changing the 
`isImplicitTemplateInstantiation` to accept a `Decl*`.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:387
+  private:
+  bool isImplicitTemplateInstantiation(const NamedDecl *D) {   
+return isImplicitTemplateSpecialization(D) ||

we can move it to the above anonymous namespace as well (it is fine to have two 
functions with same name because of C++ overloads). In this case, I'd probably 
inline it in `HandleTopLevelDecl`.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:105
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, TemplateInstantiations) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(

maybe DropImplicitTemplateTopDecl?



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:120
+  };
+  int v = dispatch();
+  )cpp";

this example is a nice example to show the original bug (where we inserted 
unexpected additional headers) in include-cleaner. 

The scope of this patch is to filter out the implicit template instantiation, a 
simple testcase like below could work. (the current testcase is fine as well, 
up to you)

```
template
int func(T) { return 0; } 
auto s = func(1);
```



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:123
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots, 
testing::Not(testing::ElementsAre(named("get";
+}

nit: instead doing a negative verification, I'd check the captured root decl, 
which is (a `MyGetter` CXXRecordDecl, and a `v` var decl), and with a comment 
saying the implicitly-instantiated method decl is filtered out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141271

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

[PATCH] D140551: [include-cleaner] Don't count references to operators as uses

2023-01-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 487382.
hokein marked an inline comment as done.
hokein added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140551

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -245,6 +245,16 @@
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+   "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+   "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+   "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -66,6 +66,19 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+if (!WalkUpFromCXXOperatorCallExpr(S))
+  return false;
+
+// Operators are always ADL extension points, by design references to them
+// doesn't count as uses (generally the type should provide them).
+// Ignore them by traversing arguments instead of children.
+for (auto *Arg : S->arguments())
+  if (!TraverseStmt(Arg))
+return false;
+return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 report(DRE->getLocation(), DRE->getFoundDecl());
 return true;


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -245,6 +245,16 @@
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+   "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+   "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+   "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -66,6 +66,19 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+if (!WalkUpFromCXXOperatorCallExpr(S))
+  return false;
+
+// Operators are always ADL extension points, by design references to them
+// doesn't count as uses (generally the type should provide them).
+// Ignore them by traversing arguments instead of children.
+for (auto *Arg : S->arguments())
+  if (!TraverseStmt(Arg))
+return false;
+return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 report(DRE->getLocation(), DRE->getFoundDecl());
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140551: [include-cleaner] Don't count references to operators as uses

2023-01-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:69
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+// Operators are always ADL extension points, by design references to them

sammccall wrote:
> Rather than override TraverseCXXOperatorCallExpr to do everything *except* 
> call WalkUpFrom, maybe override WalkUpFromCXXOperatorCallExpr to call 
> VisitCXXOperatorCallExpr but not WalkUpFromCallExpr? (With a comment that we 
> don't want to treat operators as calls)
> 
> Rather than override TraverseCXXOperatorCallExpr to do everything *except* 
> call WalkUpFrom.

Ah, not really (it is my fault that I missed to call `WalkUpFrom`, but it is 
not the key point) .
The key point is to traverse every child of `CXXOperatorCallExpr` *except* the 
operator.  E.g. for the AST node below, we don't want to traverse its first 
child which is the operator.

```
-CXXOperatorCallExpr 0x5591a97f5280  'int' '+'
|-ImplicitCastExpr 0x5591a97f5268  'int (*)(string, string)' 

| `-DeclRefExpr 0x5591a97f51e8  'int (string, string)' lvalue 
Function 0x5591a97f4878 'operator+' 'int (string, string)'
|-CXXTemporaryObjectExpr 0x5591a97f5068  'string':'string' 
'void () noexcept' zeroing
`-CXXTemporaryObjectExpr 0x5591a97f51b8 
```

I added the `WalkUpFromCXXOperatorCallExpr` back (I think it doesn't have any 
effect on the current code because we don't do anything on the 
`CXXOperatorCallExpr` hierarchy classes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140551

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent LE GARREC via Phabricator via cfe-commits

bansan added a comment.

The current rule is quite simple. I tried this example:

  template
  T h(int x) {
return 1024 * 1024 * 1024 * x;
  }
  
  int main() {
h(1024);
  }

The rule does not complain. So I think that the "resolved" type (`size_t`) 
should be use instead of the templated one (`std::vector>::size_type`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D134677: [Clang][AArch64][SME] Add ZA zeroing intrinsics

2023-01-09 Thread Bryan Chan via Phabricator via cfe-commits

bryanpkc updated this revision to Diff 487384.
bryanpkc retitled this revision from "[Clang][AArch64] Add SME zero intrinsic" 
to "[Clang][AArch64][SME] Add ZA zeroing intrinsics".
bryanpkc edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134677

Files:
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Basic/arm_sme.td
  clang/include/clang/Basic/arm_sve_sme_incl.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_zero.c

Index: clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_zero.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_zero.c
@@ -0,0 +1,47 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -D__ARM_FEATURE_SME -triple aarch64-none-linux-gnu -target-feature +sme -target-feature +sve -S -O1 -Werror -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -D__ARM_FEATURE_SME -triple aarch64-none-linux-gnu -target-feature +sme -target-feature +sve -S -O1 -Werror -emit-llvm -o - -x c++ %s | FileCheck %s -check-prefixes=CHECK,CHECK-CXX
+// RUN: %clang_cc1 -D__ARM_FEATURE_SME -triple aarch64-none-linux-gnu -target-feature +sme -target-feature +sve -S -O1 -Werror -o /dev/null %s
+
+#include 
+
+// CHECK-C-LABEL: @test_svzero_mask_za(
+// CHECK-CXX-LABEL: @_Z19test_svzero_mask_zav(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void @llvm.aarch64.sme.zero(i32 0)
+// CHECK-NEXT:ret void
+//
+void test_svzero_mask_za() {
+  svzero_mask_za(0);
+}
+
+// CHECK-C-LABEL: @test_svzero_mask_za_1(
+// CHECK-CXX-LABEL: @_Z21test_svzero_mask_za_1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void @llvm.aarch64.sme.zero(i32 176)
+// CHECK-NEXT:ret void
+//
+void test_svzero_mask_za_1() {
+  svzero_mask_za(176);
+}
+
+// CHECK-C-LABEL: @test_svzero_mask_za_2(
+// CHECK-CXX-LABEL: @_Z21test_svzero_mask_za_2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void @llvm.aarch64.sme.zero(i32 255)
+// CHECK-NEXT:ret void
+//
+void test_svzero_mask_za_2() {
+  svzero_mask_za(255);
+}
+
+// CHECK-C-LABEL: @test_svzero_za(
+// CHECK-CXX-LABEL: @_Z14test_svzero_zav(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void @llvm.aarch64.sme.zero(i32 255)
+// CHECK-NEXT:ret void
+//
+void test_svzero_za() {
+  svzero_za();
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4256,6 +4256,9 @@
   llvm::Value *EmitSMEReadWrite(SMETypeFlags TypeFlags,
 llvm::SmallVectorImpl &Ops,
 unsigned IntID);
+  llvm::Value *EmitSMEZero(SMETypeFlags TypeFlags,
+   llvm::SmallVectorImpl &Ops,
+   unsigned IntID);
   llvm::Value *EmitAArch64SMEBuiltinExpr(unsigned BuiltinID, const CallExpr *E);
 
   llvm::Value *EmitAArch64BuiltinExpr(unsigned BuiltinID, const CallExpr *E,
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -9438,6 +9438,16 @@
   return Builder.CreateCall(F, Ops);
 }
 
+Value *CodeGenFunction::EmitSMEZero(SMETypeFlags TypeFlags,
+SmallVectorImpl &Ops,
+unsigned IntID) {
+  // svzero_za() intrinsic zeros the entire za tile and has no paramters.
+  if (Ops.size() == 0)
+Ops.push_back(llvm::ConstantInt::get(Int32Ty, 255));
+  Function *F = CGM.getIntrinsic(IntID, {});
+  return Builder.CreateCall(F, Ops);
+}
+
 // Limit the usage of scalable llvm IR generated by the ACLE by using the
 // sve dup.x intrinsic instead of IRBuilder::CreateVectorSplat.
 Value *CodeGenFunction::EmitSVEDupX(Value *Scalar, llvm::Type *Ty) {
@@ -9894,6 +9904,8 @@
 return EmitSMELd1St1(TypeFlags, Ops, Builtin->LLVMIntrinsic);
   else if (TypeFlags.isMove())
 return EmitSMEReadWrite(TypeFlags, Ops, Builtin->LLVMIntrinsic);
+  else if (TypeFlags.isZero())
+return EmitSMEZero(TypeFlags, Ops, Builtin->LLVMIntrinsic);
 
   /// Should not happen
   return nullptr;
Index: clang/include/clang/Basic/arm_sve_sme_incl.td
===
--- clang/include/clang/Basic/arm_sve_sme_incl.td
+++ clang/include/clang/Basic/arm_sve_sme_incl.td
@@ -216,6 +216,7 @@
 def IsSharedZA: FlagType<0x20>;
 def IsPreservesZA : FlagType<0x40>;
 def IsMove: FlagType<0x80>;
+def IsZero: FlagType<0x100>;
 
 // These must be kep

[PATCH] D140551: [include-cleaner] Don't count references to operators as uses

2023-01-09 Thread Sam McCall via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:75
+// doesn't count as uses (generally the type should provide them).
+// Ignore them by traversing arguments instead of children.
+for (auto *Arg : S->arguments())

This comment doesn't mention the callee at all, and the callee is the point.

Maybe: "Don't traverse the callee." and add a newline before the arguments() 
loop?
(because this comment isn't really related to that part, arguments() is just 
part of the "everything else" that we're still doing)



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:69
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+// Operators are always ADL extension points, by design references to them

hokein wrote:
> sammccall wrote:
> > Rather than override TraverseCXXOperatorCallExpr to do everything *except* 
> > call WalkUpFrom, maybe override WalkUpFromCXXOperatorCallExpr to call 
> > VisitCXXOperatorCallExpr but not WalkUpFromCallExpr? (With a comment that 
> > we don't want to treat operators as calls)
> > 
> > Rather than override TraverseCXXOperatorCallExpr to do everything *except* 
> > call WalkUpFrom.
> 
> Ah, not really (it is my fault that I missed to call `WalkUpFrom`, but it is 
> not the key point) .
> The key point is to traverse every child of `CXXOperatorCallExpr` *except* 
> the operator.  E.g. for the AST node below, we don't want to traverse its 
> first child which is the operator.
> 
> ```
> -CXXOperatorCallExpr 0x5591a97f5280  'int' '+'
> |-ImplicitCastExpr 0x5591a97f5268  'int (*)(string, string)' 
> 
> | `-DeclRefExpr 0x5591a97f51e8  'int (string, string)' lvalue 
> Function 0x5591a97f4878 'operator+' 'int (string, string)'
> |-CXXTemporaryObjectExpr 0x5591a97f5068  'string':'string' 
> 'void () noexcept' zeroing
> `-CXXTemporaryObjectExpr 0x5591a97f51b8 
> ```
> 
> I added the `WalkUpFromCXXOperatorCallExpr` back (I think it doesn't have any 
> effect on the current code because we don't do anything on the 
> `CXXOperatorCallExpr` hierarchy classes).
Yeah, sorry I was confused and thought we handled function refs in CallExpr 
instead of DeclRefExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140551

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

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Viktoriia Bakalova via Phabricator via cfe-commits

VitaNuo updated this revision to Diff 487385.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141271

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,29 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, ImplicitTemplates) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots,
+  testing::ElementsAre(named("MyGetter"), named("v")));
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,14 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template  bool isImplicitTemplateSpecialization(const Decl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,7 +373,11 @@
   for (Decl *D : DG) {
 if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation(
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D))
+  continue;
+// FIXME: Filter out certain Obj-C as well.
 Out->Roots.push_back(D);
   }
   return ASTConsumer::HandleTopLevelDecl(DG);


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,29 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, ImplicitTemplates) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots,
+  testing::ElementsAre(named("MyGetter"), named("v")));
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,14 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template  bool isImplicitTemplateSpecialization(const Decl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,7 +373,11 @@
   for (Decl *D : DG) {
 if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation(
   continue;
-// FIXME: Filter out certa

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Viktoriia Bakalova via Phabricator via cfe-commits

VitaNuo added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:377
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (const NamedDecl *ND = dyn_cast(D))
+  if (isImplicitTemplateInstantiation(ND))

hokein wrote:
> as we already do a dyn_cast inside the `isImplicitTemplateInstantiation`, we 
> can get rid of this NamedDecl cast by changing the 
> `isImplicitTemplateInstantiation` to accept a `Decl*`.
Oh right, I didn't notice. Thanks.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:387
+  private:
+  bool isImplicitTemplateInstantiation(const NamedDecl *D) {   
+return isImplicitTemplateSpecialization(D) ||

hokein wrote:
> we can move it to the above anonymous namespace as well (it is fine to have 
> two functions with same name because of C++ overloads). In this case, I'd 
> probably inline it in `HandleTopLevelDecl`.
I am not sure I fully understand this comment, since I cannot both move this 
method to the anonymous namespace above and inline it at the same time. I have 
inlined it now, hopefully it looks fine to you.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:105
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, TemplateInstantiations) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(

hokein wrote:
> maybe DropImplicitTemplateTopDecl?
Not sure, since the rest of tests above only use nouns, e.g., "Macros", 
"Inclusion", "Namespace", etc. I've called it "ImplicitTemplates" now, 
hopefully it's fine.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:120
+  };
+  int v = dispatch();
+  )cpp";

hokein wrote:
> this example is a nice example to show the original bug (where we inserted 
> unexpected additional headers) in include-cleaner. 
> 
> The scope of this patch is to filter out the implicit template instantiation, 
> a simple testcase like below could work. (the current testcase is fine as 
> well, up to you)
> 
> ```
> template
> int func(T) { return 0; } 
> auto s = func(1);
> ```
You're right, but IMHO I would rather keep the current example. The current 
example, although simplified, still highly resembles the pattern we've found it 
real code. The snippet above does not anymore. Since it's a bug fix for a real 
pattern, I'd rather keep it as is. Let me know if you disagree.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:123
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots, 
testing::Not(testing::ElementsAre(named("get";
+}

hokein wrote:
> nit: instead doing a negative verification, I'd check the captured root decl, 
> which is (a `MyGetter` CXXRecordDecl, and a `v` var decl), and with a comment 
> saying the implicitly-instantiated method decl is filtered out.
Sure, I have no preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141271

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

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Haojian Wu via Phabricator via cfe-commits

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

thanks looks good!

Please remember to add a `FIX: ` to the commit message.




Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:387
+  private:
+  bool isImplicitTemplateInstantiation(const NamedDecl *D) {   
+return isImplicitTemplateSpecialization(D) ||

VitaNuo wrote:
> hokein wrote:
> > we can move it to the above anonymous namespace as well (it is fine to have 
> > two functions with same name because of C++ overloads). In this case, I'd 
> > probably inline it in `HandleTopLevelDecl`.
> I am not sure I fully understand this comment, since I cannot both move this 
> method to the anonymous namespace above and inline it at the same time. I 
> have inlined it now, hopefully it looks fine to you.
the current code looks good.  (I meant either, not both, sorry for the 
confusion)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141271

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

> The rule does not complain.

Sorry, I'm confused. Could you please explain what you expect this example 
should achieve and what is "the rule"? Thanks!

> So I think that the "resolved" type (size_t) should be use instead of the 
> templated one (std::vector>::size_type).

Note: the "desugared type" I mention is the **FULLY** desugared type, so it 
will achieve `unsigned long` instead of `size_t`. There is a method 
`QualType::getSingleStepDesugaredType()` which may get `size_t`, but I haven't 
had a try. But even though it works, this will still turn `int64_t` into `long`.

BTW, if you think the desugared type is better, which do you think is better, 
to get single step desugared or to get fully desugared?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[clang-tools-extra] de81dc8 - [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Viktoriia Bakalova via cfe-commits


Author: Viktoriia Bakalova
Date: 2023-01-09T12:54:20Z
New Revision: de81dc8fdf2764fb14a3c70e5e845cfd7f3b366c

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

LOG: [include-cleaner] Filter template instantiations from AST roots.

Fix: https://github.com/llvm/llvm-project/issues/59825
Differential Revision: https://reviews.llvm.org/D141271

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/Record.cpp 
b/clang-tools-extra/include-cleaner/lib/Record.cpp
index ab5a2415cbf9..4df65959011c 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,14 @@ bool PragmaIncludes::isPrivate(const FileEntry *FE) const {
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template  bool isImplicitTemplateSpecialization(const Decl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,7 +373,11 @@ std::unique_ptr RecordedAST::record() {
   for (Decl *D : DG) {
 if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation(
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D))
+  continue;
+// FIXME: Filter out certain Obj-C as well.
 Out->Roots.push_back(D);
   }
   return ASTConsumer::HandleTopLevelDecl(DG);

diff  --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 919d6bdc0d68..cc99146c4419 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,29 @@ TEST_F(RecordASTTest, Macros) {
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, ImplicitTemplates) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots,
+  testing::ElementsAre(named("MyGetter"), named("v")));
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;



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

[PATCH] D141271: [include-cleaner] Filter template instantiations from AST roots.

2023-01-09 Thread Viktoriia Bakalova 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 rGde81dc8fdf27: [include-cleaner] Filter template 
instantiations from AST roots. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141271

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,29 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, ImplicitTemplates) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots,
+  testing::ElementsAre(named("MyGetter"), named("v")));
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,14 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template  bool isImplicitTemplateSpecialization(const Decl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,7 +373,11 @@
   for (Decl *D : DG) {
 if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation(
   continue;
-// FIXME: Filter out certain Obj-C and template-related decls.
+if (isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D) ||
+isImplicitTemplateSpecialization(D))
+  continue;
+// FIXME: Filter out certain Obj-C as well.
 Out->Roots.push_back(D);
   }
   return ASTConsumer::HandleTopLevelDecl(DG);


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -101,6 +101,29 @@
   EXPECT_THAT(Recorded.Roots, testing::ElementsAre(named("x")));
 }
 
+// Decl from template instantiation is filtered out from roots.
+TEST_F(RecordASTTest, ImplicitTemplates) {
+  Inputs.ExtraFiles["dispatch.h"] = R"cpp(
+  struct A {
+static constexpr int value = 1;
+  };
+  template 
+  int dispatch() {
+return Getter::template get();
+  }
+  )cpp";
+  Inputs.Code = R"cpp(
+  #include "dispatch.h"  
+  struct MyGetter {
+template  static int get() { return T::value; }
+  };
+  int v = dispatch();
+  )cpp";
+  auto AST = build();
+  EXPECT_THAT(Recorded.Roots,
+  testing::ElementsAre(named("MyGetter"), named("v")));
+}
+
 class RecordPPTest : public ::testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -352,6 +353,14 @@
   return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end();
 }
 
+namespace {
+template  bool isImplicitTemplateSpecialization(const Decl *D) {
+  if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
+}
+} // namespace
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
@@ -364,7 +373,11 @@

[clang-tools-extra] 499bf67 - [include-cleaner] Don't count references to operators as uses

2023-01-09 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2023-01-09T13:59:15+01:00
New Revision: 499bf67208d982948e2580b56a09944a285fee76

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

LOG: [include-cleaner] Don't count references to operators as uses

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

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index f32221018caf..18e6d5e21df0 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -66,6 +66,20 @@ class ASTWalker : public RecursiveASTVisitor {
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+if (!WalkUpFromCXXOperatorCallExpr(S))
+  return false;
+
+// Operators are always ADL extension points, by design references to them
+// doesn't count as uses (generally the type should provide them).
+// Don't traverse the callee.
+
+for (auto *Arg : S->arguments())
+  if (!TraverseStmt(Arg))
+return false;
+return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 report(DRE->getLocation(), DRE->getFoundDecl());
 return true;

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index ca2eb25eceee..c8959e7eb673 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -245,6 +245,16 @@ TEST(WalkAST, ConstructExprs) {
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+   "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+   "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+   "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");



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

[PATCH] D140551: [include-cleaner] Don't count references to operators as uses

2023-01-09 Thread Haojian Wu 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 rG499bf67208d9: [include-cleaner] Don't count references 
to operators as uses (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D140551?vs=487382&id=487388#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140551

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -245,6 +245,16 @@
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+   "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+   "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+   "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -66,6 +66,20 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+if (!WalkUpFromCXXOperatorCallExpr(S))
+  return false;
+
+// Operators are always ADL extension points, by design references to them
+// doesn't count as uses (generally the type should provide them).
+// Don't traverse the callee.
+
+for (auto *Arg : S->arguments())
+  if (!TraverseStmt(Arg))
+return false;
+return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 report(DRE->getLocation(), DRE->getFoundDecl());
 return true;


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -245,6 +245,16 @@
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+   "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+   "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+   "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -66,6 +66,20 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+if (!WalkUpFromCXXOperatorCallExpr(S))
+  return false;
+
+// Operators are always ADL extension points, by design references to them
+// doesn't count as uses (generally the type should provide them).
+// Don't traverse the callee.
+
+for (auto *Arg : S->arguments())
+  if (!TraverseStmt(Arg))
+return false;
+return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 report(DRE->getLocation(), DRE->getFoundDecl());
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

> Sorry, I'm confused. Could you please explain what you expect this example 
> should achieve and what is "the rule"? Thanks!

Ah, I think I understand it. You mean the 
`bugprone-implicit-widening-of-multiplication-result` check doesn't warn 
anything about this example, right?

If so, although I don't understand what you mean by coming up with this 
example, to classify, when I said

> The second one will turn int64_t into long, which I don't have a good feeling 
> about.

the point is that if we choose to use the desugared type,

  int64_t foobar() {
return 1024 * 1024;
  }

will change to

  int64_t foobar() {
return static_cast(1024 * 1024);
    instead of int64_t
  }

Here I think this will break the portability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

2023-01-09 Thread Vincent Hong via Phabricator via cfe-commits

v1nh1shungry added a comment.

> There is a method QualType::getSingleStepDesugaredType() which may get size_t

Just did a quick test, and it doesn't work like this. So I can only get the 
fully desugared type for now. Sorry :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141230: [clang-format-diff.py] give clang-format-diff a job pool (10x speed)

2023-01-09 Thread Sean Maher via Phabricator via cfe-commits

seanptmaher updated this revision to Diff 487407.
seanptmaher added a comment.

Possibly fix the patchset? Sorry I've never used phabricator berfore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141230

Files:
  clang/tools/clang-format/clang-format-diff.py

Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -25,6 +25,7 @@
 
 import argparse
 import difflib
+import os
 import re
 import subprocess
 import sys
@@ -99,46 +100,63 @@
   ['-lines', str(start_line) + ':' + str(end_line)])
 
   # Reformat files containing changes in place.
-  for filename, lines in lines_by_file.items():
-if args.i and args.verbose:
-  print('Formatting {}'.format(filename))
-command = [args.binary, filename]
-if args.i:
-  command.append('-i')
-if args.sort_includes:
-  command.append('-sort-includes')
-command.extend(lines)
-if args.style:
-  command.extend(['-style', args.style])
-if args.fallback_style:
-  command.extend(['-fallback-style', args.fallback_style])
-
+  lbf = list(lines_by_file.items())
+  procs = None
+  try:
+procs = [None for i in range(len(os.sched_getaffinity(0)) * 8)]
+  except AttributeError as e:
+# os.sched_getaffinity isn't defined on all platforms.
+import multiprocessing
 try:
-  p = subprocess.Popen(command,
-   stdout=subprocess.PIPE,
-   stderr=None,
-   stdin=subprocess.PIPE,
-   universal_newlines=True)
-except OSError as e:
-  # Give the user more context when clang-format isn't
-  # found/isn't executable, etc.
-  raise RuntimeError(
-'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
-
-stdout, stderr = p.communicate()
-if p.returncode != 0:
-  sys.exit(p.returncode)
+  procs = [None for i in range(multiprocessing.cpu_count())]
+except NotImplementedError as e:
+  # Fallback to 8 concurrent processes in the case that CPU count cannot be
+  # determined.
+  procs = [None for i in range(8)]
 
-if not args.i:
-  with open(filename) as f:
-code = f.readlines()
-  formatted_code = StringIO(stdout).readlines()
-  diff = difflib.unified_diff(code, formatted_code,
-  filename, filename,
-  '(before formatting)', '(after formatting)')
-  diff_string = ''.join(diff)
-  if len(diff_string) > 0:
-sys.stdout.write(diff_string)
+  while lbf:
+for i, proc in enumerate(procs):
+  if not lbf:
+break
+  if proc is None or proc.poll() is not None:
+if proc is not None:
+  stdout, stderr = proc.communicate()
+  if proc.returncode != 0:
+sys.exit(proc.returncode)
+  if not args.i:
+with open(filename) as f:
+  code = f.readlines()
+formatted_code = StringIO(stdout).readlines()
+diff = difflib.unified_diff(code, formatted_code,
+filename, filename,
+'(before formatting)', '(after formatting)')
+diff_string = ''.join(diff)
+if len(diff_string) > 0:
+  sys.stdout.write(diff_string)
+filename, lines = lbf.pop()
+if args.i and args.verbose:
+  print('Formatting {}'.format(filename))
+command = [args.binary, filename]
+if args.i:
+  command.append('-i')
+if args.sort_includes:
+  command.append('-sort-includes')
+command.extend(lines)
+if args.style:
+  command.extend(['-style', args.style])
+if args.fallback_style:
+  command.extend(['-fallback-style', args.fallback_style])
+try:
+  procs[i] = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=None,
+  stdin=subprocess.PIPE,
+  universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+  'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
 
 if __name__ == '__main__':
   main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

2023-01-09 Thread Jake Egan via Phabricator via cfe-commits

Jake-Egan added a comment.

Hi, this test is failing on AIX, could you take a look please? 
https://lab.llvm.org/buildbot/#/builders/214/builds/5242/steps/6/logs/FAIL__Clang__pr59765-modules-global-ctor-dtor_cppm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140867

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

[PATCH] D141280: [clang][wip] Build UsingType for elaborated type specifiers.

2023-01-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
Herald added a subscriber: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added projects: clang, clang-tools-extra.

This is a hacky prototype to support building UsingType for elaborated
type specifiers:

  namespace ns { class Foo {}; }
  
  using ns::Foo;
  
  // The TypeLoc of `Foo` below should be a ElaboratedTypeLoc with an
  inner UsingTypeLoc rather than the underlying `CXXRecordTypeLoc`
  class Foo foo;

It seems to work, and improves some diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141280

Files:
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/test/SemaCXX/using-decl-1.cpp

Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -177,6 +177,10 @@
 
 struct S {
   friend struct HiddenTag1; // expected-error {{tag type that does not match previous}}
+#if __cplusplus < 201103L
+// expected-warning@-2 {{befriending enumeration type 'struct HiddenTag1' is a C++11 extension}}
+#endif
+
   friend struct HiddenTag2; // expected-note {{conflicting declaration}}
   friend void HiddenFn1(); // expected-error {{cannot befriend target of using declaration}}
   friend void HiddenFn2(); // expected-note {{conflicting declaration}}
Index: clang/test/CXX/drs/dr4xx.cpp
===
--- clang/test/CXX/drs/dr4xx.cpp
+++ clang/test/CXX/drs/dr4xx.cpp
@@ -301,9 +301,8 @@
 struct F;
 struct H;
   }
-  // FIXME: This is ill-formed.
   using N::D;
-  struct dr417::D {}; // expected-warning {{extra qualification}}
+  struct dr417::D {}; // expected-error {{forward declaration of struct cannot}} expected-warning {{extra qualification}}
   using namespace N;
   struct dr417::E {}; // expected-warning {{extra qualification}} expected-error {{no struct named 'E'}}
   struct N::F {};
@@ -311,7 +310,7 @@
   using N::H;
   namespace M {
 struct dr417::G {}; // expected-error {{namespace 'M' does not enclose}}
-struct dr417::H {}; // expected-error {{namespace 'M' does not enclose}}
+struct dr417::H {}; // expected-error {{namespace 'M' does not enclose}} expected-error {{forward declaration of struct cannot have}}
   }
 }
 
Index: clang/test/CXX/drs/dr2xx.cpp
===
--- clang/test/CXX/drs/dr2xx.cpp
+++ clang/test/CXX/drs/dr2xx.cpp
@@ -992,7 +992,7 @@
   }
   struct B::V {}; // expected-error {{no struct named 'V'}}
   struct B::W {};
-  struct B::X {}; // FIXME: ill-formed
+  struct B::X {}; // expected-error {{forward declaration of struct cannot have}}
   enum B::Y e; // ok per dr417
   class B::Z z; // ok per dr417
 
@@ -1009,7 +1009,7 @@
   };
   struct D::V {}; // expected-error {{no struct named 'V'}}
   struct D::W {};
-  struct D::X {}; // FIXME: ill-formed
+  struct D::X {}; // expected-error {{forward declaration of struct cannot have}}
   enum D::Y e2; // ok per dr417
   class D::Z z2; // ok per dr417
 }
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1569,7 +1569,14 @@
   case DeclSpec::TST_union:
   case DeclSpec::TST_struct:
   case DeclSpec::TST_interface: {
-TagDecl *D = dyn_cast_or_null(DS.getRepAsDecl());
+TagDecl *D = nullptr;
+UsingShadowDecl *Using =
+dyn_cast_or_null(DS.getRepAsOriginDecl());
+if (Using)
+  D = dyn_cast_or_null(Using->getTargetDecl());
+else
+  D = dyn_cast_or_null(DS.getRepAsOriginDecl());
+
 if (!D) {
   // This can happen in C++ with ambiguous lookups.
   Result = Context.IntTy;
@@ -1587,6 +1594,8 @@
 
 // TypeQuals handled by caller.
 Result = Context.getTypeDeclType(D);
+if (Using)
+  Result = Context.getUsingType(Using, Result);
 
 // In both C and C++, make an ElaboratedType.
 ElaboratedTypeKeyword Keyword
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6070,7 +6070,6 @@
 
 Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
   D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration);
-
   // Check if we are in an `omp begin/end declare variant` scope. Handle this
   // declaration only if the `bind_to_declaration` extension is set.
   SmallVector Bases;
@@ -16573,7 +16572,7 @@
  SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeRe

[PATCH] D139446: [clangd] Add flag to control #import include insertions

2023-01-09 Thread David Goldman via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG814c0bb31660: [clangd] Add flag to control #import include 
insertions (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139446

Files:
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -264,6 +264,14 @@
 "Never insert #include directives as part of code completion")),
 };
 
+opt ImportInsertions{
+"import-insertions",
+cat(Features),
+desc("If header insertion is enabled, add #import directives when "
+ "accepting code completions or fixing includes in Objective-C code"),
+init(CodeCompleteOptions().ImportInsertions),
+};
+
 opt IncludeCleanerStdlib{
 "include-cleaner-stdlib",
 cat(Features),
@@ -913,6 +921,7 @@
 Opts.CodeComplete.BundleOverloads = CompletionStyle != Detailed;
   Opts.CodeComplete.ShowOrigins = ShowOrigins;
   Opts.CodeComplete.InsertIncludes = HeaderInsertion;
+  Opts.CodeComplete.ImportInsertions = ImportInsertions;
   if (!HeaderInsertionDecorators) {
 Opts.CodeComplete.IncludeIndicator.Insert.clear();
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -70,6 +70,10 @@
 NeverInsert,
   } InsertIncludes = IncludeInsertion::IWYU;
 
+  /// Whether include insertions for Objective-C code should use #import 
instead
+  /// of #include.
+  bool ImportInsertions = false;
+
   /// A visual indicator to prepend to the completion label to indicate whether
   /// completion result would trigger an #include insertion or not.
   struct IncludeInsertionIndicator {


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -264,6 +264,14 @@
 "Never insert #include directives as part of code completion")),
 };
 
+opt ImportInsertions{
+"import-insertions",
+cat(Features),
+desc("If header insertion is enabled, add #import directives when "
+ "accepting code completions or fixing includes in Objective-C code"),
+init(CodeCompleteOptions().ImportInsertions),
+};
+
 opt IncludeCleanerStdlib{
 "include-cleaner-stdlib",
 cat(Features),
@@ -913,6 +921,7 @@
 Opts.CodeComplete.BundleOverloads = CompletionStyle != Detailed;
   Opts.CodeComplete.ShowOrigins = ShowOrigins;
   Opts.CodeComplete.InsertIncludes = HeaderInsertion;
+  Opts.CodeComplete.ImportInsertions = ImportInsertions;
   if (!HeaderInsertionDecorators) {
 Opts.CodeComplete.IncludeIndicator.Insert.clear();
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -70,6 +70,10 @@
 NeverInsert,
   } InsertIncludes = IncludeInsertion::IWYU;
 
+  /// Whether include insertions for Objective-C code should use #import instead
+  /// of #include.
+  bool ImportInsertions = false;
+
   /// A visual indicator to prepend to the completion label to indicate whether
   /// completion result would trigger an #include insertion or not.
   struct IncludeInsertionIndicator {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139458: [clangd] Full support for #import insertions

2023-01-09 Thread David Goldman via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dgoldman marked an inline comment as done.
Closed by commit rG042dd99484d6: [clangd] Full support for #import insertions 
(authored by dgoldman).

Changed prior to commit:
  https://reviews.llvm.org/D139458?vs=486706&id=487418#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139458

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/ASTSignals.cpp
  clang-tools-extra/clangd/ASTSignals.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2736,17 +2736,21 @@
   Sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
   Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000,
   Symbol::Include | Symbol::Import);
-
-  auto Results = completions("Fun^", {Sym}).Completions;
+  CodeCompleteOptions Opts;
+  // Should only take effect in import contexts.
+  Opts.ImportInsertions = true;
+  auto Results = completions("Fun^", {Sym}, Opts).Completions;
   assert(!Results.empty());
   EXPECT_THAT(Results[0],
   AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n")));
 
-  Results = completions("Fun^", {Sym}, {}, "Foo.m").Completions;
+  ASTSignals Signals;
+  Signals.InsertionDirective = Symbol::IncludeDirective::Import;
+  Opts.MainFileSignals = &Signals;
+  Results = completions("Fun^", {Sym}, Opts, "Foo.m").Completions;
   assert(!Results.empty());
-  // TODO: Once #import integration support is done this should be #import.
   EXPECT_THAT(Results[0],
-  AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n")));
+  AllOf(named("Func"), insertIncludeText("#import \"bar.h\"\n")));
 
   Sym.IncludeHeaders[0].SupportedDirectives = Symbol::Import;
   Results = completions("Fun^", {Sym}).Completions;
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -11,6 +11,7 @@
 #include "Annotations.h"
 #include "ParsedAST.h"
 #include "TestTU.h"
+#include "index/Symbol.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
@@ -618,6 +619,45 @@
   hasReservedScope(*findUnqualifiedDecl(AST, "secret").getDeclContext()));
 }
 
+TEST(ClangdAST, PreferredIncludeDirective) {
+  auto ComputePreferredDirective = [](TestTU &TU) {
+auto AST = TU.build();
+return preferredIncludeDirective(AST.tuPath(), AST.getLangOpts(),
+ AST.getIncludeStructure().MainFileIncludes,
+ AST.getLocalTopLevelDecls());
+  };
+  TestTU ObjCTU = TestTU::withCode(R"cpp(
+  int main() {}
+  )cpp");
+  ObjCTU.Filename = "TestTU.m";
+  EXPECT_EQ(ComputePreferredDirective(ObjCTU),
+Symbol::IncludeDirective::Import);
+
+  TestTU HeaderTU = TestTU::withCode(R"cpp(
+  #import "TestTU.h"
+  )cpp");
+  HeaderTU.Filename = "TestTUHeader.h";
+  HeaderTU.ExtraArgs = {"-xobjective-c++-header"};
+  EXPECT_EQ(ComputePreferredDirective(HeaderTU),
+Symbol::IncludeDirective::Import);
+
+  // ObjC language option is not enough for headers.
+  HeaderTU.Code = R"cpp(
+  #include "TestTU.h"
+  )cpp";
+  EXPECT_EQ(ComputePreferredDirective(HeaderTU),
+Symbol::IncludeDirective::Include);
+
+  HeaderTU.Code = R"cpp(
+  @interface Foo
+  @end
+
+  Foo * getFoo();
+  )cpp";
+  EXPECT_EQ(ComputePreferredDirective(HeaderTU),
+Symbol::IncludeDirective::Import);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
@@ -68,6 +68,7 @@
   Pair(sym("Y", index::SymbolKind::Variable, "@N@tar@S@X@FI@\\0").ID,
2),
   Pair(_ /*a*/, 3)));
+  EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDir

[clang-tools-extra] 814c0bb - [clangd] Add flag to control #import include insertions

2023-01-09 Thread David Goldman via cfe-commits


Author: David Goldman
Date: 2023-01-09T09:48:29-05:00
New Revision: 814c0bb31660b2441c9a9a6eeaafc2e33c416842

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

LOG: [clangd] Add flag to control #import include insertions

This will be disabled by default, hopefully we can enable for the next
major release.

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.h 
b/clang-tools-extra/clangd/CodeComplete.h
index 44482d1047294..a7c1ae95dcbf4 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -70,6 +70,10 @@ struct CodeCompleteOptions {
 NeverInsert,
   } InsertIncludes = IncludeInsertion::IWYU;
 
+  /// Whether include insertions for Objective-C code should use #import 
instead
+  /// of #include.
+  bool ImportInsertions = false;
+
   /// A visual indicator to prepend to the completion label to indicate whether
   /// completion result would trigger an #include insertion or not.
   struct IncludeInsertionIndicator {

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 4cdfb1a8b4449..06cfdcc1ca233 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -264,6 +264,14 @@ opt HeaderInsertion{
 "Never insert #include directives as part of code completion")),
 };
 
+opt ImportInsertions{
+"import-insertions",
+cat(Features),
+desc("If header insertion is enabled, add #import directives when "
+ "accepting code completions or fixing includes in Objective-C code"),
+init(CodeCompleteOptions().ImportInsertions),
+};
+
 opt IncludeCleanerStdlib{
 "include-cleaner-stdlib",
 cat(Features),
@@ -913,6 +921,7 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
 Opts.CodeComplete.BundleOverloads = CompletionStyle != Detailed;
   Opts.CodeComplete.ShowOrigins = ShowOrigins;
   Opts.CodeComplete.InsertIncludes = HeaderInsertion;
+  Opts.CodeComplete.ImportInsertions = ImportInsertions;
   if (!HeaderInsertionDecorators) {
 Opts.CodeComplete.IncludeIndicator.Insert.clear();
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();



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

[clang-tools-extra] 042dd99 - [clangd] Full support for #import insertions

2023-01-09 Thread David Goldman via cfe-commits


Author: David Goldman
Date: 2023-01-09T09:48:30-05:00
New Revision: 042dd99484d6f393cc8a365def250e9d74c24d37

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

LOG: [clangd] Full support for #import insertions

These are still disabled by default, but will work in ObjC code if you
enable the `-import-insertions` flag.

Completion requires ASTSignals to be available; before ASTSignals are
available, we will always use #include. Once they are available, the
behavior varies as follows:

- For source files, use #import if the ObjC language flag is enabled
- For header files:
  - If the ObjC language flag is disabled, use #include
  - If the header file contains any #imports, use #import
  - If the header file references any ObjC decls, use #import
  - Otherwise, use #include

IncludeFixer support is similar, but it does not rely upon ASTSignals,
instead it does the above checks excluding the scan for ObjC symbols.

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/ASTSignals.cpp
clang-tools-extra/clangd/ASTSignals.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/IncludeFixer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
clang-tools-extra/clangd/unittests/ASTTests.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index ab4cf4e7a82f4..17c4dbf6dbe70 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -376,6 +376,38 @@ const ObjCImplDecl *getCorrespondingObjCImpl(const 
ObjCContainerDecl *D) {
   return nullptr;
 }
 
+Symbol::IncludeDirective
+preferredIncludeDirective(llvm::StringRef FileName, const LangOptions 
&LangOpts,
+  ArrayRef MainFileIncludes,
+  ArrayRef TopLevelDecls) {
+  // Always prefer #include for non-ObjC code.
+  if (!LangOpts.ObjC)
+return Symbol::IncludeDirective::Include;
+  // If this is not a header file and has ObjC set as the language, prefer
+  // #import.
+  if (!isHeaderFile(FileName, LangOpts))
+return Symbol::IncludeDirective::Import;
+
+  // Headers lack proper compile flags most of the time, so we might treat a
+  // header as ObjC accidentally. Perform some extra checks to make sure this
+  // works.
+
+  // Any file with a #import, should keep #import-ing.
+  for (auto &Inc : MainFileIncludes)
+if (Inc.Directive == tok::pp_import)
+  return Symbol::IncludeDirective::Import;
+
+  // Any file declaring an ObjC decl should also be #import-ing.
+  // No need to look over the references, as the file doesn't have any 
#imports,
+  // it must be declaring interesting ObjC-like decls.
+  for (const Decl *D : TopLevelDecls)
+if (isa(
+D))
+  return Symbol::IncludeDirective::Import;
+
+  return Symbol::IncludeDirective::Include;
+}
+
 std::string printType(const QualType QT, const DeclContext &CurContext,
   const llvm::StringRef Placeholder) {
   std::string Result;

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index bdc0862e96347..fb0722d697cd0 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -13,6 +13,8 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
 
+#include "Headers.h"
+#include "index/Symbol.h"
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
@@ -118,6 +120,18 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, 
const MacroInfo *MI,
 /// return nullptr as protocols don't have an implementation.
 const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D);
 
+/// Infer the include directive to use for the given \p FileName. It aims for
+/// #import for ObjC files and #include for the rest.
+///
+/// - For source files we use LangOpts directly to infer ObjC-ness.
+/// - For header files we also check for symbols declared by the file and
+///   existing include directives, as the language can be set to ObjC++ as a
+///   fallback in the absence of compile flags.
+Symbol::IncludeDirective
+preferredIncludeDirective(llvm::StringRef FileName, const LangOptions 
&LangOpts,
+  ArrayRef MainFileIncludes,
+  ArrayRef TopLe

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

2023-01-09 Thread Evan Smal via Phabricator via cfe-commits

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

This patch improves the diagnostic message "initializer-string for char array 
is too long" 
by specifying an expected array length and by indicating that the initializer 
string implicitly
includes the null terminator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141283

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -239,6 +239,8 @@
 if (StrLength > CAT->getSize().getZExtValue())
   S.Diag(Str->getBeginLoc(),
  diag::err_initializer_string_for_char_array_too_long)
+  << CAT->getSize().getZExtValue()
+  << StrLength
   << Str->getSourceRange();
   } else {
 // C99 6.7.8p14.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5953,7 +5953,7 @@
   "excess elements in char array initializer">,
   InGroup;
 def err_initializer_string_for_char_array_too_long : Error<
-  "initializer-string for char array is too long">;
+  "initializer-string for char array is too long, expected %0 but was %1 
(including the null terminating character)">;
 def ext_initializer_string_for_char_array_too_long : ExtWarn<
   "initializer-string for char array is too long">,
   InGroup;


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -239,6 +239,8 @@
 if (StrLength > CAT->getSize().getZExtValue())
   S.Diag(Str->getBeginLoc(),
  diag::err_initializer_string_for_char_array_too_long)
+  << CAT->getSize().getZExtValue()
+  << StrLength
   << Str->getSourceRange();
   } else {
 // C99 6.7.8p14.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5953,7 +5953,7 @@
   "excess elements in char array initializer">,
   InGroup;
 def err_initializer_string_for_char_array_too_long : Error<
-  "initializer-string for char array is too long">;
+  "initializer-string for char array is too long, expected %0 but was %1 (including the null terminating character)">;
 def ext_initializer_string_for_char_array_too_long : ExtWarn<
   "initializer-string for char array is too long">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

2023-01-09 Thread Francesco Petrogalli via Phabricator via cfe-commits

fpetrogalli added a comment.

Gentle ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-01-09 Thread Guillot Tony via Phabricator via cfe-commits

to268 updated this revision to Diff 487427.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/C/C2x/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1192,7 +1192,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007
-  No
+  Clang 16
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_sizeof_typeof(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  typeof(auto) tpof = 4;  // expected-error {{expected expression}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 'auto'}}
+
+  // FIXME: This should be accepted per C2x 6.5.2.5p4
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+}
+
+void test_array_pointers(void) {
+  double array[3] = { 0 };
+  auto a = array;
+  auto b = &array;
+}
+
+void test_qualifiers(const int y) {
+  const auto a = 12;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa = &a; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = &b;
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+
+}
+
+void test_strings(void) {
+  auto str = "this is a string";
+  // FIXME: This should work for char*
+  auto str2[] = "this is a string"; // expected-error {{str2' declared as array of 'auto'}}
+  auto (str3) = "this is a string";
+  auto (((str4))) = "this is a string";
+}
+
+void test_pointers(void) {
+  auto a = 12;
+  auto *ptr = &a; // expected-error {{cannot declare 'ptr' as an explcit 'auto*' in C2x}}
+  auto *str = "this is a string"; // expected-error {{cannot declare 'str' as an explcit 'auto*' in C2x}}
+  const auto *str2 = "this is a string";  // expected-error {{cannot declare 'str2' as an explcit 'auto*' in C2x}}
+  auto *b = &a;   // expected-error {{cannot declare 'b' as an explcit 'auto*' in C2x}}
+  *b = &a;
+}
+
+void test_scopes(void) {
+  double a = 7;
+  double b = 9;
+  {
+auto a = a * a; // expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}} \
+   expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}}
+  }
+  {
+auto b = a * a;
+auto a = b;
+  }
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,123 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+  auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+int a;
+auto b;   // c2x-error {{'auto' not allowed in struct member}} \
+ c17-error {{type name does not allow storage class to be specified}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+union {
+  char c;
+  auto smth;  // c2x-error {{'auto' not allowed in union member}} \
+ c17-error {{type name does not allow storage class to be specified}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+} u;
+};
+
+enum E : auto { // c2x-error {{'auto' not allowed here}} \
+   c17-error {{expected a type}} \
+   c17-error {{type name does not allow storage class to be specified}}
+  One,
+  Two,
+  Tree,
+};
+
+auto basic_u

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

2023-01-09 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik added a comment.

Thank you for this diagnostic fix, please make sure you update 
`clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp`.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5956
 def err_initializer_string_for_char_array_too_long : Error<
-  "initializer-string for char array is too long">;
+  "initializer-string for char array is too long, expected %0 but was %1 
(including the null terminating character)">;
 def ext_initializer_string_for_char_array_too_long : ExtWarn<

Note. we are allowed to initialize with a string-literal that is smaller than 
the array size so maybe something along the lines of `array size is %0 but 
initializer has size %1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

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

[PATCH] D141287: [clang-format] Inherit RightAlign option across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits

rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D119599  added the ability to align compound 
assignments, right aligning
them in order to line up at the equals sign.
However, that patch didn't account for AlignTokens being called
recursively across scopes, which reset the right justification to be
false in any scope besides the top scope. This meant the compound
assignments were aligned, just not at the right place.
(No tests also ever introduced any scopes)

This patch makes sure to inherit the right justification value, just as
every other parameter is passed on.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141287

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -1136,6 +1136,14 @@
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
+  auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22996,6 +22996,41 @@
   verifyFormat("x = (_Atomic( uint64_t ))&a;", Style);
 }
 
+TEST_F(FormatTest, C11Generic) {
+  verifyFormat("_Generic(x, int: 1, default: 0)");
+  verifyFormat("#define cbrt(X) _Generic((X), float: cbrtf, default: cbrt)(X)");
+  verifyFormat("_Generic(x, const char *: 1, char *const: 16, int: 8);");
+  verifyFormat("_Generic(x, int: f1, const int: f2)();");
+  verifyFormat("_Generic(x, struct A: 1, void (*)(void): 2);");
+
+  verifyFormat("_Generic(x,\n"
+   "float: f,\n"
+   "default: d,\n"
+   "long double: ld,\n"
+   "float _Complex: fc,\n"
+   "double _Complex: dc,\n"
+   "long double _Complex: ldc)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("#define LIMIT_MAX(T)   \\\n"
+   "  _Generic(((T)0), \\\n"
+   "  unsigned int: UINT_MAX,  \\\n"
+   "  unsigned long: ULONG_MAX,\\\n"
+   "  unsigned long long: ULLONG_MAX)",
+   Style);
+  verifyFormat("_Generic(x,\n"
+   "struct A: 1,\n"
+   "void (*)(void): 2);",
+   Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("_Generic(x,\n"
+   "  struct A: 1,\n"
+   "  void (*)(void): 2);",
+   Style);
+}
+
 TEST_F(FormatTest, AmbersandInLamda) {
   // Test case reported in https://bugs.llvm.org/show_bug.cgi?id=41899
   FormatStyle AlignStyle = getLLVMStyle();
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -323,6 +323,10 @@
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.is(TT_RequiresExpressionLParen)) {
   Contexts.back().IsExpression = false;
+} else if (OpeningParen.Previous &&
+   OpeningParen.Previous->is(tok::kw__Generic)) {
+  Contexts.back().ContextType = Context::C11GenericSelection;
+  Contexts.back().IsExpression = true;
 } else if (Line.InPPDirective &&
(!OpeningParen.Previous ||
 !OpeningParen.Previous->is(tok::identifier))) {
@@ -1028,6 +1032,8 @@
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->setType(TT_RangeBasedForLoopColon);
+  } else if (Contexts.back().ContextType == Context::C11GenericSelection) {
+Tok->setType(TT_GenericSelectionColon);
   } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
 Tok->setType(TT_BitFieldColon);
   } else if (Contexts.size() == 1 &

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

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Given that we already have other setters to set global state, I'm less 
concerned about adding another one. I hadn't realized we already introduced the 
dangers here. But we should document the expectation that the call be made 
before creating the index.

In terms of the C API, I think it'd make more sense to name in terms of 
"override" rather than "set", but I don't feel as strongly about it given the 
other setters. In terms of the C++ file system API, I think "override" makes 
the most sense though (we don't have setters to follow the naming convention 
for) because some systems do allow you to set the system directory.

In terms of memory ownership, WDYT of requiring the caller to handle this? 
e.g., calling `set_system_temp_directory_erased_on_reboot` will `strdup` a 
nonnull pointer and `free` the stored pointer when given nullptr.


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] D141287: [clang-format] Inherit RightAlign option across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits

rymiel abandoned this revision.
rymiel added a comment.

I apologize, I managed to really mess up with git somehow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141287

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

[PATCH] D140693: [Driver][RISCV] Adjust the priority between -mcpu, -mtune and -march

2023-01-09 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng planned changes to this revision.
kito-cheng added a comment.

- @craig.topper has suggested we could pass all extension with `-` or `+` to 
neutralize the effect of the `-target-cpu`, that's less intrusive way.
- Add release note to mention the behavior change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140693

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

[PATCH] D141288: [clang-format] Inherit RightAlign options across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits

rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D119599  added the ability to align compound 
assignments, right aligning
them in order to line up at the equals sign.
However, that patch didn't account for AlignTokens being called
recursively across scopes, which reset the right justification to be
false in any scope besides the top scope. This meant the compound
assignments were aligned, just not at the right place.
(No tests also ever introduced any scopes)

This patch makes sure to inherit the right justification value, just as
every other parameter is passed on.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141288

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

2023-01-09 Thread Evan Smal via Phabricator via cfe-commits

evansmal added a comment.

In D141283#4036449 , @shafik wrote:

> Thank you for this diagnostic fix, please make sure you update 
> `clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp`.

I noticed that there are also some tests in: 
//clang/test/SemaCXX/pascal-strings.cpp//, //clang/test/Sema/array-init.c//, 
and //clang/test/Sema/format-strings.c//. I assume these should also be updated?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

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

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

I spoke with @to268 during office hours about the current status of the NB 
comments for this feature in the C committee. For the moment, he's going to 
pause work on this patch until the C committee weighs in on whether `auto` is a 
type specifier or not. The committee meets the week of Jan 23, so we should 
have a better understanding of direction by the week of Jan 30. If the C 
committee turns `auto` into a type specifier, we may go back to the original 
implementation of this feature which exposes the C++ feature in C (and add 
diagnostics around the things that need to be constrained in C such as use in a 
function signature).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

2023-01-09 Thread Evan Smal via Phabricator via cfe-commits

evansmal updated this revision to Diff 487443.
evansmal added a comment.

Improve specificity of the diagnostic message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp


Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
@@ -1,2 +1,2 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-char test1[1]="f"; // expected-error {{initializer-string for char array is 
too long}}
+char test1[1]="f"; // expected-error {{initializer-string for char array is 
too long, array size is 1 but initializer has size 2 (including the null 
terminating character)}}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -239,6 +239,8 @@
 if (StrLength > CAT->getSize().getZExtValue())
   S.Diag(Str->getBeginLoc(),
  diag::err_initializer_string_for_char_array_too_long)
+  << CAT->getSize().getZExtValue()
+  << StrLength
   << Str->getSourceRange();
   } else {
 // C99 6.7.8p14.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5953,7 +5953,7 @@
   "excess elements in char array initializer">,
   InGroup;
 def err_initializer_string_for_char_array_too_long : Error<
-  "initializer-string for char array is too long">;
+  "initializer-string for char array is too long, array size is %0 but 
initializer has size %1 (including the null terminating character)">;
 def ext_initializer_string_for_char_array_too_long : ExtWarn<
   "initializer-string for char array is too long">,
   InGroup;


Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp
@@ -1,2 +1,2 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-char test1[1]="f"; // expected-error {{initializer-string for char array is too long}}
+char test1[1]="f"; // expected-error {{initializer-string for char array is too long, array size is 1 but initializer has size 2 (including the null terminating character)}}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -239,6 +239,8 @@
 if (StrLength > CAT->getSize().getZExtValue())
   S.Diag(Str->getBeginLoc(),
  diag::err_initializer_string_for_char_array_too_long)
+  << CAT->getSize().getZExtValue()
+  << StrLength
   << Str->getSourceRange();
   } else {
 // C99 6.7.8p14.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5953,7 +5953,7 @@
   "excess elements in char array initializer">,
   InGroup;
 def err_initializer_string_for_char_array_too_long : Error<
-  "initializer-string for char array is too long">;
+  "initializer-string for char array is too long, array size is %0 but initializer has size %1 (including the null terminating character)">;
 def ext_initializer_string_for_char_array_too_long : ExtWarn<
   "initializer-string for char array is too long">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a5098e5 - [OpenMP] Fix some tests failing with 'libgomp' as the default library

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


Author: Joseph Huber
Date: 2023-01-09T10:03:19-06:00
New Revision: a5098e5f27badc3ba16533418accd2e17641e4e4

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

LOG: [OpenMP] Fix some tests failing with 'libgomp' as the default library

Summary:
There's some static checks on the library, we can't do offloading with
`libgomp` for OpenMP. This patch specifies the library for the tests to
avoid this breaking tests.

Added: 


Modified: 
clang/test/Driver/amdgpu-openmp-system-arch-fail.c
clang/test/Driver/amdgpu-openmp-toolchain.c
clang/test/Driver/hip-options.hip
clang/test/Driver/openmp-offload-gpu.c
clang/test/Driver/openmp-offload-headers.c
clang/test/Driver/openmp-offload-infer.c
clang/test/Driver/openmp-offload-jit.c

Removed: 




diff  --git a/clang/test/Driver/amdgpu-openmp-system-arch-fail.c 
b/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
index 74d736618..20e1dcd8b4171 100644
--- a/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
+++ b/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
@@ -13,16 +13,16 @@
 // RUN: chmod +x %t/amdgpu_arch_empty
 
 // case when amdgpu_arch returns nothing or fails
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib 
--amdgpu-arch-tool=%t/amdgpu_arch_fail %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib 
--amdgpu-arch-tool=%t/amdgpu_arch_fail %s 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=NO-OUTPUT-ERROR
 // NO-OUTPUT-ERROR: error: cannot determine AMDGPU architecture{{.*}}Exited 
with error code 1; consider passing it via '--march'
 
 // case when amdgpu_arch returns multiple gpus but all are 
diff erent
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib --amdgpu-arch-tool=%t/amdgpu_arch_
diff erent %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib --amdgpu-arch-tool=%t/amdgpu_arch_
diff erent %s 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=MULTIPLE-OUTPUT-ERROR
 // MULTIPLE-OUTPUT-ERROR: error: cannot determine AMDGPU architecture: 
Multiple AMD GPUs found with 
diff erent archs; consider passing it via '--march'
 
 // case when amdgpu_arch does not return anything with successful execution
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib 
--amdgpu-arch-tool=%t/amdgpu_arch_empty %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
-fopenmp-targets=amdgcn-amd-amdhsa -nogpulib 
--amdgpu-arch-tool=%t/amdgpu_arch_empty %s 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=EMPTY-OUTPUT
 // EMPTY-OUTPUT: error: cannot determine AMDGPU architecture: No AMD GPU 
detected in the system; consider passing it via '--march'

diff  --git a/clang/test/Driver/amdgpu-openmp-toolchain.c 
b/clang/test/Driver/amdgpu-openmp-toolchain.c
index 288122c89bf0d..ea713b6537de8 100644
--- a/clang/test/Driver/amdgpu-openmp-toolchain.c
+++ b/clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -1,9 +1,9 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
-fopenmp-targets=amdgcn-amd-amdhsa \
 // RUN:  -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 
--libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
-fopenmp-targets=amdgcn-amd-amdhsa \
 // RUN:  --offload-arch=gfx906 
--libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
 
@@ -13,7 +13,7 @@
 // CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
 // CHECK: clang-linker-wrapper{{.*}}"--"{{.*}} "-o" "a.out"
 
-// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa 
-march=gfx906 %s 2>&1 \
+// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu 
-fopenmp=libomp -fopenmp-targets=amdgcn-amd-amdhsa 
-Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PHASES %s
 // CHECK-PHASES: 0: input, "[[INPUT:.+]]", c, (host-openmp)
 // CHECK-PHASES: 1: preprocessor, {0}, cpp-output, (host-openmp)
@@ -32,29 +32,29 @@
 // CHECK-PHASES: 14: clang-linker-wrapper, {13}, image, (host-openmp)
 
 // handling of --libomp

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

2023-01-09 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added a comment.

In D141158#4035875 , @gribozavr2 
wrote:

> The newly added test fails when `tools/clang/include/clang/Config/config.h` 
> is configured with `#define CLANG_DEFAULT_OPENMP_RUNTIME "libgomp"`. Please 
> fix. Thanks!

Should be fixed now, let me know if there's anything else failing.


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] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Mostly just nits from me at this point.




Comment at: clang/lib/AST/DeclCXX.cpp:3232
+VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
+  assert((isa(this) || isa(this)) &&
+ "expected a VarDecl or a BindingDecl");





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:125
+diag::err_param_default_argument_references_local)
+ << D->getDeclName() << DefaultArg->getSourceRange();
   }

`ValueDecl` is a subclass of `NamedDecl` and the diagnostic printer knows how 
to properly print those -- I assume this is an NFC change (or near enough to 
it).



Comment at: clang/lib/Sema/SemaExpr.cpp:19648-19649
 // odr-used, but we may still need to track them for lambda capture.
 // FIXME: Do we also need to do this inside dependent typeid expressions
 // (which are modeled as unevaluated at this point)?
+DoMarkPotentialCapture(SemaRef, Loc, Var, E);

Should this comment be moved elsewhere, as it now seems detached from the 
original logic.



Comment at: clang/lib/Sema/TreeTransform.h:13326-13327
 // Transform the captured variable.
-VarDecl *CapturedVar
-  = cast_or_null(getDerived().TransformDecl(C->getLocation(),
- C->getCapturedVar()));
+ValueDecl *CapturedVar = cast_or_null(
+getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
 if (!CapturedVar || CapturedVar->isInvalidDecl()) {





Comment at: clang/test/SemaCXX/cxx20-decomposition.cpp:96
 
+
 template 

Spurious whitespace?



Comment at: clang/test/SemaCXX/cxx20-decomposition.cpp:160
+return a;
+}() ; }(0);
+(void)[&](auto c) { return b + [&a](auto) {

Same edit elsewhere. Did clang-format get confused?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:3232
+VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
+  assert((isa(this) || isa(this)) &&
+ "expected a VarDecl or a BindingDecl");

aaron.ballman wrote:
> 
That's future tech, `assert` would part that as 2 macros arguments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:3232
+VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
+  assert((isa(this) || isa(this)) &&
+ "expected a VarDecl or a BindingDecl");

cor3ntin wrote:
> aaron.ballman wrote:
> > 
> That's future tech, `assert` would part that as 2 macros arguments!
Ugh.

`(isa(this)) && ...`

should suppress that problem, right? I don't feel strongly though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

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

Address Aaron's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaLambda.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/cxx20-decomposition.cpp

Index: clang/test/SemaCXX/cxx20-decomposition.cpp
===
--- clang/test/SemaCXX/cxx20-decomposition.cpp
+++ clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wunused-variable %s
 
 template 
 constexpr bool is_same = false;
@@ -80,7 +79,17 @@
 namespace std {
 
 template 
-struct tuple_size {
+struct tuple_size;
+
+template 
+struct tuple_size : tuple_size{};
+
+template 
+requires requires { tuple_size::value; }
+struct tuple_size : tuple_size{};
+
+template <>
+struct tuple_size {
   static constexpr unsigned long value = 2;
 };
 
@@ -139,3 +148,37 @@
 };
   }
 }
+
+namespace ODRUseTests {
+  struct P { int a; int b; };
+  void GH57826() {
+const auto [a, b] = P{1, 2}; //expected-note 2{{'b' declared here}} \
+ //expected-note 3{{'a' declared here}}
+(void)[&](auto c) { return b + [&a] {
+return a;
+}(); }(0);
+(void)[&](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[=](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [a](auto) {
+return a;
+}(0); }(0);
+(void)[&a](auto c) { return b + [&a](auto) { // expected-error 2{{variable 'b' cannot be implicitly captured}} \
+ // expected-note 2{{lambda expression begins here}} \
+ // expected-note 4{{capture 'b'}}
+return a;
+}(0); }(0); // expected-note {{in instantiation}}
+(void)[&b](auto c) { return b + [](auto) {   // expected-note 3{{lambda expression begins here}} \
+ // expected-note 6{{capture 'a'}} \
+ // expected-note 6{{default capture}} \
+ // expected-note {{in instantiation}}
+return a;  // expected-error 3{{variable 'a' cannot be implicitly captured}}
+}(0); }(0); // expected-note 2{{in instantiation}}
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13399,9 +13399,8 @@
 }
 
 // Transform the captured variable.
-VarDecl *CapturedVar
-  = cast_or_null(getDerived().TransformDecl(C->getLocation(),
- C->getCapturedVar()));
+auto *CapturedVar = cast_or_null(
+getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
 if (!CapturedVar || CapturedVar->isInvalidDecl()) {
   Invalid = true;
   continue;
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -62,7 +62,7 @@
 static inline Optional
 getStackIndexOfNearestEnclosingCaptureReadyLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture) {
+ValueDecl *VarToCapture) {
   // Label failure to capture.
   const Optional NoLambdaIsCaptureReady;
 
@@ -172,7 +172,7 @@
 
 Optional clang::getStackIndexOfNearestEnclosingCaptureCapableLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture, Sema &S) {
+ValueDecl *VarToCapture, Sema &S) {
 
   const Optional NoLambdaIsCaptureCapable;
 
@@ -1232,11 +1232,7 @@
 if (Var->isInvalidDecl())
   continue;
 
-VarDecl *Underlying;
-if (auto *BD = dyn_cast(Var))
-  Underlying = dyn_cast(BD->getDecomposedDecl());
-else
-  Underlying = cast(Var);
+VarDecl *Underlying = Var->getPotentiallyDecomposedVarDecl();
 
 if (!Underlying->hasLocalStorage()) {
   Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8283,7 +8283,7 @@
   // All the potentially captureable variables in the current nested

[PATCH] D136817: [RISCV] Add H extension

2023-01-09 Thread Philip Reames via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136817

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:3232
+VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
+  assert((isa(this) || isa(this)) &&
+ "expected a VarDecl or a BindingDecl");

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > 
> > That's future tech, `assert` would part that as 2 macros arguments!
> Ugh.
> 
> `(isa(this)) && ...`
> 
> should suppress that problem, right? I don't feel strongly though.
Oh right, that works too :)



Comment at: clang/lib/Sema/SemaExpr.cpp:19648-19649
 // odr-used, but we may still need to track them for lambda capture.
 // FIXME: Do we also need to do this inside dependent typeid expressions
 // (which are modeled as unevaluated at this point)?
+DoMarkPotentialCapture(SemaRef, Loc, Var, E);

aaron.ballman wrote:
> Should this comment be moved elsewhere, as it now seems detached from the 
> original logic.
I think it's still relevant. the logic is still doing lambda captures, and we 
still don't need to do anything else, and the fixme is not resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

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

Rewrite assert in DeclCXX.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaLambda.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/cxx20-decomposition.cpp

Index: clang/test/SemaCXX/cxx20-decomposition.cpp
===
--- clang/test/SemaCXX/cxx20-decomposition.cpp
+++ clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wunused-variable %s
 
 template 
 constexpr bool is_same = false;
@@ -80,7 +79,17 @@
 namespace std {
 
 template 
-struct tuple_size {
+struct tuple_size;
+
+template 
+struct tuple_size : tuple_size{};
+
+template 
+requires requires { tuple_size::value; }
+struct tuple_size : tuple_size{};
+
+template <>
+struct tuple_size {
   static constexpr unsigned long value = 2;
 };
 
@@ -139,3 +148,37 @@
 };
   }
 }
+
+namespace ODRUseTests {
+  struct P { int a; int b; };
+  void GH57826() {
+const auto [a, b] = P{1, 2}; //expected-note 2{{'b' declared here}} \
+ //expected-note 3{{'a' declared here}}
+(void)[&](auto c) { return b + [&a] {
+return a;
+}(); }(0);
+(void)[&](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[=](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [a](auto) {
+return a;
+}(0); }(0);
+(void)[&a](auto c) { return b + [&a](auto) { // expected-error 2{{variable 'b' cannot be implicitly captured}} \
+ // expected-note 2{{lambda expression begins here}} \
+ // expected-note 4{{capture 'b'}}
+return a;
+}(0); }(0); // expected-note {{in instantiation}}
+(void)[&b](auto c) { return b + [](auto) {   // expected-note 3{{lambda expression begins here}} \
+ // expected-note 6{{capture 'a'}} \
+ // expected-note 6{{default capture}} \
+ // expected-note {{in instantiation}}
+return a;  // expected-error 3{{variable 'a' cannot be implicitly captured}}
+}(0); }(0); // expected-note 2{{in instantiation}}
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13399,9 +13399,8 @@
 }
 
 // Transform the captured variable.
-VarDecl *CapturedVar
-  = cast_or_null(getDerived().TransformDecl(C->getLocation(),
- C->getCapturedVar()));
+auto *CapturedVar = cast_or_null(
+getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
 if (!CapturedVar || CapturedVar->isInvalidDecl()) {
   Invalid = true;
   continue;
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -62,7 +62,7 @@
 static inline Optional
 getStackIndexOfNearestEnclosingCaptureReadyLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture) {
+ValueDecl *VarToCapture) {
   // Label failure to capture.
   const Optional NoLambdaIsCaptureReady;
 
@@ -172,7 +172,7 @@
 
 Optional clang::getStackIndexOfNearestEnclosingCaptureCapableLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture, Sema &S) {
+ValueDecl *VarToCapture, Sema &S) {
 
   const Optional NoLambdaIsCaptureCapable;
 
@@ -1232,11 +1232,7 @@
 if (Var->isInvalidDecl())
   continue;
 
-VarDecl *Underlying;
-if (auto *BD = dyn_cast(Var))
-  Underlying = dyn_cast(BD->getDecomposedDecl());
-else
-  Underlying = cast(Var);
+VarDecl *Underlying = Var->getPotentiallyDecomposedVarDecl();
 
 if (!Underlying->hasLocalStorage()) {
   Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8283,7 +8283,7 @@
   // All the potentially captureable variables in the current nest

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits

rymiel added inline comments.



Comment at: clang/test/SemaCXX/cxx20-decomposition.cpp:160
+return a;
+}() ; }(0);
+(void)[&](auto c) { return b + [&a](auto) {

aaron.ballman wrote:
> Same edit elsewhere. Did clang-format get confused?
It can't be clang-format because it would figuratively explode due to 
https://github.com/llvm/llvm-project/issues/40694! (but it did remind me to 
investigate that issue)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

___
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-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Thank you for the fix, please be sure to add a release note for the fix as 
well. Also, I'd like to see some additional C++ test coverage for: 
`alignas(int) struct S {};` to demonstrate we still suggest moving the keyword 
in that case.




Comment at: clang/lib/Sema/SemaDecl.cpp:5296
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.

I think this isn't quite right -- it'll return true in C++ for `alignas`: 
`alignas(int) struct S {};` will give the wrong diagnostic now, I suspect. You 
need to check the language mode as well to ensure we're not in C++ mode.



Comment at: clang/lib/Sema/SemaDecl.cpp:5299-5304
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }





Comment at: clang/lib/Sema/SemaDecl.cpp:5312-5317
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }

Pretty sure you can use something like this instead now.



Comment at: clang/test/C/drs/dr4xx.c:167
 
- /* FIXME: The diagnostic in this case is really bad; moving the specifier to
-  * where the diagnostic recommends causes a different, more inscrutable error
-  * about anonymous structures.
-  */
-  _Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is 
ignored, place it after "struct" to apply attribute to type declaration}} */
+  _Alignas(int) struct T { /* expected-warning {{1 attribute ignored}} */
 int i;

This diagnostic is incorrect but should be fixed with the suggested edit above.



Comment at: clang/test/Parser/c1x-alignas.c:12-13
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension

Same here.


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

https://reviews.llvm.org/D141177

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:125
+diag::err_param_default_argument_references_local)
+ << D->getDeclName() << DefaultArg->getSourceRange();
   }

aaron.ballman wrote:
> `ValueDecl` is a subclass of `NamedDecl` and the diagnostic printer knows how 
> to properly print those -- I assume this is an NFC change (or near enough to 
> it).
Did this one not work the way I expected?



Comment at: clang/lib/Sema/SemaExpr.cpp:19648-19649
 // odr-used, but we may still need to track them for lambda capture.
 // FIXME: Do we also need to do this inside dependent typeid expressions
 // (which are modeled as unevaluated at this point)?
+DoMarkPotentialCapture(SemaRef, Loc, Var, E);

cor3ntin wrote:
> aaron.ballman wrote:
> > Should this comment be moved elsewhere, as it now seems detached from the 
> > original logic.
> I think it's still relevant. the logic is still doing lambda captures, and we 
> still don't need to do anything else, and the fixme is not resolved.
Okay, fine by me, I thought it might make more sense in 
`DoMarkPotentialCapture`, but I see now that the comment relates more to where 
it is currently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:125
+diag::err_param_default_argument_references_local)
+ << D->getDeclName() << DefaultArg->getSourceRange();
   }

aaron.ballman wrote:
> aaron.ballman wrote:
> > `ValueDecl` is a subclass of `NamedDecl` and the diagnostic printer knows 
> > how to properly print those -- I assume this is an NFC change (or near 
> > enough to it).
> Did this one not work the way I expected?
I'm still running the tests, It does seem fine :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits

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

Symplify printing a NamedDecl in moved code per Aaron's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaLambda.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/cxx20-decomposition.cpp

Index: clang/test/SemaCXX/cxx20-decomposition.cpp
===
--- clang/test/SemaCXX/cxx20-decomposition.cpp
+++ clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wunused-variable %s
 
 template 
 constexpr bool is_same = false;
@@ -80,7 +79,17 @@
 namespace std {
 
 template 
-struct tuple_size {
+struct tuple_size;
+
+template 
+struct tuple_size : tuple_size{};
+
+template 
+requires requires { tuple_size::value; }
+struct tuple_size : tuple_size{};
+
+template <>
+struct tuple_size {
   static constexpr unsigned long value = 2;
 };
 
@@ -139,3 +148,37 @@
 };
   }
 }
+
+namespace ODRUseTests {
+  struct P { int a; int b; };
+  void GH57826() {
+const auto [a, b] = P{1, 2}; //expected-note 2{{'b' declared here}} \
+ //expected-note 3{{'a' declared here}}
+(void)[&](auto c) { return b + [&a] {
+return a;
+}(); }(0);
+(void)[&](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[=](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [&a](auto) {
+return a;
+}(0); }(0);
+(void)[&a,&b](auto c) { return b + [a](auto) {
+return a;
+}(0); }(0);
+(void)[&a](auto c) { return b + [&a](auto) { // expected-error 2{{variable 'b' cannot be implicitly captured}} \
+ // expected-note 2{{lambda expression begins here}} \
+ // expected-note 4{{capture 'b'}}
+return a;
+}(0); }(0); // expected-note {{in instantiation}}
+(void)[&b](auto c) { return b + [](auto) {   // expected-note 3{{lambda expression begins here}} \
+ // expected-note 6{{capture 'a'}} \
+ // expected-note 6{{default capture}} \
+ // expected-note {{in instantiation}}
+return a;  // expected-error 3{{variable 'a' cannot be implicitly captured}}
+}(0); }(0); // expected-note 2{{in instantiation}}
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13399,9 +13399,8 @@
 }
 
 // Transform the captured variable.
-VarDecl *CapturedVar
-  = cast_or_null(getDerived().TransformDecl(C->getLocation(),
- C->getCapturedVar()));
+auto *CapturedVar = cast_or_null(
+getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
 if (!CapturedVar || CapturedVar->isInvalidDecl()) {
   Invalid = true;
   continue;
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -62,7 +62,7 @@
 static inline Optional
 getStackIndexOfNearestEnclosingCaptureReadyLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture) {
+ValueDecl *VarToCapture) {
   // Label failure to capture.
   const Optional NoLambdaIsCaptureReady;
 
@@ -172,7 +172,7 @@
 
 Optional clang::getStackIndexOfNearestEnclosingCaptureCapableLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture, Sema &S) {
+ValueDecl *VarToCapture, Sema &S) {
 
   const Optional NoLambdaIsCaptureCapable;
 
@@ -1232,11 +1232,7 @@
 if (Var->isInvalidDecl())
   continue;
 
-VarDecl *Underlying;
-if (auto *BD = dyn_cast(Var))
-  Underlying = dyn_cast(BD->getDecomposedDecl());
-else
-  Underlying = cast(Var);
+VarDecl *Underlying = Var->getPotentiallyDecomposedVarDecl();
 
 if (!Underlying->hasLocalStorage()) {
   Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8283,7 +8283,7 @@
   // All the potentially capture

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

2023-01-09 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl:6
 // RUN: %clang_cc1 -no-opaque-pointers -no-enable-noundef-analysis %s 
-cl-std=CL3.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple 
"spir64-unknown-unknown" | FileCheck %s --check-prefix=COMMON --check-prefix=B64
 // RUN: %clang_cc1 -no-opaque-pointers -no-enable-noundef-analysis %s 
-cl-std=CL3.0 -ffake-address-space-map -O1 -emit-llvm -o - -triple 
"spir64-unknown-unknown" | FileCheck %s --check-prefix=CHECK-LIFETIMES
 

need to add a non-spir target, otherwise we lose coverage for other targets. 



Comment at: clang/test/CodeGenOpenCL/opencl_types.cl:2
 // RUN: %clang_cc1 -no-opaque-pointers -cl-std=CL2.0 %s -triple 
"spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s 
--check-prefixes=CHECK-COM,CHECK-SPIR
 // RUN: %clang_cc1 -no-opaque-pointers -cl-std=CL2.0 %s -triple 
"amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s 
--check-prefixes=CHECK-COM,CHECK-AMDGCN
 

need a non-spir target



Comment at: clang/test/CodeGenOpenCL/sampler.cl:2
-// RUN: %clang_cc1 -no-opaque-pointers %s -emit-llvm -triple 
spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=CL2.0 -emit-llvm -triple 
spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=clc++ -emit-llvm -triple 
spir-unknown-unknown -o - -O0 | FileCheck %s

need a non-spir target


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] D139534: [analyzer] Don't escape local static memregions on bind

2023-01-09 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D139534#4034719 , @xazax.hun wrote:

>> Here is the gist of one *new* TP:
>
> Where would `sprops` get escaped? Did I miss that or was that reduced out of 
> the example?

You are right, it 'never' escapes, yet in the past we modelled all stores to 
local statics as an 'immediate escape'.
This is what I think we should not do. And this is what this patch removes.

> Overall, this looks like a hard nut to crack. Escaping too much or too little 
> are both problematic, and we don't have the information we need to make the 
> decision. The question is whether we want to make an absolute decision or 
> come up with a heuristic like:
>
>   static int* p;
>   MyStruct reachable(&p);
>   
>   indirect(&reachable);
>   direct(&p);
>
> escaping when `direct` is called, but not escaping when `indirect` is called.
>
> Do you see any patterns in the real-world results that would show a pattern? 
> I am not opposed to making a change, but I wonder if we should start 
> documenting these decisions somewhere that are likely need revision in the 
> future when we have more data.  What do you think?

I've seen only those 3 diffs: 2 new 1 absent issues. But there could be 
projects which make use of local static variables a lot. It was the case with 
one of our customers, but I cannot comment on that. I could somehow find 
open-source projects affected, but I'm not sure if it would be easy unless you 
have projects in mind.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139534

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

[PATCH] D141297: [OpenCL] Allow undefining header-only features

2023-01-09 Thread Sven van Haastregt via Phabricator via cfe-commits

svenvh created this revision.
svenvh added reviewers: Anastasia, FMarno.
Herald added subscribers: Naghasan, ldrumm, yaxunl.
Herald added a project: All.
svenvh requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`opencl-c-base.h` always defines 5 particular feature macros for
SPIR-V, making it impossible to disable those features.

To allow disabling any of those features, let the header recognize
`__undef_` macros.  The user can then pass the
`-D__undef_` flag on the command line to disable a specific
feature.  The `__undef` macro could potentially also be set from
`-cl-ext=-feature`, but for now only change the header and only
provide `__undef` macros for the 5 features that are always enabled in
`opencl-c-base.h`.

This is an alternative to https://reviews.llvm.org/D137652


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141297

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/test/SemaOpenCL/features.cl


Index: clang/test/SemaOpenCL/features.cl
===
--- clang/test/SemaOpenCL/features.cl
+++ clang/test/SemaOpenCL/features.cl
@@ -26,6 +26,15 @@
 // RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl 
-cl-std=clc++1.0 \
 // RUN:   | FileCheck -match-full-lines %s  --check-prefix=NO-FEATURES
 
+// For OpenCL C 3.0, header-only features can be disabled using macros.
+// RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl 
-cl-std=CL3.0 -fdeclare-opencl-builtins -finclude-default-header \
+// RUN:-D__undef___opencl_c_work_group_collective_functions=1 \
+// RUN:-D__undef___opencl_c_atomic_order_seq_cst=1 \
+// RUN:-D__undef___opencl_c_atomic_scope_device=1 \
+// RUN:-D__undef___opencl_c_atomic_scope_all_devices=1 \
+// RUN:-D__undef___opencl_c_read_write_images=1 \
+// RUN:   | FileCheck %s --check-prefix=NO-HEADERONLY-FEATURES
+
 // Note that __opencl_c_int64 is always defined assuming
 // always compiling for FULL OpenCL profile
 
@@ -43,14 +52,20 @@
 // FEATURES: #define __opencl_c_subgroups 1
 
 // NO-FEATURES: #define __opencl_c_int64 1
-// NO-FEATURES-NOT: __opencl_c_3d_image_writes
-// NO-FEATURES-NOT: __opencl_c_atomic_order_acq_rel
-// NO-FEATURES-NOT: __opencl_c_atomic_order_seq_cst
-// NO-FEATURES-NOT: __opencl_c_device_enqueue
-// NO-FEATURES-NOT: __opencl_c_fp64
-// NO-FEATURES-NOT: __opencl_c_generic_address_space
-// NO-FEATURES-NOT: __opencl_c_images
-// NO-FEATURES-NOT: __opencl_c_pipes
-// NO-FEATURES-NOT: __opencl_c_program_scope_global_variables
-// NO-FEATURES-NOT: __opencl_c_read_write_images
-// NO-FEATURES-NOT: __opencl_c_subgroups
+// NO-FEATURES-NOT: #define __opencl_c_3d_image_writes
+// NO-FEATURES-NOT: #define __opencl_c_atomic_order_acq_rel
+// NO-FEATURES-NOT: #define __opencl_c_atomic_order_seq_cst
+// NO-FEATURES-NOT: #define __opencl_c_device_enqueue
+// NO-FEATURES-NOT: #define __opencl_c_fp64
+// NO-FEATURES-NOT: #define __opencl_c_generic_address_space
+// NO-FEATURES-NOT: #define __opencl_c_images
+// NO-FEATURES-NOT: #define __opencl_c_pipes
+// NO-FEATURES-NOT: #define __opencl_c_program_scope_global_variables
+// NO-FEATURES-NOT: #define __opencl_c_read_write_images
+// NO-FEATURES-NOT: #define __opencl_c_subgroups
+
+// NO-HEADERONLY-FEATURES-NOT: #define 
__opencl_c_work_group_collective_functions
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_order_seq_cst
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_scope_device
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_scope_all_devices
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_read_write_images
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -74,6 +74,25 @@
 #define __opencl_c_atomic_scope_all_devices 1
 #define __opencl_c_read_write_images 1
 #endif // defined(__SPIR__)
+
+// Undefine any feature macros that have been explicitly disabled using
+// an __undef_ macro.
+#ifdef __undef___opencl_c_work_group_collective_functions
+#undef __opencl_c_work_group_collective_functions
+#endif
+#ifdef __undef___opencl_c_atomic_order_seq_cst
+#undef __opencl_c_atomic_order_seq_cst
+#endif
+#ifdef __undef___opencl_c_atomic_scope_device
+#undef __opencl_c_atomic_scope_device
+#endif
+#ifdef __undef___opencl_c_atomic_scope_all_devices
+#undef __opencl_c_atomic_scope_all_devices
+#endif
+#ifdef __undef___opencl_c_read_write_images
+#undef __opencl_c_read_write_images
+#endif
+
 #endif // (__OPENCL_CPP_VERSION__ == 202100 || __OPENCL_C_VERSION__ == 300)
 
 #if !defined(__opencl_c_generic_address_space)


Index: clang/test/SemaOpenCL/features.cl
===
--- clang/test/SemaOpenCL/features.cl
+++ clang/test/SemaOpenCL/features.cl
@@ -26,6 +26,15 @@
 // RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl -cl

[PATCH] D140086: [analyzer][solver] Improve reasoning for not equal to operator

2023-01-09 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Sorry, I don't have the time this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140086

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

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

2023-01-09 Thread Igor Kushnir via Phabricator via cfe-commits

vedgy marked an inline comment as not done.
vedgy added a comment.

In D139774#4036496 , @aaron.ballman 
wrote:

> In terms of the C API, I think it'd make more sense to name in terms of 
> "override" rather than "set", but I don't feel as strongly about it given the 
> other setters. In terms of the C++ file system API, I think "override" makes 
> the most sense though (we don't have setters to follow the naming convention 
> for) because some systems do allow you to set the system directory.

Let's keep the naming in C and C++ APIs consistent: 
`clang_overrideTemporaryDirectory()` and 
`override_system_temp_directory_erased_on_reboot()`.

> In terms of memory ownership, WDYT of requiring the caller to handle this? 
> e.g., calling `set_system_temp_directory_erased_on_reboot` will `strdup` a 
> nonnull pointer and `free` the stored pointer when given nullptr.

I like this idea. libclang-user code would become easier to use than it is now 
(though less easy compared to libclang managing memory itself). The libclang 
API documentation can require overriding the temp directory before creating an 
index and un-overriding it with `nullptr` after calling `clang_disposeIndex()`.
Now in order to make this libclang API harder to misuse, I lean towards passing 
the temporary directory in `clang_createIndexWithTempDir()` and letting 
`clang_disposeIndex()` handle the un-overriding (call 
`override_system_temp_directory_erased_on_reboot(nullptr)`) automatically. 
Makes sense? I feel that the `clang_createIndexWithTempDir()` name could be 
improved, but don't know how...

What memory [de]allocation method should 
`override_system_temp_directory_erased_on_reboot()` use? `new[]` and 
`delete[]`? Or should `strdup()` from POSIX be used because it is defined as 
`_strdup` on Windows? (along with standard `free()`)

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] D141298: Move from llvm::makeArrayRef to ArrayRef deduction guides - last part

2023-01-09 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.
Herald added subscribers: Michael137, JDevlieghere.

Once that patch lands, I'll mark `makeArrayRef` as deprecated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141298

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

[PATCH] D72103: [Sema] Avoid using an invalid InsertPos

2023-01-09 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik added a comment.
Herald added a project: All.

It looks like the original bug no longer reproduces: 
https://github.com/llvm/llvm-project/issues/42566

Is this PR still needed?


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

https://reviews.llvm.org/D72103

___
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-09 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added a comment.

> Header units have even more need to be involved in the build graph than named 
> units. ODR violations and cache invalidation problems await anyone just 
> winging it on header units (at least that's the understanding I've gotten 
> from SG15 meetings).

I think that latter claim applies equally to all module units. The ODR 
violation and cache invalidation concerns sometimes associated with header 
units occur in implicit module build systems in which a header unit might be 
built multiple times with different sets of options that result in an ODR 
violation. The same problem can occur with other kinds of module units if they 
are built multiple times with different options and then imported by distinct 
TUs that are then linked together. The general rule is, given a set of TUs that 
will be linked together, all imported module units should be built exactly one 
time.


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

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

2023-01-09 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

Please split vmulh* from vmul.c. Remove the vmul tests from the handcrafted 
vmul.c and vmul-eew64.c. Rename those to vmulh.c and vmulh-eew64.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

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

[PATCH] D141298: Move from llvm::makeArrayRef to ArrayRef deduction guides - last part

2023-01-09 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.

Seems mechanical, and if it build everywhere LGTM :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141298

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

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

2023-01-09 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.
Herald added a subscriber: StephenFan.

@nikic: gentle ping


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

https://reviews.llvm.org/D140699

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

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

2023-01-09 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

@nikic : gentle ping :-)


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

https://reviews.llvm.org/D140800

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM assuming precommit CI doesn't discover any problems from the latest 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-09 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added inline comments.



Comment at: clang/test/ClangScanDeps/P1689.cppm:9
+//
+// Check the seperated dependency format.
+// RUN: clang-scan-deps -format=p1689 --p1689-targeted-file-name=%t/M.cppm 
--p1689-targeted-output=%t/M.o \

ben.boeckel wrote:
> jansvoboda11 wrote:
> > What does "separated" mean in this context?
> Yeah, this isn't the right term. There are two things being done:
> 
> - discovering dependencies for a future compile (P1689)
> - collecting deps for the scanning itself to know that "if included file X 
> changes, I need to rescan"
> 
> Both are required for correct builds.
@ChuanqiXu Can you write up a better comment here?



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:571-581
+
+  // FIXME: createInvocation will drop the `-o` option since it requires
+  // `-fsyntax-only`. So here we try to parse the output file manually.
+  auto CommandLineIter =
+  std::find(CommandLine.rbegin(), CommandLine.rend(), StringRef("-o"));
+  if (CommandLineIter == CommandLine.rend() ||
+  --CommandLineIter == CommandLine.rend()) {

ChuanqiXu wrote:
> ChuanqiXu wrote:
> > Here is the corresponding code: 
> > https://github.com/llvm/llvm-project/blob/0e11d65a58da32311b562ecea2b5ba9d4d655659/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp#L39-L43.
> >  There is another FIXME.
> > 
> > Simply, we will lost the output file by using `createInvocation` and I feel 
> > it is not easy to fix. Then I feel it is not too bad to parse `-o` manually 
> > here if we document it clearly.  I understand the current style will miss 
> > some cases like using `-output=`, `-output` instead of `-o` or missing 
> > `-o`. But I don't feel too bad for it.
> `createInvocation` will be helpful to recognize many cases, e.g., `-MF` in 
> https://reviews.llvm.org/D139168. I feel it'll be worse if we don't use 
> `createInvocation `.
This is again doing ad-hoc parsing of the command-line. `clang-cl` accepts 
output file path in the `/o` argument, which this code will not recognize. You 
should be able to use `CompilerInvocation::CreateFromArgs()` to avoid having 
the `-fsyntax-only` option injected.


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

https://reviews.llvm.org/D137534

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

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

2023-01-09 Thread Paul Kirth via Phabricator via cfe-commits

paulkirth added a comment.

@tejohnson @xur I kind of dropped the ball on these patches, but what are your 
thoughts on this approach over the old(more invasive) change to the profdata 
format I had prototyped before? the patch will obviously need to be rebased, 
but other than that, do we see a downside to handling provenance tracking for 
branch weights this way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

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

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-09 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added a comment.

In D137534#4025898 , @ben.boeckel 
wrote:

> - the format supports doing this for a set of object files (but given the way 
> it tangles the dep graph, is unlikely to be a perf win for 
> incremental/developer builds; CI may prefer it, but that can be future work)

Another thing to be aware of is that the scanner is tuned for scanning multiple 
TUs. Single `clang-scan-deps` invocation maintains a shared in-memory cache of 
the filesystem between its threads for all TUs it's given. This means invoking 
`clang-scan-deps` once for each TU is not as efficient as it could be. At 
Apple, we only use `clang-scan-deps` for the in-tree tests. In production, we 
actually wrap the C++ interface and expose a libclang API that is able to take 
advantage of caching to improve performance. To be honest, I'm surprised 
`clang-scan-deps` is being integrated into build systems as-is, especially 
without utilizing the cache. How's the performance looking for larger projects?

> - because it is a rule that can itself read extra files that affect the 
> scanning; this is the `-MF`-style output so that make/ninja can know "oh, 
> frabnitz.h changed, it can affect the scan results in glom.ddi, so I will 
> rescan")

I see. So the P1689  output is the primary 
scanner output, but you're also relying on emitting `.d` files to track the 
actual FS dependencies.

> The object can be obtained from the `-o` on the command line, but the rest is 
> "lying" if it is extracted from the clang command line and not given to 
> `clang-scan-deps` directly.

I see your point. But since `clang-scan-deps` is built around reusing the same 
FS cache for scanning multiple TUs, you'd need to specify these arguments (that 
we currently extract from Clang command line) for all of those TUs. That's not 
very convenient, neither through the command line nor via a separate config 
file.

Comment at: clang/test/ClangScanDeps/P1689.cppm:11
+// RUN: clang-scan-deps -format=p1689 \
+// RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/M.cppm -o 
%t/M.o \
+// RUN:   | FileCheck %t/M.cppm -DPREFIX=%/t

I'm fairly happy with the `clang-scan-deps` interface now (besides the 
performance aspect mentioned in another comment).

@ChuanqiXu what would happen if you run this Clang command line directly?

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

https://reviews.llvm.org/D137534

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

[PATCH] D140547: Perform access checking to private members in simple requirement.

2023-01-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

I think the only major problem is not checking for error case when accessing 
`TransReq`, the rest are NITs.
Thanks for the change! Will be happy to LGTM it as soon as the access to 
`TransReq` is fixed.

Comment at: clang/lib/Parse/ParseExprCXX.cpp:3512
   ParseScope BodyScope(this, Scope::DeclScope);
+  ParsingDeclRAIIObject ParsingBodyDecl(*this, 
ParsingDeclRAIIObject::NoParent);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(

NIT: could you add a comment explaining that we need this helper in order to 
capture dependent diagnostics properly?

Comment at: clang/lib/Sema/SemaConcept.cpp:1005
   } else if (auto *RE = dyn_cast(SubstExpr)) {
+// TODO(usx): Store and diagnose dependent diagnositcs here.
 for (concepts::Requirement *Req : RE->getRequirements())

NIT: `s/Store/RequiresExpr should store dependent diagnostics`. I was confused 
at first and thought we need to store something in this function rather than 
the other place.
NIT2: Use of `FIXME` is more common in LLVM.
NIT3:  Google LDAP `usx` might be trickier to find in LLVM communication 
channels, I suggest removing it completely or using a full name instead.

Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1373
+SemaRef.PerformDependentDiagnostics(E->getBody(), TemplateArgs);
+// TODO(usx): Store SFINAE diagnostics in RequiresExpr for diagnosis.
+if (Trap.hasErrorOccurred())

NIT: LLVM uses FIXME more often. 

Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1375
+if (Trap.hasErrorOccurred())
+  TransReq.getAs()->setSatisfied(false);
+  }

`TransReq` may be `ExprError` and this will cause a crash. Worth adding a test 
too.

Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1375
+if (Trap.hasErrorOccurred())
+  TransReq.getAs()->setSatisfied(false);
+  }

ilya-biryukov wrote:
> `TransReq` may be `ExprError` and this will cause a crash. Worth adding a 
> test too.
Could you please add `assert(!TransReq || *TransReq != E)`.
The common optimization in TreeTransform is to avoid rebuilding the AST nodes 
if nothing changes. There is no optimization like this for `RequireExpr` right 
now, but it would not be unexpected if this gets implemented in the future.

In those situations, the current code can potentially change value of 
`isSatisfied` for an existing expression rather than for a newly created, which 
seems like asking for trouble. It would be nice to give an early warning to 
implementors of this optimization that they should think how to handle this 
case.

Comment at: 
clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp:235
+void test() {
+  // TODO: Propagate diagnostic.
+  Use::foo(); //expected-error {{invalid reference to function 'foo': 
constraints not satisfied}}

NIT: FIXME

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140547

___
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-09 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added inline comments.



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:775
+static llvm::StringMap OSs;
+std::unique_lock LockGuard(Lock);
+

How will this work when a different process tries to write the same file? Could 
we write into a temporary file and then do atomic rename?


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

[PATCH] D141307: [WIP] Add -f[no-]loop-versioning option

2023-01-09 Thread Mats Petersson via Phabricator via cfe-commits

Leporacanthicus created this revision.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
Leporacanthicus requested review of this revision.
Herald added subscribers: cfe-commits, jdoerfert, MaskRay.
Herald added a project: clang.

Posted to satisfy depenences of another patch. Still work in progress.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141307

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CodeGenOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/tools/bbc/bbc.cpp
  flang/tools/tco/tco.cpp

Index: flang/tools/tco/tco.cpp
===
--- flang/tools/tco/tco.cpp
+++ flang/tools/tco/tco.cpp
@@ -122,7 +122,7 @@
   fir::createDefaultFIRCodeGenPassPipeline(pm);
 } else {
   // Run tco with O2 by default.
-  fir::createMLIRToLLVMPassPipeline(pm, llvm::OptimizationLevel::O2);
+  fir::createMLIRToLLVMPassPipeline(pm, false, llvm::OptimizationLevel::O2);
 }
 fir::addLLVMDialectToLLVMPass(pm, out.os());
   }
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -273,7 +273,8 @@
 pm.addPass(std::make_unique());
 
 // Add O2 optimizer pass pipeline.
-fir::createDefaultFIROptimizerPassPipeline(pm, llvm::OptimizationLevel::O2);
+fir::createDefaultFIROptimizerPassPipeline(pm, false,
+   llvm::OptimizationLevel::O2);
   }
 
   if (mlir::succeeded(pm.run(mlirModule))) {
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -541,7 +541,7 @@
   pm.enableVerifier(/*verifyPasses=*/true);
 
   // Create the pass pipeline
-  fir::createMLIRToLLVMPassPipeline(pm, level);
+  fir::createMLIRToLLVMPassPipeline(pm, opts.LoopVersioning, level);
   mlir::applyPassManagerCLOptions(pm);
 
   // run the pass manager
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -126,6 +126,11 @@
clang::driver::options::OPT_fno_debug_pass_manager, false))
 opts.DebugPassManager = 1;
 
+  if (args.hasFlag(clang::driver::options::OPT_floop_versioning,
+   clang::driver::options::OPT_fno_loop_versioning, false)) {
+opts.LoopVersioning = 1;
+  }
+
   for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
 opts.LLVMPassPlugins.push_back(a->getValue());
 
Index: flang/include/flang/Frontend/CodeGenOptions.def
===
--- flang/include/flang/Frontend/CodeGenOptions.def
+++ flang/include/flang/Frontend/CodeGenOptions.def
@@ -24,6 +24,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 
+CODEGENOPT(LoopVersioning, 1, 0) ///< PIC level of the LLVM module.
 CODEGENOPT(PICLevel, 2, 0) ///< PIC level of the LLVM module.
 CODEGENOPT(IsPIE, 1, 0) ///< PIE level is the same as PIC Level.
 
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -57,6 +57,12 @@
options::OPT_fintrinsic_modules_path, options::OPT_pedantic,
options::OPT_std_EQ, options::OPT_W_Joined,
options::OPT_fconvert_EQ, options::OPT_fpass_plugin_EQ});
+  Arg *loopVersioning =
+  Args.getLastArg(options::OPT_Ofast, options::OPT_floop_versioning,
+  options::OPT_fno_loop_versioning);
+  if (loopVersioning &&
+  !loopVersioning->getOption().matches(options::OPT_fno_loop_versioning))
+CmdArgs.push_back("-floop-versioning");
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5052,6 +5052,11 @@
 def fno_automatic : Flag<["-"], "fno-automatic">, Group,
   HelpText<"Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE">;
 
+def floop_versioning : Flag<["-"], "floop-versioning">, Group,
+  HelpText<"Attempt to version loops">;
+def fno_loop_versioning : Flag<["-"], "fno-loop-versioning">, Group,
+  HelpText<"Do not version loops (default)">;
+
 } // let Flags = [FC1Option, FlangOption, FlangOnlyOption]
 
 def J : JoinedOrSeparate<["-"], "J">,
__

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

2023-01-09 Thread Gábor Horváth via Phabricator via cfe-commits

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

In D139534#4036783 , @steakhal wrote:

> In D139534#4034719 , @xazax.hun 
> wrote:
>
>>> Here is the gist of one *new* TP:
>>
>> Where would `sprops` get escaped? Did I miss that or was that reduced out of 
>> the example?
>
> You are right, it 'never' escapes, yet in the past we modelled all stores to 
> local statics as an 'immediate escape'.
> This is what I think we should not do. And this is what this patch removes.

Oh, now I understand. Yeah, I guess the idea was that we only have escape 
information for the current path, but the static's address might have escaped 
in another path that we did not process. Overall, I think those cases should be 
rare, so I do support this change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139534

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

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-09 Thread Ben Boeckel via Phabricator via cfe-commits

ben.boeckel added a comment.

In D137534#4037064 , @jansvoboda11 
wrote:

> Another thing to be aware of is that the scanner is tuned for scanning 
> multiple TUs. Single `clang-scan-deps` invocation maintains a shared 
> in-memory cache of the filesystem between its threads for all TUs it's given. 
> This means invoking `clang-scan-deps` once for each TU is not as efficient as 
> it could be. At Apple, we only use `clang-scan-deps` for the in-tree tests. 
> In production, we actually wrap the C++ interface and expose a libclang API 
> that is able to take advantage of caching to improve performance. To be 
> honest, I'm surprised `clang-scan-deps` is being integrated into build 
> systems as-is, especially without utilizing the cache. How's the performance 
> looking for larger projects?

I originally investigated (and ended up lost) for something like GCC where 
P1689  information is extracted via `-E 
-fdep-file=p1689.json -fdep-output=module.o -fdep-format=p1689` (which is 
"abusing" `-E`, but works), but using `clang-scan-deps` was where we ended up 
after a more-knowledgeable LLVM developer took over knowing what needed to be 
done.

>> - because it is a rule that can itself read extra files that affect the 
>> scanning; this is the `-MF`-style output so that make/ninja can know "oh, 
>> frabnitz.h changed, it can affect the scan results in glom.ddi, so I will 
>> rescan")
>
> I see. So the P1689  output is the primary 
> scanner output, but you're also relying on emitting `.d` files to track the 
> actual FS dependencies.

Yes, that's exactly it. It'd be great if *every* command had `-MF`-style 
information for more accurate builds, but I'll roll that rock up another hill 
some other day.

>> The object can be obtained from the `-o` on the command line, but the rest 
>> is "lying" if it is extracted from the clang command line and not given to 
>> `clang-scan-deps` directly.
>
> I see your point. But since `clang-scan-deps` is built around reusing the 
> same FS cache for scanning multiple TUs, you'd need to specify these 
> arguments (that we currently extract from Clang command line) for all of 
> those TUs. That's not very convenient, neither through the command line nor 
> via a separate config file.

While batch scanning is probably better for one-shot (basically, CI) builds, I 
suspect the excess work during development/incremental builds will cause that 
to "lose" over a long enough time span.


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

https://reviews.llvm.org/D137534

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

[PATCH] D141051: [CUDA][HIP] Add support for `--offload-arch=native` to CUDA and refactor

2023-01-09 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: clang/test/Driver/amdgpu-hip-system-arch.c:24
+
+// case when amdgpu_arch does not return anything with successful execution
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib 
--offload-arch=native --amdgpu-arch-tool=%t/amdgpu_arch_gfx906 -x hip %s 2>&1 \

comment incorrect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141051

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

2023-01-09 Thread Alvin Wong via Phabricator via cfe-commits

alvinhochun added a comment.

The idea sounds reasonable. I don't know mingw-w64 toolchains well enough, but 
I'll try:

How does it interact with the following conditions (lines 469-475)? They look 
like they may be looking for a mingw-w64 sysroot, which may be ignored by the 
new check.

  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(
   getDriver(), LiteralTriple, getTriple(), SubdirName))
Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
  else if (llvm::ErrorOr GPPName =
   findGcc(LiteralTriple, getTriple()))
Base = std::string(llvm::sys::path::parent_path(
llvm::sys::path::parent_path(GPPName.get(;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D141051: [CUDA][HIP] Add support for `--offload-arch=native` to CUDA and refactor

2023-01-09 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added inline comments.



Comment at: clang/test/Driver/amdgpu-hip-system-arch.c:24
+
+// case when amdgpu_arch does not return anything with successful execution
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib 
--offload-arch=native --amdgpu-arch-tool=%t/amdgpu_arch_gfx906 -x hip %s 2>&1 \

yaxunl wrote:
> comment incorrect?
Yes, thanks for catching that. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141051

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

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-01-09 Thread Thorsten via Phabricator via cfe-commits

tschuett added a comment.

It surprised me that there are no type inference messages? The type of this 
auto is double. I only found warnings of misuse of auto and codegen tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:1834-1846
+  void setLambdaTypeInfo(TypeSourceInfo *TS) {
+auto *DD = DefinitionData;
+assert(DD && DD->IsLambda && "setting lambda property of non-lambda 
class");
+auto &DL = static_cast(*DD);
+DL.MethodTyInfo = TS;
+  }
+

Minor simplification



Comment at: clang/include/clang/Sema/Scope.h:148
+/// This is the scope for a lambda, after the lambda introducer.
+/// Lambdas need 2 FunctionPrototypeScope scopes (because there is a
+/// template scope in between), the outer scope does not increase the





Comment at: clang/include/clang/Sema/ScopeInfo.h:854-857
   /// Whether this is a mutable lambda.
-  bool Mutable = false;
+  /// Until the mutable keyword is parsed,
+  /// we assume the lambda is mutable
+  bool Mutable = true;

Re-flow comment (I might have gotten that wrong) and add a full stop at the end 
of the comment.



Comment at: clang/include/clang/Sema/Sema.h:7115-7119
+  /// Once the Lambdas capture are known, we can
+  /// start to create the closure, call operator method,
+  /// and keep track of the captures.
+  /// We do the capture lookup here, but they are not actually captured
+  /// until after we know what the qualifiers of the call operator are.

You can re-flow this comment to 80 col as well.



Comment at: clang/include/clang/Sema/Sema.h:7238
+  /// Introduce the instantiated captures of the lambda into the local
+  /// instantiation scope
+  bool addInstantiatedCapturesToScope(





Comment at: clang/lib/Parse/ParseExprCXX.cpp:1293
   Actions.PushLambdaScope();
+  Actions.ActOnLambdaIntroducer(Intro, getCurScope());
 

Typically, we call an `ActOn` method after having parsed the construct; in this 
case, we're calling `ActOnLambdaIntroducer()` when it was parsed elsewhere 
(this is the parsing code for after the introducer). So perhaps this should be 
moved elsewhere or renamed?



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1381
+  SourceLocation MutableLoc;
+  LateParsedAttrList LateParsedAttrs(true);
+

This isn't being used?



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1387-1393
+// However, because GNU attributes could refer to captured variables,
+// which only become visible after the mutable keyword is parsed
+// we delay the parsing of gnu attributes - by reusing the mechanism used
+// for C++ late method parsing. Note, __declspec attributes do not make
+// use of late parsing (expressions cannot appear in __declspec arguments),
+// so only GNU style attributes are affected here.
+MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, Attributes);

The comment doesn't seem to match the code -- this isn't parsing into the late 
parsed attribute list?



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1521-1523
+  if (HasParentheses || HasSpecifiers) {
+ParseConstexprAndMutableSpecifiers();
   }





Comment at: clang/lib/Parse/ParseExprCXX.cpp:1527-1529
+  if (!HasParentheses) {
+Actions.ActOnLambdaClosureQualifiers(Intro, MutableLoc);
+  }





Comment at: clang/lib/Sema/SemaConcept.cpp:503-505
+  const CXXRecordDecl *LambdaClass = 
cast(Function)->getParent();
+  const CXXRecordDecl *LambdaPattern =
+  cast(PatternDecl)->getParent();





Comment at: clang/lib/Sema/SemaConcept.cpp:508-509
+  unsigned Instantiated = 0;
+  for (unsigned I = 0; I < LambdaPattern->capture_size(); I++) {
+const LambdaCapture *CapturePattern = LambdaPattern->getCapture(I);
+if (!CapturePattern->capturesVariable()) {





Comment at: clang/lib/Sema/SemaConcept.cpp:584-588
+  if (isLambdaCallOperator(FD)) {
+if (addInstantiatedCapturesToScope(FD, 
FromMemTempl->getTemplatedDecl(),
+   Scope, MLTAL))
+  return true;
+  }





Comment at: clang/lib/Sema/SemaConcept.cpp:615-618
+if (isLambdaCallOperator(FD)) {
+  if (addInstantiatedCapturesToScope(FD, InstantiatedFrom, Scope, MLTAL))
+return true;
+}





Comment at: clang/lib/Sema/SemaLambda.cpp:449-452
+if (!LSI->ReturnType->isDependentType() && !LSI->ReturnType->isVoidType()) 
{
+  S.RequireCompleteType(CallOperator->getBeginLoc(), LSI->ReturnType,
+diag::err_lambda_incomplete_result);
+}





Comment at: clang/lib/Sema/SemaLambda.cpp:875-877
 if (!FTI.hasMutableQualifier() && !IsLambdaStatic) {
-  FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const,
-

1 2 >

1 - 100 of 175 matches

Mail list logo