[clang] e685bde - [Clang][RISCV] Bump rvv intrinsics version to v0.11

[via cfe-commits]

Author: eopXD
Date: 2023-02-02T00:08:29-08:00
New Revision: e685bde1e0b0dcfeb1619b434a5dce3c755b9631

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

LOG: [Clang][RISCV] Bump rvv intrinsics version to v0.11

The LLVM now supports v0.11 of the RVV intrinsics. Users can use the macro
`riscv_v_intrinsic` to distinguish what kind of intrinsics is supported in
the compiler.

Please refer to tag descriptions under

https://github.com/riscv-non-isa/rvv-intrinsic-doc/tags

Reviewed By: kito-cheng, asb

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

Added: 


Modified: 
clang/lib/Basic/Targets/RISCV.cpp
clang/test/Preprocessor/riscv-target-features.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/RISCV.cpp 
b/clang/lib/Basic/Targets/RISCV.cpp
index 25fda05da033f..7c801657b6ac3 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -197,8 +197,8 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions 
&Opts,
 
   if (ISAInfo->hasExtension("zve32x")) {
 Builder.defineMacro("__riscv_vector");
-// Currently we support the v0.10 RISC-V V intrinsics.
-Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 10)));
+// Currently we support the v0.11 RISC-V V intrinsics.
+Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 11)));
   }
 }
 

diff  --git a/clang/test/Preprocessor/riscv-target-features.c 
b/clang/test/Preprocessor/riscv-target-features.c
index e312ea0781911..9f3ab6ff03ec5 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -267,7 +267,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 100{{$}}
@@ -281,7 +281,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 100{{$}}
@@ -294,7 +294,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64X-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64X-EXT: __riscv_vector 1
 // CHECK-ZVE64X-EXT: __riscv_zve32x 100{{$}}
@@ -305,7 +305,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32F-EXT %s
 // CHECK-ZVE32F-EXT: __riscv_v_elen 32
 // CHECK-ZVE32F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32F-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32F-EXT: __riscv_vector 1
 // CHECK-ZVE32F-EXT: __riscv_zve32f 100{{$}}
@@ -316,7 +316,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32X-EXT %s
 // CHECK-ZVE32X-EXT: __riscv_v_elen 32
 // CHECK-ZVE32X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32X-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32X-EXT: __riscv_vector 1
 // CHECK-ZVE32X-EXT: __riscv_zve32x 100{{$}}



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

[PATCH] D143051: [Clang][RISCV] Bump rvv intrinsics version to v0.11

[Yueh-Ting (eop) Chen 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 rGe685bde1e0b0: [Clang][RISCV] Bump rvv intrinsics version to 
v0.11 (authored by eopXD).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143051

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -267,7 +267,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 100{{$}}
@@ -281,7 +281,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 100{{$}}
@@ -294,7 +294,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64X-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64X-EXT: __riscv_vector 1
 // CHECK-ZVE64X-EXT: __riscv_zve32x 100{{$}}
@@ -305,7 +305,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32F-EXT %s
 // CHECK-ZVE32F-EXT: __riscv_v_elen 32
 // CHECK-ZVE32F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32F-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32F-EXT: __riscv_vector 1
 // CHECK-ZVE32F-EXT: __riscv_zve32f 100{{$}}
@@ -316,7 +316,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32X-EXT %s
 // CHECK-ZVE32X-EXT: __riscv_v_elen 32
 // CHECK-ZVE32X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32X-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32X-EXT: __riscv_vector 1
 // CHECK-ZVE32X-EXT: __riscv_zve32x 100{{$}}
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -197,8 +197,8 @@
 
   if (ISAInfo->hasExtension("zve32x")) {
 Builder.defineMacro("__riscv_vector");
-// Currently we support the v0.10 RISC-V V intrinsics.
-Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 10)));
+// Currently we support the v0.11 RISC-V V intrinsics.
+Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 11)));
   }
 }
 


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -267,7 +267,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 100{{$}}
@@ -281,7 +281,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 100{{$}}
@@ -294,7 +294,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64X-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64X-EXT: __riscv_vector 1
 // CHECK-ZVE64X-EXT: __riscv_zve32x 100{{$}}
@@ -305,7 +305,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32F-EXT %s
 // CHECK-ZVE32F-EXT: __riscv_v_elen 32
 // CHECK-ZVE32F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32F-EXT: __riscv_v_min_vlen 

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

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

Another disadvantage of using `CINDEX_VERSION_MINOR` instead of the `sizeof` is 
that an older libclang version won't know about compatibility of newer versions 
with itself. But this reasoning brought me to a different thought: when a 
program is compiled against a libclang version X, it should not be expected to 
be runtime-compatible with a libclang version older than X. For example, 
KDevelop uses `CINDEX_VERSION_MINOR` to decide whether or not certain libclang 
API is available.

I suspect modifying the exposed struct's size will violate ODR in C++ programs 
that use libclang. Quote from the cppreference ODR page 
:

> Note: in C, there is no program-wide ODR for types, and even extern 
> declarations of the same variable in different translation units may have 
> different types as long as they are compatible. In C++, the source-code 
> tokens used in declarations of the same type must be the same as described 
> above: if one .cpp file defines `struct S { int x; };` and the other .cpp 
> file defines `struct S { int y; };`, the behavior of the program that links 
> them together is undefined. This is usually resolved with unnamed namespaces.

I think the answer to this StackOverflow question 

 confirms my suspicion.

In order to avoid the undefined behavior, libclang can use the same approach as 
for other structs/classes: make the struct opaque and provide functions 
`clang_createIndexOptions()`,  `clang_disposeIndexOptions()`, 
`clang_setTempDirPath(CXIndexOptions options, const char *path)`.

`int excludeDeclarationsFromPCH` and `int displayDiagnostics` currently passed 
to `clang_createIndex()` can also be included in the struct and acquire their 
own setter functions. Then only a single argument will be passed to 
`clang_createIndexWithOptions()`: `CXIndexOptions`.

--

In D139774#4094619 , @aaron.ballman 
wrote:

> In D139774#4092553 , @vedgy wrote:
>
>> In D139774#4091631 , 
>> @aaron.ballman wrote:
>>
>>> My preference is still for specific API names. If we want to do something 
>>> general to all temp files, I think `FileSystemOptions` is the way to go.
>>>
>>> My concern with not using a constructor is that, because this is the C API, 
>>> we will be supporting the functionality for a long time even if we do 
>>> switch to using a global override in `FileSystemOptions` where you would 
>>> need to set the information up before executing the compiler. To me, these 
>>> paths are something that should not be possible to change once the compiler 
>>> has started executing. (That also solves the multithreading problem where 
>>> multiple threads are fighting over what the temp directory should be and 
>>> stepping on each other's toes.)
>>
>> As I understand, this leaves two choices:
>>
>> 1. Specific preamble API names, two separate non-constructor setters; the 
>> option values are stored in a separate struct (or even as two separate data 
>> members), not inside `FileSystemOptions`.
>> 2. General temporary-storage arguments (possibly combined in a struct) to a 
>> new overloaded constructor function; the option values are stored inside the 
>> `FileSystemOptions` struct.
>>
>> The second alternative is likely more difficult to implement, more risky and 
>> less convenient to use (the store-in-memory bool option cannot be modified 
>> at any time). Perhaps it should be delayed until (and if) we learn of other 
>> temporary files libclang creates? A downside of implementing the first 
>> option now is that the specific API would have to be supported for a long 
>> time, even after the general temporary-file API is implemented.
>
> I still think the general solution is where we ultimately want to get to and 
> that makes me hesitant to go with specific preamble API names despite that 
> being the direction you prefer. If this wasn't the C APIs, I'd be less 
> concerned because we make no stability guarantees about our C++ interfaces. 
> But we try really hard not to break the C APIs, so adding the specific names 
> is basically committing to not only the APIs but their semantics. I think 
> that makes implementing the general solution slightly more awkward because 
> these are weird special cases that barely get tested in-tree, so they'd be 
> very easy to overlook and accidentally break.
>
> Is there a middle ground where, instead of #2 for general temporary storage, 
> we went with #2 but with compiler-specific directories instead of system 
> directories. e.g., don't let the caller set the temp directory, but do let 
> the caller set the preamble directory (which defaults to the temp directory) 
> so long as it's set before invoking the c

[PATCH] D142448: [clang][Interp] Handle TypeTraitExprs

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142448

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

[PATCH] D140614: [C++20] Mark lambdas in lambda specifiers as dependent if necessary

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/lambda-unevaluated.cpp:127
+auto foo(int t) {
+  int(*f)(int) = [](auto t) -> decltype([=] { return t; } ()) { return t; };
+  return f;

I thought unevaluated lambdas could not have captures. that might be the issue 
here


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

https://reviews.llvm.org/D140614

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

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:17
+
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})

This variable is only used once on the following line, I'd just inline it which 
is more idiomatic.



Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+

It's more idiomatic to parse arguments at the start of the function.



Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+

phosek wrote:
> It's more idiomatic to parse arguments at the start of the function.
This is just a nit, but we usually use a prefix like `ARGS` to make it clear 
that these are input arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[PATCH] D137070: [clang][Interp] Support destructors

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D137070

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

[PATCH] D142534: Fix emission of consteval constructor of derived type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142534

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

[PATCH] D138802: [clang][Interp] Implement DecompositionDecls

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1417-1418
+  assert(!BD->getHoldingVar()); // FIXME
+  if (!this->allocateVariable(BD, BD->getBinding()))
+return false;
+}

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Is this correct? IIRC, the decomposition declaration is its 
> > > own object, but the bindings themselves are references back to the 
> > > decomposition declaration object directly and so they're not distinct 
> > > objects themselves (they're more like aliases).
> > Is this not reflected in the individual bindings? What does it mean that 
> > the bindings aren't "distinct objects themselves"?
> Bindings are basically a label back to an object. Taking the easy case of a 
> structure being bound:
> ```
> struct S { int i, j; } s;
> 
> int main() {
>   auto [val1, val2] = s;
>   return val1 + val2;
> }
> ```
> The way this works under the hood is akin to:
> ```
> struct S { int i, j; } s;
> 
> int main() {
>   S __s = s; // This is the decomposition declaration
>   return __s.i + __s.j; // And the structured bindings give alternative names 
> to the fields in the decomposition declaration
> }
> ```
> so there's no allocation made for `val1` or `val2` because they're not really 
> objects, just names. You can see that in: https://godbolt.org/z/sdj3Mvqhb 
> (note the LLVM IR, which is identical between `foo` and `bar` aside from 
> debug info).
Ah, I see. I guess my way works as well, but yours is better, so I'll change 
the patch. I can basically replace the added stuff in `VisitDeclRefExpr()` by 
just `return this->visit(BD->getBinding())` and that will give me the right 
thing. I hope that way I can remove some of the `BindingDecl` special cases 
I've added as well.

Thanks!


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

https://reviews.llvm.org/D138802

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

[PATCH] D142584: [CodeGen] Add a boolean flag to `Address::getPointer` and `Lvalue::getPointer` that indicates whether the pointer is known not to be null

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This seems reasonable to me.  Eli, are your questions answered?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142584

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

[PATCH] D142144: [RISCV][Driver] Add -mrvv-vector-bits= option similar to -msve-vector-bits=

[Fraser Cormack via Phabricator via cfe-commits]

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

LGTM other than test header comment that needs changed.




Comment at: clang/test/Driver/riscv-rvv-vector-bits.c:2
+// 
-
+// Tests for the -msve-vector-bits flag
+// 
-

Still need to adjust this comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142144

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

[PATCH] D142933: Add -print-multi-selection-flags argument

[Joseph Faulls via Phabricator via cfe-commits]

Joe added a comment.



>> The main trouble I had was at that point we don't have a reference to the 
>> ToolChain, so calling `getMultiSelectionFlags` was not possible. [...] I'm 
>> not sure what the solution here is.
>
> I see two options:
>
> 1. Split getMultiSelectionFlags into needs-ToolChain and 
> doesn't-need-ToolChain parts, with the former calling the latter. From 
> Gnu.cpp, call the latter, with the limitation that it can use fewer flags.
> 2. Pass a ToolChain to the Gnu.cpp functions.

I did briefly try to pass a ToolChain to Gnu.cpp functions, but it became 
intrusive and had to add it in 5+ places, so unless I'm missing a trick to get 
the ToolChain from the Driver, my vote would go to the former.

>> I added the arch extensions and abi to the Result in `getMultiSelectionFlags`
>
> What was the syntax you used for that?
> I'm not super keen on the `march=+ext` syntax I came up with so I'm open to 
> alternatives.

I dropped the `march=` and used the same format as target-features, so it was 
simply `+m` or `+a`. I think this is intuitive enough without the `march=`, and 
the form `x=y` is already broken when you add the flags from the flag list. I 
don't have a strong stance on this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 494221.
Fznamznon added a comment.

Apply suggestion and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
@@ -0,0 +1,12 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
+  "file": ""
+},
+{
+  "directory": "DIR",
+  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
+  "file": ""
+},
+]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,10 @@
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates. This fixes
   `Issue 60344 `_.
+- Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
+  driver mode and emit an error which suggests using ``/TC`` or ``/TP``
+  ``clang-cl`` options instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- /de

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 494222.
Fznamznon added a comment.

Fix incorrect conflict resolution


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,10 @@
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates. This fixes
   `Issue 60344 `_.
+- Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
+  driver mode and emit an error which suggests using ``/TC`` or ``/TP``
+  ``clang-cl`` options instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto 

[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D142826#4098014 , @chapuni wrote:

> This discovers a warning in 
> https://reviews.llvm.org/rGa68d4b11465f5b3326be1dd820f59fac275b7581
> I think its checking is valid if size_t is uint32_t (eg. -m32)
>
> Could you guys teach me what the right fix for it? I don't know canonical 
> fixes for it.

+1 we have some code like this too and I'm not sure what's the best way to 
rewrite it to pacify this warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/Driver/x-args.c:12
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?

MaskRay wrote:
> Fznamznon wrote:
> > Fznamznon wrote:
> > > MaskRay wrote:
> > > > You can remove this `CL-NOT` negative pattern. Instead, use 
> > > > `--check-prefix=CL --implicit-check-not=error:` to check that there is 
> > > > no other error.
> > > I probably don't fully understand the suggestion, but there is a `error:` 
> > > that is being checked in this test. The one that this revision adds. It 
> > > is checked on the next line.
> > Is that ok if I submit the patch "as is"?
> `CL-NOT:` checks one specific error message does not appear before `error: 
> unsupported option ...`. You need another pattern to check that it does not 
> appear after `error: unsupported option ...`. If you use 
> --implicit-check-not, you additionally check that there is no other error 
> diagnostic whatever their message is.
Okay, this sounds reasonable, thank you. Applied.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Junchang Liu via Phabricator via cfe-commits]

paperchalice updated this revision to Diff 494223.
paperchalice added a comment.

- Use `clang::driver::Driver::GetResourcesPath`
- Use `ARG` as prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  cmake/Modules/GetClangResourceDir.cmake
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/CMakeLists.txt

Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -86,8 +86,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LIBOMP_HEADERS_INSTALL_PATH SUBDIR include)
 endif()
 
 # Build host runtime library, after LIBOMPTARGET variables are set since they are needed
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -257,7 +257,11 @@
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(resource_dir ${LLVM_TOOLS_BINARY_DIR}/${CLANG_RESOURCE_DIR})
+  else()
+set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  endif()
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangHost.h"
 #include "TestingSupport/SubsystemRAII.h"
@@ -37,13 +39,11 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir =
-  "/foo/bar/" LLDB_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
-  std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir =
-  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
+  std::string path_to_liblldb = "C:\\foo\\bar\\lib\\";
 #endif
+  std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
+  path_to_liblldb + "liblldb", CLANG_RESOURCE_DIR);
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
   // The path doesn't really exist, so setting verify to true should make
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -51,11 +52,14 @@
   Log *log = GetLog(LLDBLog::Host);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
+  const std::string clang_resource_path =
+  clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
-  // in $install_dir/lib{,64}/clang/$clang_version.
-  CLANG_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING,
+  // in $install_dir/lib{,64}/clang/$clang_version or
+  // $install_dir/bin/$CLANG_RESOURCE_DIR
+  clang_resource_path,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_INSTALL_LIBDIR_BASENAME.
@@ -82,7 +86,8 @@
 }
 
 bool lldb_private::ComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,
-  

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

zero9178 wrote:
> Any reason this is `mutable`? I don't see it being used a non-const way in a 
> const method
yeah, `T const *operator->() const` updates the optional by actually creating 
the value. (`std::optional::empalce` is not const)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Gentle ping :-)


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

https://reviews.llvm.org/D142181

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

[PATCH] D142704: [C++20][Modules] Handle template declarations in header units.

[Iain Sandoe 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 rGcdd44e2c8554: [C++20][Modules] Handle template declarations 
in header units. (authored by iains).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142704

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/module/module.import/p6.cpp


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -22,6 +22,8 @@
 
 int bad_var_definition = 3;  // expected-error {{non-inline external 
definitions are not permitted in C++ header units}}
 
+/* The cases below should compile without diagnostics.  */
+
 class A {
 public:
 // This is a declaration instead of definition.
@@ -36,3 +38,32 @@
   S(S&);
 };
 S::S(S&) = default;
+
+template 
+_X tmpl_var_ok_0 = static_cast<_X>(-1);
+
+template 
+constexpr _T tmpl_var_ok_1 = static_cast<_T>(42);
+
+inline int a = tmpl_var_ok_1;
+
+template  class _T>
+constexpr int tmpl_var_ok_2 = _T<_Tp>::value ? 42 : 6174 ;
+
+template
+int tmpl_OK (_Ep) { return 0; }
+
+template 
+bool
+operator==(_T1& , _T1& ) { return false; }
+
+constexpr long one_k = 1000L;
+
+template 
+void* tmpl_fn_ok
+(_Args ...__args) { return nullptr; }
+
+inline int foo (int a) {
+  return tmpl_OK (a);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13090,9 +13090,10 @@
   // C++ [module.import/6] external definitions are not permitted in header
   // units.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  VDecl->isThisDeclarationADefinition() &&
+  !VDecl->isInvalidDecl() && VDecl->isThisDeclarationADefinition() &&
   VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&
-  !VDecl->isInline()) {
+  !VDecl->isInline() && !VDecl->isTemplated() &&
+  !isa(VDecl)) {
 Diag(VDecl->getLocation(), diag::err_extern_def_in_header_unit);
 VDecl->setInvalidDecl();
   }
@@ -15261,9 +15262,10 @@
   // FIXME: Consider an alternate location for the test where the inlined()
   // state is complete.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+  !FD->isInvalidDecl() && !FD->isInlined() &&
+  BodyKind != FnBodyKind::Delete && BodyKind != FnBodyKind::Default &&
   FD->getFormalLinkage() == Linkage::ExternalLinkage &&
-  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
-  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
+  !FD->isTemplated() && !FD->isTemplateInstantiation()) {
 assert(FD->isThisDeclarationADefinition());
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -22,6 +22,8 @@
 
 int bad_var_definition = 3;  // expected-error {{non-inline external definitions are not permitted in C++ header units}}
 
+/* The cases below should compile without diagnostics.  */
+
 class A {
 public:
 // This is a declaration instead of definition.
@@ -36,3 +38,32 @@
   S(S&);
 };
 S::S(S&) = default;
+
+template 
+_X tmpl_var_ok_0 = static_cast<_X>(-1);
+
+template 
+constexpr _T tmpl_var_ok_1 = static_cast<_T>(42);
+
+inline int a = tmpl_var_ok_1;
+
+template  class _T>
+constexpr int tmpl_var_ok_2 = _T<_Tp>::value ? 42 : 6174 ;
+
+template
+int tmpl_OK (_Ep) { return 0; }
+
+template 
+bool
+operator==(_T1& , _T1& ) { return false; }
+
+constexpr long one_k = 1000L;
+
+template 
+void* tmpl_fn_ok
+(_Args ...__args) { return nullptr; }
+
+inline int foo (int a) {
+  return tmpl_OK (a);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13090,9 +13090,10 @@
   // C++ [module.import/6] external definitions are not permitted in header
   // units.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  VDecl->isThisDeclarationADefinition() &&
+  !VDecl->isInvalidDecl() && VDecl->isThisDeclarationADefinition() &&
   VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&
-  !VDecl->isInline()) {
+  !VDecl->isInline() && !VDecl->isTemplated() &&
+  !isa(VDecl)) {
 Diag(VDecl->getLocation(), diag::err_extern_def_in_header_unit);
 VDecl->setInvalidDecl();
   }
@@ -15261,9 +15262,10 @@
   // FIXME: Consider an alternate location for the test where the inlined()
   // state is complete.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+  !FD->is

[clang] cdd44e2 - [C++20][Modules] Handle template declarations in header units.

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2023-02-02T10:51:08Z
New Revision: cdd44e2c85542d152aef19cfd1d2ad451d774935

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

LOG: [C++20][Modules] Handle template declarations in header units.

This addresses part of https://github.com/llvm/llvm-project/issues/60079

The test for external functions was not considering function templates.

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/CXX/module/module.import/p6.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0cfabed8c7dc..b2bece4d9db0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13090,9 +13090,10 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
   // C++ [module.import/6] external definitions are not permitted in header
   // units.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  VDecl->isThisDeclarationADefinition() &&
+  !VDecl->isInvalidDecl() && VDecl->isThisDeclarationADefinition() &&
   VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&
-  !VDecl->isInline()) {
+  !VDecl->isInline() && !VDecl->isTemplated() &&
+  !isa(VDecl)) {
 Diag(VDecl->getLocation(), diag::err_extern_def_in_header_unit);
 VDecl->setInvalidDecl();
   }
@@ -15261,9 +15262,10 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope 
*FnBodyScope, Decl *D,
   // FIXME: Consider an alternate location for the test where the inlined()
   // state is complete.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+  !FD->isInvalidDecl() && !FD->isInlined() &&
+  BodyKind != FnBodyKind::Delete && BodyKind != FnBodyKind::Default &&
   FD->getFormalLinkage() == Linkage::ExternalLinkage &&
-  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
-  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
+  !FD->isTemplated() && !FD->isTemplateInstantiation()) {
 assert(FD->isThisDeclarationADefinition());
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();

diff  --git a/clang/test/CXX/module/module.import/p6.cpp 
b/clang/test/CXX/module/module.import/p6.cpp
index 4360d3e67385..0ed8b5958dff 100644
--- a/clang/test/CXX/module/module.import/p6.cpp
+++ b/clang/test/CXX/module/module.import/p6.cpp
@@ -22,6 +22,8 @@ int ok_var_decl;
 
 int bad_var_definition = 3;  // expected-error {{non-inline external 
definitions are not permitted in C++ header units}}
 
+/* The cases below should compile without diagnostics.  */
+
 class A {
 public:
 // This is a declaration instead of definition.
@@ -36,3 +38,32 @@ struct S {
   S(S&);
 };
 S::S(S&) = default;
+
+template 
+_X tmpl_var_ok_0 = static_cast<_X>(-1);
+
+template 
+constexpr _T tmpl_var_ok_1 = static_cast<_T>(42);
+
+inline int a = tmpl_var_ok_1;
+
+template  class _T>
+constexpr int tmpl_var_ok_2 = _T<_Tp>::value ? 42 : 6174 ;
+
+template
+int tmpl_OK (_Ep) { return 0; }
+
+template 
+bool
+operator==(_T1& , _T1& ) { return false; }
+
+constexpr long one_k = 1000L;
+
+template 
+void* tmpl_fn_ok
+(_Args ...__args) { return nullptr; }
+
+inline int foo (int a) {
+  return tmpl_OK (a);
+}



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

[PATCH] D138802: [clang][Interp] Implement DecompositionDecls

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 494235.

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

https://reviews.llvm.org/D138802

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Program.h
  clang/test/AST/Interp/cxx17.cpp

Index: clang/test/AST/Interp/cxx17.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx17.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify=ref %s
+
+// ref-no-diagnostics
+
+struct F { int a; int b;};
+constexpr F getF() {
+  return {12, 3};
+}
+constexpr int f() {
+  auto [a1, b1] = getF();
+  auto [a2, b2] = getF();
+
+  return a1 + a2 + b1 + b2;
+}
+static_assert(f() == 30);
+
+
+constexpr int structRefs() {
+  const auto &[a, b] = getF();
+
+  return a + b;
+}
+// FIXME: This should work, but the MaterializeTemporaryExpr handling is not ready for it.
+static_assert(structRefs() == 15); // expected-error {{not an integral constant expression}}
+
+constexpr int structRefs2() {
+  F f = getF();
+  const auto &[a, b] = f;
+
+  return a + b;
+}
+static_assert(structRefs2() == 15);
+
+
+template
+struct Tuple {
+  T first;
+  T second;
+  constexpr Tuple(T a, T b) : first(a), second(b) {}
+};
+template
+constexpr T addTuple(const Tuple &Tu) {
+  auto [a, b] = Tu;
+  return a + b;
+}
+
+constexpr Tuple T1 = Tuple(1,2);
+static_assert(addTuple(T1) == 3);
+
+constexpr Tuple T2 = Tuple(11,2);
+static_assert(addTuple(T2) == 13);
+
+
+
+
+constexpr int a() {
+  int a[2] = {5, 3};
+  auto [x, y] = a;
+  return x + y;
+}
+static_assert(a() == 8);
+
+
+constexpr int b() {
+  int a[2] = {5, 3};
+  auto &[x, y] = a;
+  x += 1;
+  y += 2;
+  return a[0] + a[1];
+}
+static_assert(b() == 11);
Index: clang/lib/AST/Interp/Program.h
===
--- clang/lib/AST/Interp/Program.h
+++ clang/lib/AST/Interp/Program.h
@@ -131,7 +131,9 @@
   /// Context to manage declaration lifetimes.
   class DeclScope {
   public:
-DeclScope(Program &P, const VarDecl *VD) : P(P) { P.startDeclaration(VD); }
+DeclScope(Program &P, const ValueDecl *VD) : P(P) {
+  P.startDeclaration(VD);
+}
 ~DeclScope() { P.endDeclaration(); }
 
   private:
@@ -222,7 +224,7 @@
   unsigned CurrentDeclaration = NoDeclaration;
 
   /// Starts evaluating a declaration.
-  void startDeclaration(const VarDecl *Decl) {
+  void startDeclaration(const ValueDecl *Decl) {
 LastDeclaration += 1;
 CurrentDeclaration = LastDeclaration;
   }
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -205,17 +205,11 @@
 template 
 bool ByteCodeStmtGen::visitDeclStmt(const DeclStmt *DS) {
   for (auto *D : DS->decls()) {
-// Variable declarator.
-if (auto *VD = dyn_cast(D)) {
-  if (!this->visitVarDecl(VD))
-return false;
-  continue;
-}
-
-// Decomposition declarator.
-if (auto *DD = dyn_cast(D)) {
-  return this->bail(DD);
-}
+const auto *VD = dyn_cast(D);
+if (!VD)
+  return false;
+if (!this->visitVarDecl(VD))
+  return false;
   }
 
   return true;
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -234,9 +234,12 @@
   }
 
   /// Returns whether we should create a global variable for the
-  /// given VarDecl.
-  bool shouldBeGloballyIndexed(const VarDecl *VD) const {
-return VD->hasGlobalStorage() || VD->isConstexpr();
+  /// given ValueDecl.
+  bool shouldBeGloballyIndexed(const ValueDecl *VD) const {
+if (const auto *V = dyn_cast(VD))
+  return V->hasGlobalStorage() || V->isConstexpr();
+
+return false;
   }
 
   llvm::RoundingMode getRoundingMode(const Expr *E) const {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -28,9 +28,11 @@
 /// Scope used to handle temporaries in toplevel variable declarations.
 template  class DeclScope final : public LocalScope {
 public:
-  DeclScope(ByteCodeExprGen *Ctx, const VarDecl *VD)
+  DeclScope(ByteCodeExprGen *Ctx, const ValueDecl *VD)
   : LocalScope(Ctx), Scope(Ctx->P, VD) {}
 
+  virtual ~DeclScope() override { this->emitDestruction(); }
+
   void addExtended(const Scope::Local &Local) override {
 return this->addLocal(Local);
   }
@@ -895,11 +897,11 @@
   if (std::optional T = classify(LV->getType())) {
 if (!LV->refersToBitField()) {
   // Only primitive, non bit-field types can be dereferenced directl

[clang] 4a1832a - [Clang] Add builtin_nondeterministic_value

[via cfe-commits]

Author: ManuelJBrito
Date: 2023-02-02T11:14:17Z
New Revision: 4a1832a5c801a61bf4c30891aaebe63993712fd9

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

LOG: [Clang] Add builtin_nondeterministic_value

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

Added: 
clang/test/CodeGen/builtins-nondeterministic-value.c

Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Builtins.def
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 0b533a2ff7596..d3a8d958dc7ca 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3080,6 +3080,32 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_nondeterministic_value``
+
+
+``__builtin_nondeterministic_value`` returns a valid nondeterministic value of 
the same type as the provided argument.
+
+**Syntax**:
+
+.. code-block:: c++
+
+type __builtin_nondeterministic_value(type x)
+
+**Examples**:
+
+.. code-block:: c++
+
+int x = __builtin_nondeterministic_value(x);
+float y = __builtin_nondeterministic_value(y);
+__m256i a = __builtin_nondeterministic_value(a);
+
+**Description**
+
+Each call to ``__builtin_nondeterministic_value`` returns a valid value of the 
type given by the argument.
+
+The types currently supported are: integer types, floating-point types, vector 
types.
+
+Query for this feature with 
``__has_builtin(__builtin_nondeterministic_value)``.
 
 ``__builtin_sycl_unique_stable_name``
 -

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9cc2a72f4c864..5f0f3247cf77d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -72,6 +72,8 @@ Non-comprehensive list of changes in this release
 - Clang now saves the address of ABI-indirect function parameters on the stack,
   improving the debug information available in programs compiled without
   optimizations.
+- Clang now supports ``__builtin_nondeterministic_value`` that returns a
+  nondeterministic value of the same type as the provided argument.
 
 New Compiler Flags
 --

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 6b54ff7c40e82..f401a6b4c62b2 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -655,6 +655,7 @@ BUILTIN(__builtin_alloca_uninitialized, "v*z", "Fn")
 BUILTIN(__builtin_alloca_with_align, "v*zIz", "Fn")
 BUILTIN(__builtin_alloca_with_align_uninitialized, "v*zIz", "Fn")
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
+BUILTIN(__builtin_nondeterministic_value, "v.", "nt")
 
 BUILTIN(__builtin_elementwise_abs, "v.", "nct")
 BUILTIN(__builtin_elementwise_max, "v.", "nct")

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 741c2503127af..67d55ab3d8c6d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13571,6 +13571,8 @@ class Sema final {
   bool PrepareBuiltinElementwiseMathOneArgCall(CallExpr *TheCall);
   bool PrepareBuiltinReduceMathOneArgCall(CallExpr *TheCall);
 
+  bool SemaBuiltinNonDeterministicValue(CallExpr *TheCall);
+
   // Matrix builtin handling.
   ExprResult SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult);

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index bf1c9acc7bec7..4a8ca1f918442 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3060,6 +3060,15 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+llvm::Type *Ty = ConvertType(E->getArg(0)->getType());
+
+Value *Result = PoisonValue::get(Ty);
+Result = Builder.CreateFreeze(Result);
+
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_elementwise_abs: {
 Value *Result;
 QualType QT = E->getArg(0)->getType();

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 4602284309491..2618b0cd64b20 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2584,6 +2584,12 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
 break;
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+if (SemaBuiltinNonDeterministicValue(TheCall))
+  ret

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Manuel Brito 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 rG4a1832a5c801: [Clang] Add builtin_nondeterministic_value 
(authored by ManuelJBrito).

Changed prior to commit:
  https://reviews.llvm.org/D142388?vs=494024&id=494237#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-nondeterministic-value.c

Index: clang/test/CodeGen/builtins-nondeterministic-value.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-nondeterministic-value.c
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+typedef _Bool bool4 __attribute__((ext_vector_type(4)));
+
+int clang_nondet_i( int x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca i32, align 4
+// CHECK: store i32 [[X:%.*]], ptr [[A]], align 4
+// CHECK: [[R:%.*]] = freeze i32 poison
+// CHECK: ret i32 [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+float clang_nondet_f( float x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca float, align 4
+// CHECK: store float [[X:%.*]], ptr [[A]], align 4
+// CHECK: [[R:%.*]] = freeze float poison
+// CHECK: ret float [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+double clang_nondet_d( double x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca double, align 8
+// CHECK: store double [[X:%.*]], ptr [[A]], align 8
+// CHECK: [[R:%.*]] = freeze double poison
+// CHECK: ret double [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+_Bool clang_nondet_b( _Bool x) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca i8, align 1
+// CHECK: [[B:%.*]] = zext i1 %x to i8
+// CHECK: store i8 [[B]], ptr [[A]], align 1
+// CHECK: [[R:%.*]] = freeze i1 poison
+// CHECK: ret i1 [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+float4 clang_nondet_fv( float4 x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca <4 x float>, align 16
+// CHECK: store <4 x float> [[X:%.*]], ptr [[A]], align 16
+// CHECK: [[R:%.*]] = freeze <4 x float> poison
+// CHECK: ret <4 x float> [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+bool4 clang_nondet_bv( bool4 x ) {
+// CHECK: [[V:%.*]] = freeze <4 x i1> poison
+// CHECK: store <4 x i1> [[V]], ptr [[P:%.*]], align 1
+// CHECK: [[R:%.*]] = load i8, ptr [[P]], align 1
+// CHECK: ret i8 [[R]]
+  return __builtin_nondeterministic_value(x);
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2584,6 +2584,12 @@
 break;
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+if (SemaBuiltinNonDeterministicValue(TheCall))
+  return ExprError();
+break;
+  }
+
   // __builtin_elementwise_abs restricts the element type to signed integers or
   // floating point types only.
   case Builtin::BI__builtin_elementwise_abs: {
@@ -17857,6 +17863,21 @@
   return false;
 }
 
+bool Sema::SemaBuiltinNonDeterministicValue(CallExpr *TheCall) {
+  if (checkArgCount(*this, TheCall, 1))
+return true;
+
+  ExprResult Arg = TheCall->getArg(0);
+  QualType TyArg = Arg.get()->getType();
+
+  if (!TyArg->isBuiltinType() && !TyArg->isVectorType())
+return Diag(TheCall->getArg(0)->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+   << 1 << /*vector, integer or floating point ty*/ 0 << TyArg;
+
+  TheCall->setType(TyArg);
+  return false;
+}
+
 ExprResult Sema::SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult) {
   if (checkArgCount(*this, TheCall, 1))
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3060,6 +3060,15 @@
 return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+llvm::Type *Ty = ConvertType(E->getArg(0)->getType());
+
+Value *Result = PoisonValue::get(Ty);
+Result = Builder.CreateFreeze(Result);
+
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_elementwise_abs: {
 Value *Result;
 QualType QT = E->getArg(0)->getType();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -13571,6 +13571,8 @@
   bool PrepareBuiltinElementwiseMathOneArgCall(CallExpr *TheCall);
   bool PrepareBuiltinReduceMathOneArgCall

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added a comment.

Thank you all for the reviews and helping me see this patch through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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

[PATCH] D142384: [C++20] Fix a crash with modules.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In D142384#4098650 , @usaxena95 wrote:

> I think this is fine, and we should just use the definition when it is 
> available without asking the callers to request fields only when definition 
> is available.

Not sure about C, but ObjC stacktrace definitely looks like a potential bug to 
me.
We actually might introduce new failure modes (calling the function on the same 
`RecordDecl` without fields may start retuning different results).

However, we also need to fix this crash in C++, so I suggest to land this and 
see if this will cause any fallout.




Comment at: clang/lib/AST/Decl.cpp:4772
 LoadFieldsFromExternalStorage();
-
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();

Let's add a comment to make sure we avoid someone deleting this code. (Since we 
don't have a test).
```
// This is necessary for correctness for C++ with modules.
// FIXME: come up with a test case that breaks.
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

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

[PATCH] D143054: [include-mapping] Regenerate the mappings from the 20220730 html book.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks!

The changes in C++ mapping are good in general. Most removals are reasonable, 
my only concern is the removal of `std::{begin, end, size, empty, data}` (as 
they are now provided by multiple headers), IMO these are important symbols (no 
action needed in this patch, I added them back manually as part of the 
c-compatibility header change https://reviews.llvm.org/D143160).




Comment at: clang/include/clang/Tooling/Inclusions/CSymbolMap.inc:12
 
-SYMBOL(ATOMIC_BOOL_LOCK_FREE, None, )
-SYMBOL(ATOMIC_CHAR16_T_LOCK_FREE, None, )

We're removing many macros, macros seems to be moved out to a separated page 
(https://en.cppreference.com/w/c/symbol_index/macro) in the latest cppreference 
snapshot, there are some more work needed to update this file. Let's postpone 
it and focus on the C++ symbols in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143054

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

[PATCH] D142933: Add -print-multi-selection-flags argument

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

In D142933#4099043 , @Joe wrote:

> I did briefly try to pass a ToolChain to Gnu.cpp functions, but it became 
> intrusive and had to add it in 5+ places, so unless I'm missing a trick to 
> get the ToolChain from the Driver, my vote would go to the former.

The Driver class does in fact have a private getToolChain method. So two new 
possibilities:

1. Make getToolChain public.
2. Make getMultiSelectionFlags a method of Driver.

> I dropped the `march=` and used the same format as target-features, so it was 
> simply `+m` or `+a`. I think this is intuitive enough without the `march=`

Thanks for explaining. I found that when I was writing a regex to match flags 
it was helpful to have a part of the string I could be sure would be there to 
avoid matching the wrong type of flag, and `march=` helped with that. I also 
worry that names from different types of flags could clash without some kind of 
namespacing.

> the form `x=y` is already broken when you add the flags from the flag list.

Can you give an example of what you mean by that? Sounds like something that 
might need fixing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

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

[PATCH] D142277: [clang][Interp] Clear metadata when destroying locals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:260-261
+// local variable is used after being destroyed.
+InlineDescriptor &ID = *reinterpret_cast(B->rawData());
+std::memset(&ID, 0, sizeof(InlineDescriptor));
   }

aaron.ballman wrote:
> I'm not certain this is a good idea -- we've just deallocated `B` and then 
> we're saying "cool, now get me your raw data so I can muck about with it".
> 
> The comments in `InterpState::deallocate()` say `// Free storage, if 
> necessary.`, so this looks a lot like a use-after-free. Am I missing 
> something?
`deallocate()` doesn't free the `Block`'s memory though, so we can still use it 
afterwards. That's why I had the problems with 
https://reviews.llvm.org/rG5b54cf1a2892767fe949826a32d7820732028a38 and neither 
a
 I could also move this code to `deallocate` directly.

This is just a security measure so we don't end up emitting a `load` 
instruction for a variable we've already emitted a `destroy` instruction for. 
So just for me, not for users.


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

https://reviews.llvm.org/D142277

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

[PATCH] D143054: [include-mapping] Regenerate the mappings from the 20220730 html book.

[Viktoriia Bakalova via Phabricator via cfe-commits]

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

Remove the C symbol map from commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143054

Files:
  clang/include/clang/Tooling/Inclusions/StdRemovedSymbolMap.inc
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc

Index: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
===
--- clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
+++ clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
@@ -6,45 +6,18 @@
 // This file was generated automatically by
 // clang/tools/include-mapping/gen_std.py, DO NOT EDIT!
 //
-// Generated from cppreference offline HTML book (modified on 2018-10-28).
+// Generated from cppreference offline HTML book (modified on 2022-07-30).
 //===--===//
 
-SYMBOL(Assignable, std::, )
-SYMBOL(Boolean, std::, )
-SYMBOL(Common, std::, )
-SYMBOL(CommonReference, std::, )
-SYMBOL(Constructible, std::, )
-SYMBOL(ConvertibleTo, std::, )
-SYMBOL(CopyConstructible, std::, )
-SYMBOL(Copyable, std::, )
-SYMBOL(DefaultConstructible, std::, )
-SYMBOL(DerivedFrom, std::, )
-SYMBOL(Destructible, std::, )
-SYMBOL(EqualityComparable, std::, )
-SYMBOL(EqualityComparableWith, std::, )
 SYMBOL(FILE, std::, )
-SYMBOL(Integral, std::, )
-SYMBOL(Invocable, std::, )
-SYMBOL(Movable, std::, )
-SYMBOL(MoveConstructible, std::, )
-SYMBOL(Predicate, std::, )
-SYMBOL(Regular, std::, )
-SYMBOL(RegularInvocable, std::, )
-SYMBOL(Relation, std::, )
-SYMBOL(Same, std::, )
-SYMBOL(Semiregular, std::, )
-SYMBOL(SignedIntegral, std::, )
-SYMBOL(StrictTotallyOrdered, std::, )
-SYMBOL(StrictTotallyOrderedWith, std::, )
-SYMBOL(StrictWeakOrder, std::, )
-SYMBOL(Swappable, std::, )
-SYMBOL(SwappableWith, std::, )
-SYMBOL(UniformRandomBitGenerator, std::, )
-SYMBOL(UnsignedIntegral, std::, )
 SYMBOL(_Exit, std::, )
 SYMBOL(accumulate, std::, )
 SYMBOL(acos, std::, )
+SYMBOL(acosf, std::, )
 SYMBOL(acosh, std::, )
+SYMBOL(acoshf, std::, )
+SYMBOL(acoshl, std::, )
+SYMBOL(acosl, std::, )
 SYMBOL(add_const, std::, )
 SYMBOL(add_const_t, std::, )
 SYMBOL(add_cv, std::, )
@@ -73,7 +46,10 @@
 SYMBOL(alignment_of, std::, )
 SYMBOL(alignment_of_v, std::, )
 SYMBOL(all_of, std::, )
+SYMBOL(allocate_at_least, std::, )
 SYMBOL(allocate_shared, std::, )
+SYMBOL(allocate_shared_for_overwrite, std::, )
+SYMBOL(allocation_result, std::, )
 SYMBOL(allocator, std::, )
 SYMBOL(allocator_arg, std::, )
 SYMBOL(allocator_arg_t, std::, )
@@ -83,15 +59,35 @@
 SYMBOL(apply, std::, )
 SYMBOL(arg, std::, )
 SYMBOL(array, std::, )
+SYMBOL(as_bytes, std::, )
 SYMBOL(as_const, std::, )
+SYMBOL(as_writable_bytes, std::, )
 SYMBOL(asctime, std::, )
 SYMBOL(asin, std::, )
+SYMBOL(asinf, std::, )
 SYMBOL(asinh, std::, )
+SYMBOL(asinhf, std::, )
+SYMBOL(asinhl, std::, )
+SYMBOL(asinl, std::, )
+SYMBOL(assignable_from, std::, )
+SYMBOL(assoc_laguerre, std::, )
+SYMBOL(assoc_laguerref, std::, )
+SYMBOL(assoc_laguerrel, std::, )
+SYMBOL(assoc_legendre, std::, )
+SYMBOL(assoc_legendref, std::, )
+SYMBOL(assoc_legendrel, std::, )
+SYMBOL(assume_aligned, std::, )
 SYMBOL(async, std::, )
 SYMBOL(at_quick_exit, std::, )
 SYMBOL(atan, std::, )
 SYMBOL(atan2, std::, )
+SYMBOL(atan2f, std::, )
+SYMBOL(atan2l, std::, )
+SYMBOL(atanf, std::, )
 SYMBOL(atanh, std::, )
+SYMBOL(atanhf, std::, )
+SYMBOL(atanhl, std::, )
+SYMBOL(atanl, std::, )
 SYMBOL(atexit, std::, )
 SYMBOL(atof, std::, )
 SYMBOL(atoi, std::, )
@@ -116,19 +112,28 @@
 SYMBOL(atomic_flag, std::, )
 SYMBOL(atomic_flag_clear, std::, )
 SYMBOL(atomic_flag_clear_explicit, std::, )
+SYMBOL(atomic_flag_notify_all, std::, )
+SYMBOL(atomic_flag_notify_one, std::, )
+SYMBOL(atomic_flag_test, std::, )
 SYMBOL(atomic_flag_test_and_set, std::, )
 SYMBOL(atomic_flag_test_and_set_explicit, std::, )
+SYMBOL(atomic_flag_test_explicit, std::, )
+SYMBOL(atomic_flag_wait, std::, )
+SYMBOL(atomic_flag_wait_explicit, std::, )
 SYMBOL(atomic_init, std::, )
-SYMBOL(atomic_is_lockfree, std::, )
+SYMBOL(atomic_is_lock_free, std::, )
 SYMBOL(atomic_load, std::, )
 SYMBOL(atomic_load_explicit, std::, )
+SYMBOL(atomic_notify_all, std::, )
+SYMBOL(atomic_notify_one, std::, )
 SYMBOL(atomic_ref, std::, )
 SYMBOL(atomic_signal_fence, std::, )
 SYMBOL(atomic_store, std::, )
 SYMBOL(atomic_store_explicit, std::, )
 SYMBOL(atomic_thread_fence, std::, )
+SYMBOL(atomic_wait, std::, )
+SYMBOL(atomic_wait_explicit, std::, )
 SYMBOL(atto, std::, )
-SYMBOL(auto_ptr, std::, )
 SYMBOL(back_insert_iterator, std::, )
 SYMBOL(back_inserter, std::, )
 SYMBOL(bad_alloc, std::, )
@@ -141,35 +146,54 @@
 SYMBOL(bad_typeid, std::, )
 SYMBOL(bad_variant_access, std::, )
 SYMBOL(bad_weak_ptr, std::, )
+SYMBOL(barrier, std::, )
 SYMBOL(basic_common_reference, std::, )
 SYMBOL(basic_filebuf, std::, )
+SYMBOL(basic_format_arg, std::, )
+SYMBOL(basic_format_args,

[PATCH] D143054: [include-mapping] Regenerate the mappings from the 20220730 html book.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Sure, removed the C symbol mapping from this patch. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143054

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

Hello Luca and Aaron.
I have noticed that you recently implemented/reviewed multiple interesting 
libclang features, some of which can be used in KDevelop. However, 
`CINDEX_VERSION_MINOR` was last modified in Clang 13. This constant's 
documentation states:

> `CINDEX_VERSION_MINOR` should increase when there are API additions.

Can this constant be incremented in the upcoming Clang 16 release?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Markus Böck via Phabricator via cfe-commits]

zero9178 added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

serge-sans-paille wrote:
> zero9178 wrote:
> > Any reason this is `mutable`? I don't see it being used a non-const way in 
> > a const method
> yeah, `T const *operator->() const` updates the optional by actually creating 
> the value. (`std::optional::empalce` is not const)
Your `T const *operator->() const` method does not do so, `T *operator->()` 
does, which is non-const and hence can call `emplace` without issues. 
So as far as C++ goes it is not required. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:31
+
+static llvm::DenseMap LanguageMappings;
+

VitaNuo wrote:
> hokein wrote:
> > using a map here seems like an overkill, we have just 2 elements, I'd just 
> > use two separate variables (`CMapping`, and `CXXMapping`)
> what about the design idea that we might potentially want to extend this to 
> multiple standards etc.? The idea is that it's extensible to `ObjC`, 
> `OpenCL`... and so on and so forth, as has been discussed offline.
I think having a generic `Lang` enum structure is sufficient for the future 
extension, and I don't think we're going to add other languages in the 
foreseeable future (that's why I value the simple implementation at the 
beginning).

But you're right, getting the implementation right is probably a good idea. I'd 
like to remove the DenseMap, just use a raw array, something like below should 
work 

```
enum Lang {
   C = 0,
   CXX,

   LastValue = CXX,
};

// access by e.g. LanguageMappings[static_cast(Lang::C)].
static SymbolHeaderMapping* 
LanguageMappings[static_cast(Lang::LastValue) + 1];

```



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:188
+  Lang RecognizerLang = Lang::CXX;
+  if (Language == clang::Language::C) {
+RecognizerLang = Lang::C;

VitaNuo wrote:
> hokein wrote:
> > nit: just use `LangOpts.CPlusPlus` to check the language.
> There's `LangStandard::isCPlusPlus` method that I've just discovered. That's 
> probably even better.
sorry if I was not clearer -- there is a bit `CPlusPlus` in the `LangOptions` 
which already does the equivalent thing (see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/LangOptions.cpp#L107).

So we can just use  `if (D->getLangOpts().CPlusPlus)`.



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:115
+
+  EXPECT_EQ(std::nullopt, stdlib::Symbol::named("", "int16_t"));
+  EXPECT_EQ(std::nullopt,

nit:  just `EXPECT_FALSE(stdlib::Symbol::named("", "int16_t")))`, following the 
existing pattern L46.



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:122
+
+TEST(StdlibTest, LanguageSeparationForHeaders) {
+  EXPECT_NE(std::nullopt, stdlib::Header::named("vector"));

The existing `TEST(StdlibTest, All)` test seems like a good umbrella for the 
this test and the above test, how about moving them to the All test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

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

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added a comment.

So this is getting some build failures:

- https://lab.llvm.org/buildbot#builders/38/builds/9446
- https://lab.llvm.org/buildbot#builders/245/builds/4189
- https://lab.llvm.org/buildbot#builders/65/builds/7949
- https://lab.llvm.org/buildbot#builders/188/builds/25538

I suspect it's because of the way different ABIs handle the vectors.  I was 
thinking that if we have the variable be a local variable and also not 
returning the value can fix the tests.

Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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

[PATCH] D142878: Add testing for Fuchsia multilib

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 494258.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142878

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/unittests/Driver/CMakeLists.txt
  clang/unittests/Driver/FuchsiaTest.cpp

Index: clang/unittests/Driver/FuchsiaTest.cpp
===
--- /dev/null
+++ clang/unittests/Driver/FuchsiaTest.cpp
@@ -0,0 +1,62 @@
+//===- unittests/Driver/FuchsiaTest.cpp --- Fuchsia tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Unit tests for Fuchsia
+//
+//===--===//
+
+#include "../../lib/Driver/ToolChains/Fuchsia.h"
+#include "gtest/gtest.h"
+
+using namespace clang::driver;
+
+/*
+This test was added prior to changing the behaviour of Multilib.
+The way that Fuchsia used Multilib made it very likely that the change
+would cause it to break so by adding this exhaustive test we avoid that
+possibility.
+*/
+TEST(FuchsiaTest, MultilibVariant) {
+  MultilibSet Multilibs;
+  toolchains::Fuchsia::configureMultilibs(Multilibs);
+
+  std::string Actual;
+
+  for (bool Itanium : {false, true}) {
+for (bool Hwasan : {false, true}) {
+  for (bool Asan : {false, true}) {
+for (bool Exceptions : {false, true}) {
+  Multilib::flags_list Flags;
+  toolchains::Fuchsia::configureMultilibFlags(Flags, Exceptions, Asan,
+  Hwasan, Itanium);
+  Multilib Selected;
+  EXPECT_TRUE(Multilibs.select(Flags, Selected));
+  Actual += Selected.gccSuffix() + "\n";
+}
+  }
+}
+  }
+  std::string Expected = R"(/noexcept
+
+/asan+noexcept
+/asan
+/hwasan+noexcept
+/hwasan
+/hwasan+noexcept
+/hwasan
+/compat
+/compat
+/compat
+/compat
+/compat
+/compat
+/compat
+/compat
+)";
+  EXPECT_EQ(Expected, Actual);
+}
Index: clang/unittests/Driver/CMakeLists.txt
===
--- clang/unittests/Driver/CMakeLists.txt
+++ clang/unittests/Driver/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_clang_unittest(ClangDriverTests
   DistroTest.cpp
   DXCModeTest.cpp
+  FuchsiaTest.cpp
   ToolChainTest.cpp
   ModuleCacheTest.cpp
   MultilibTest.cpp
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -112,6 +112,10 @@
 
   const char *getDefaultLinker() const override { return "ld.lld"; }
 
+  static void configureMultilibs(MultilibSet &);
+  static void configureMultilibFlags(Multilib::flags_list &, bool Exceptions,
+ bool Asan, bool Hwasan, bool Itanium);
+
 protected:
   Tool *buildLinker() const override;
   Tool *buildStaticLibTool() const override;
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -260,6 +260,32 @@
 return FP;
   };
 
+  configureMultilibs(Multilibs);
+
+  Multilibs.FilterOut([&](const Multilib &M) {
+std::vector RD = FilePaths(M);
+return llvm::all_of(RD, [&](std::string P) { return !getVFS().exists(P); });
+  });
+
+  Multilib::flags_list Flags;
+  configureMultilibFlags(
+  Flags,
+  Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true),
+  getSanitizerArgs(Args).needsAsanRt(),
+  getSanitizerArgs(Args).needsHwasanRt(),
+  Args.getLastArgValue(options::OPT_fcxx_abi_EQ) == "itanium");
+
+  Multilibs.setFilePathsCallback(FilePaths);
+
+  if (Multilibs.select(Flags, SelectedMultilib))
+if (!SelectedMultilib.isDefault())
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+for (const auto &Path : PathsCallback(SelectedMultilib))
+  // Prepend the multilib path to ensure it takes the precedence.
+  getFilePaths().insert(getFilePaths().begin(), Path);
+}
+
+void Fuchsia::configureMultilibs(MultilibSet &Multilibs) {
   Multilibs.push_back(Multilib());
   // Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
   Multilibs.push_back(Multilib("noexcept", {}, {}, 1)
@@ -284,32 +310,16 @@
   .flag("+fno-exceptions"));
   // Use Itanium C++ ABI for the compat multilib.
   Multilibs.push_back(Multilib("compat", {}, {}, 6).flag("+fc++-abi=itanium"));
+}
 
-  Multilibs.FilterOut([&](const Multilib &M) {
-

[PATCH] D142893: Class for building MultilibSet

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 494259.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142893

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/MultilibBuilder.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/CMakeLists.txt
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -11,34 +11,17 @@
 //===--===//
 
 #include "clang/Driver/Multilib.h"
+#include "../../lib/Driver/ToolChains/CommonArgs.h"
 #include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
 using namespace clang::driver;
 using namespace clang;
 
-TEST(MultilibTest, MultilibValidity) {
-
-  ASSERT_TRUE(Multilib().isValid()) << "Empty multilib is not valid";
-
-  ASSERT_TRUE(Multilib().flag("+foo").isValid())
-  << "Single indicative flag is not valid";
-
-  ASSERT_TRUE(Multilib().flag("-foo").isValid())
-  << "Single contraindicative flag is not valid";
-
-  ASSERT_FALSE(Multilib().flag("+foo").flag("-foo").isValid())
-  << "Conflicting flags should invalidate the Multilib";
-
-  ASSERT_TRUE(Multilib().flag("+foo").flag("+foo").isValid())
-  << "Multilib should be valid even if it has the same flag twice";
-
-  ASSERT_TRUE(Multilib().flag("+foo").flag("-foobar").isValid())
-  << "Seemingly conflicting prefixes shouldn't actually conflict";
-}
-
 TEST(MultilibTest, OpEqReflexivity1) {
   Multilib M;
   ASSERT_TRUE(M == M) << "Multilib::operator==() is not reflexive";
@@ -50,40 +33,28 @@
 }
 
 TEST(MultilibTest, OpEqReflexivity3) {
-  Multilib M1, M2;
-  M1.flag("+foo");
-  M2.flag("+foo");
+  Multilib M1({}, {}, {}, 0, {"+foo"});
+  Multilib M2({}, {}, {}, 0, {"+foo"});
   ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
 }
 
 TEST(MultilibTest, OpEqInequivalence1) {
-  Multilib M1, M2;
-  M1.flag("+foo");
-  M2.flag("-foo");
+  Multilib M1({}, {}, {}, 0, {"+foo"});
+  Multilib M2({}, {}, {}, 0, {"-foo"});
   ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
   ASSERT_FALSE(M2 == M1)
   << "Multilibs with conflicting flags are not the same (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence2) {
-  Multilib M1, M2;
-  M2.flag("+foo");
+  Multilib M1;
+  Multilib M2({}, {}, {}, 0, {"+foo"});
   ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
 }
 
-TEST(MultilibTest, OpEqEquivalence1) {
-  Multilib M1, M2;
-  M1.flag("+foo");
-  M2.flag("+foo").flag("+foo");
-  ASSERT_TRUE(M1 == M2) << "Flag duplication shouldn't affect equivalence";
-  ASSERT_TRUE(M2 == M1)
-  << "Flag duplication shouldn't affect equivalence (commuted)";
-}
-
 TEST(MultilibTest, OpEqEquivalence2) {
-  Multilib M1("64");
-  Multilib M2;
-  M2.gccSuffix("/64");
+  Multilib M1("/64");
+  Multilib M2("/64");
   ASSERT_TRUE(M1 == M2)
   << "Constructor argument must match Multilib::gccSuffix()";
   ASSERT_TRUE(M2 == M1)
@@ -91,9 +62,8 @@
 }
 
 TEST(MultilibTest, OpEqEquivalence3) {
-  Multilib M1("", "32");
-  Multilib M2;
-  M2.osSuffix("/32");
+  Multilib M1("", "/32");
+  Multilib M2("", "/32");
   ASSERT_TRUE(M1 == M2)
   << "Constructor argument must match Multilib::osSuffix()";
   ASSERT_TRUE(M2 == M1)
@@ -101,9 +71,8 @@
 }
 
 TEST(MultilibTest, OpEqEquivalence4) {
-  Multilib M1("", "", "16");
-  Multilib M2;
-  M2.includeSuffix("/16");
+  Multilib M1("", "", "/16");
+  Multilib M2("", "", "/16");
   ASSERT_TRUE(M1 == M2)
   << "Constructor argument must match Multilib::includeSuffix()";
   ASSERT_TRUE(M2 == M1)
@@ -111,31 +80,31 @@
 }
 
 TEST(MultilibTest, OpEqInequivalence3) {
-  Multilib M1("foo");
-  Multilib M2("bar");
+  Multilib M1("/foo");
+  Multilib M2("/bar");
   ASSERT_FALSE(M1 == M2) << "Differing gccSuffixes should be different";
   ASSERT_FALSE(M2 == M1)
   << "Differing gccSuffixes should be different (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence4) {
-  Multilib M1("", "foo");
-  Multilib M2("", "bar");
+  Multilib M1("", "/foo");
+  Multilib M2("", "/bar");
   ASSERT_FALSE(M1 == M2) << "Differing osSuffixes should be different";
   ASSERT_FALSE(M2 == M1)
   << "Differing osSuffixes should be different (commuted)";
 }
 
 TEST(MultilibTest, OpEqInequivalence5) {
-  Multilib M1("", "", "foo");
-  Multilib M2("", "", "bar");

[PATCH] D141389: [DFSAN] Add support for strnlen, strncat, strsep, sscanf and _tolower

[Andrew via Phabricator via cfe-commits]

browneee added inline comments.



Comment at: compiler-rt/lib/dfsan/dfsan_custom.cpp:213
+  char *res = strsep(s, delim);
+  s_label = dfsan_read_label(base, strlen(base));
+  if (res && (res != base)) {

tkuchta wrote:
> browneee wrote:
> > tkuchta wrote:
> > > browneee wrote:
> > > > The `s_label` represents the taint label for `s` (the pointer).
> > > > 
> > > > This line would clobber the taint label of the pointer (`s`) with a 
> > > > taint label from `s[0][0..n]`.
> > > > 
> > > > I think this line should be deleted.
> > > Agree, s_label represents the taint associated with the **s pointer. 
> > > However I am now wondering if that is the taint wich we would like to 
> > > return.
> > > For example, if we have
> > > if (flags().strict_data_dependencies) {
> > > *ret_label = res ? s_label : 0;
> > > 
> > > We would taint the return value with the value of the pointer, not the 
> > > data. It means that if we operate on a string for which the characters 
> > > are tainted, but the pointer itself isn't, we are likely going to return 
> > > label 0. My understanding was that we are more concerned with the taint 
> > > of the data, not the pointer, am I missing something?
> > > 
> > Yes, we are usually more concerned with the taint of the data, not the 
> > pointer.
> > 
> > With strict dependencies:
> > // If the input pointer is tainted, the output pointer would be tainted 
> > (because it is derived from the input pointer - maybe the same value).
> > taint(s[0]) == dfsan_read_label(s, sizeof(s)) > taint(ret) == 
> > ret_label[0]
> > 
> > // If the input data is tainted, the output data would be tainted (because 
> > it is derived from the input data).
> > taint(s[0][0]) == MEM_TO_SHADOW(s[0])[0] > taint(ret[0]) == 
> > MEM_TO_SHADOW(ret)[0]
> > 
> > Because s[0] == ret  (or ret==null), (for the non-null case) the output 
> > shadow bytes are the same bytes as input shadow bytes and so these taint 
> > labels for the string data in shadow memory do not need to be explicitly 
> > propagated in this function. 
> > 
> > I think the only case actually changing/copying string data is writing a 
> > delimiter byte to NULL, which you handled.
> I am working on the changes and I came across a strange behavior that I would 
> like to ask about.
> 
> It turned out that if we do
> 
> char *s = " ... ";
> dfsan_set_label(label, &p_s, sizeof(&p_s));
> 
> Then, the s_label within the handler is 0, not "label". This is unexpected, 
> as we would like the pointer itself to be labeled here.
> I believe this has something to do with the fact that the input string in 
> strsep is a double pointer. For example this works as expected for the 
> delimiter string, which is a single pointer. 
> It's either I'm doing something incorrectly or there is some issue with 
> propagating labels for double pointers.
> Have you perhaps come across this behavior before?
I'm not sure what p_s is in your example. Could you provide a more complete 
example?
(and maybe all in one function, not half in __dfsw_strsep and half in another 
function)

Here is an example showing taint labels at different levels of indirection:

```
#include 
#include 
#include 

int main(void) {
  char *s = " ... ";
  char **p_s = &s;
  char ***pp_s = &p_s;

  dfsan_label i_label = 1 << 0;
  dfsan_label j_label = 1 << 1;
  dfsan_label k_label = 1 << 2;
  dfsan_label m_label = 1 << 3;

  // string data
  dfsan_set_label(i_label, s, strlen(s));
  // pointer s
  dfsan_set_label(j_label, &s, sizeof(s));
  // pointer to pointer s
  dfsan_set_label(k_label, &p_s, sizeof(p_s));
  // pointer to pointer to pointer s
  dfsan_set_label(m_label, &pp_s, sizeof(pp_s));

  assert(pp_s[0][0] == s);

  // string data
  assert(dfsan_read_label(s, strlen(s)) == i_label);
  // pointer s
  assert(dfsan_read_label(&s, sizeof(s)) == j_label);
  // pointer to pointer s
  assert(dfsan_read_label(&p_s, sizeof(p_s)) == k_label);
  // pointer to pointer to pointer s
  assert(dfsan_read_label(&pp_s, sizeof(pp_s)) == m_label);

  return 0;
}
```


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

https://reviews.llvm.org/D141389

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

[PATCH] D142905: Change multilib selection algorithm

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 494260.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142905

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/MultilibBuilder.h
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -33,14 +33,14 @@
 }
 
 TEST(MultilibTest, OpEqReflexivity3) {
-  Multilib M1({}, {}, {}, 0, {"+foo"});
-  Multilib M2({}, {}, {}, 0, {"+foo"});
+  Multilib M1({}, {}, {}, {"+foo"});
+  Multilib M2({}, {}, {}, {"+foo"});
   ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
 }
 
 TEST(MultilibTest, OpEqInequivalence1) {
-  Multilib M1({}, {}, {}, 0, {"+foo"});
-  Multilib M2({}, {}, {}, 0, {"-foo"});
+  Multilib M1({}, {}, {}, {"+foo"});
+  Multilib M2({}, {}, {}, {"-foo"});
   ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
   ASSERT_FALSE(M2 == M1)
   << "Multilibs with conflicting flags are not the same (commuted)";
@@ -48,7 +48,7 @@
 
 TEST(MultilibTest, OpEqInequivalence2) {
   Multilib M1;
-  Multilib M2({}, {}, {}, 0, {"+foo"});
+  Multilib M2({}, {}, {}, {"+foo"});
   ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
 }
 
@@ -124,7 +124,7 @@
 }
 
 TEST(MultilibTest, Construction3) {
-  Multilib M({}, {}, {}, 0, {"+f1", "+f2", "-f3"});
+  Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"});
   for (Multilib::flags_list::const_iterator I = M.flags().begin(),
 E = M.flags().end();
I != E; ++I) {
@@ -149,8 +149,8 @@
 
 TEST(MultilibTest, SetPriority) {
   MultilibSet MS({
-  Multilib("/foo", {}, {}, 1, {"+foo"}),
-  Multilib("/bar", {}, {}, 2, {"+bar"}),
+  Multilib("/foo", {}, {}, {"+foo"}),
+  Multilib("/bar", {}, {}, {"+bar"}),
   });
   Multilib::flags_list Flags1 = {"+foo", "-bar"};
   Multilib Selection1;
@@ -166,3 +166,24 @@
   ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
   << "Selection picked " << Selection2 << " which was not expected";
 }
+
+TEST(MultilibTest, SelectMultiple) {
+  MultilibSet MS({
+  Multilib("/a", {}, {}, {"x"}),
+  Multilib("/b", {}, {}, {"y"}),
+  });
+  std::vector Selection;
+
+  Selection = MS.select({"x"});
+  ASSERT_EQ(1u, Selection.size());
+  EXPECT_EQ("/a", Selection[0].gccSuffix());
+
+  Selection = MS.select({"y"});
+  ASSERT_EQ(1u, Selection.size());
+  EXPECT_EQ("/b", Selection[0].gccSuffix());
+
+  Selection = MS.select({"y", "x"});
+  ASSERT_EQ(2u, Selection.size());
+  EXPECT_EQ("/a", Selection[0].gccSuffix());
+  EXPECT_EQ("/b", Selection[1].gccSuffix());
+}
Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -7,7 +7,7 @@
 //
 //===--===//
 //
-// Unit tests for Multilib and MultilibBuilder
+// Unit tests for MultilibBuilderVariant and MultilibBuilder
 //
 //===--===//
 
@@ -185,14 +185,14 @@
 bool IsSF = I & 0x2;
 Multilib::flags_list Flags;
 if (IsEL)
-  Flags.push_back("+EL");
+  Flags.insert("+EL");
 else
-  Flags.push_back("-EL");
+  Flags.insert("-EL");
 
 if (IsSF)
-  Flags.push_back("+SF");
+  Flags.insert("+SF");
 else
-  Flags.push_back("-SF");
+  Flags.insert("-SF");
 
 Multilib Selection;
 ASSERT_TRUE(MS2.select(Flags, Selection))
@@ -209,3 +209,11 @@
 << "Selection picked " << Selection << " which was not expected ";
   }
 }
+
+TEST(MultilibBuilderTest, PrintArgs) {
+  MultilibBuilderVariant MBV =
+  MultilibBuilderVariant().flag("+x").flag("-y").flag("+a").flag("-b").flag(
+  "+c");
+  auto M = MBV.makeMultilib();
+  ASSERT_EQ(Multilib::arg_list({"-x", "-a", "-c"}), M.getPrintArgs());
+}
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -289,33 +289,33 @@
 void Fuchsia::configureMultilibs(MultilibSet &Multilibs) {
   Multilibs.push_back(Multilib());
   // Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
-  Multilibs.push_back(MultilibBuilderVariant("noexcept", {}, {}, 1)
+  Multilibs.push_back(MultilibBuilderVariant("noexcept", {}, {})
   

[PATCH] D143059: [NFC] Enable selecting multiple multilibs

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 494262.
michaelplatings added a comment.

Rebase and change llvm::SmallVector to simply 
llvm::SmallVector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143059

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CSKYToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MipsLinux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/fuchsia.cpp
  clang/unittests/Driver/FuchsiaTest.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -153,18 +153,18 @@
   Multilib("/bar", {}, {}, {"+bar"}),
   });
   Multilib::flags_list Flags1 = {"+foo", "-bar"};
-  Multilib Selection1;
+  llvm::SmallVector Selection1;
   ASSERT_TRUE(MS.select(Flags1, Selection1))
   << "Flag set was {\"+foo\"}, but selection not found";
-  ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
-  << "Selection picked " << Selection1 << " which was not expected";
+  ASSERT_TRUE(Selection1.back().gccSuffix() == "/foo")
+  << "Selection picked " << Selection1.back() << " which was not expected";
 
   Multilib::flags_list Flags2 = {"+foo", "+bar"};
-  Multilib Selection2;
+  llvm::SmallVector Selection2;
   ASSERT_TRUE(MS.select(Flags2, Selection2))
   << "Flag set was {\"+bar\"}, but selection not found";
-  ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
-  << "Selection picked " << Selection2 << " which was not expected";
+  ASSERT_TRUE(Selection2.back().gccSuffix() == "/bar")
+  << "Selection picked " << Selection2.back() << " which was not expected";
 }
 
 TEST(MultilibTest, SelectMultiple) {
@@ -172,17 +172,17 @@
   Multilib("/a", {}, {}, {"x"}),
   Multilib("/b", {}, {}, {"y"}),
   });
-  std::vector Selection;
+  llvm::SmallVector Selection;
 
-  Selection = MS.select({"x"});
+  ASSERT_TRUE(MS.select({"x"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y"});
+  ASSERT_TRUE(MS.select({"y"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/b", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y", "x"});
+  ASSERT_TRUE(MS.select({"y", "x"}, Selection));
   ASSERT_EQ(2u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
@@ -317,7 +317,7 @@
 
 static bool select(const std::vector &InFlags,
const MultilibSet &MS, const MultilibFlagMap &MFM,
-   Multilib &Selected) {
+   llvm::SmallVector &Selected) {
   Multilib::flags_list Flags(MFM.getFlags(InFlags));
   Flags.insert(InFlags.begin(), InFlags.end());
   return MS.select(Flags, Selected);
@@ -326,7 +326,7 @@
 TEST(MultilibTest, SelectSoft) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: s
@@ -346,7 +346,7 @@
 TEST(MultilibTest, SelectSoftFP) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: f
@@ -363,7 +363,7 @@
   // with hard float.
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: h
@@ -378,7 +378,7 @@
 TEST(MultilibTest, SelectFloatABI) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: s
@@ -399,11 +399,11 @@
   noMatchFlags: [hasfp]
 )"));
   select({"mfloat-abi=soft"}, MS, MFM, Selected);
-  EXPECT_EQ("/s", Selected.gccSuffix());
+  EXPECT_EQ("/s", Selected.back().gccSuffix());
   select({"mfloat-abi=softfp"}, MS, MFM, Selected);
-  EXPECT_EQ("/f", Selected.gccSuffix());
+  EXPECT_EQ("/f", Selected.back().gccSuffix());
   select({"mfloat-abi=hard"}, MS, MFM, Selected);
-  EXPECT_EQ("/h", Selected.gccSuffix());
+  EXPECT_EQ("/h", Selected.back().gccSuffix());
 }
 
 TEST(MultilibTest, SelectFloatABIReversed) {
@@ -411,7 +411,7 @@
   // selected because soft is compatible with softfp and last wins.
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - d

[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

While we were there, we also dug into `std::any`, and learned that the analyzer 
can model it shockingly well. Hopefully we can submit a few patches that 
demonstrates it in a form of some test files.

In D142354#4079758 , @steakhal wrote:

> I would be interested in some of the free-functions dealing with variants, 
> such as `std::visit()`. https://godbolt.org/z/bbocrz4dG
> I hope that's also on the radar.

Thank you so much!! We definitely intend to work on this issue, and haven't 
thought of it before your suggestion.


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

https://reviews.llvm.org/D142354

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D140756#4099326 , @vedgy wrote:

> Hello Luca and Aaron.
> I have noticed that you recently implemented/reviewed multiple interesting 
> libclang features, some of which can be used in KDevelop. However, 
> `CINDEX_VERSION_MINOR` was last modified in Clang 13. This constant's 
> documentation states:
>
>> `CINDEX_VERSION_MINOR` should increase when there are API additions.
>
> Can this constant be incremented in the upcoming Clang 16 release?

That's a great catch, yes it can. I'll post a patch today to bump it on trunk 
and then file an issue to cherry-pick it over to the 16.x branch. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:243
+/// - Strict
+std::optional> Mode;
+

kadircet wrote:
> sammccall wrote:
> > I think "Diagnostics.Mode" is too vague a name.
> > 
> > I expect this to be a rollout flag that we remove in the medium term 
> > (either deciding to stick to the old or new behavior), so maybe something 
> > ultra-specific like`AllowStalePreamble: yes/no`?
> > (Preamble is jargon, maybe there's something better, but again I don't 
> > think we should plan for this to be really "user-visible")
> i actually feel like there's some value in keeping this around. some users 
> value correctness of their diagnostics too much, and option isn't really 
> costly to implement. why do you think we should stick to one or the other?
> some users value correctness of their diagnostics too much

First, citation needed! (People *claiming* that they value correctness when 
they actually prefer latency seems common based on our past optimizations).

Second, this is a fine-grained knob that probably doesn't yield a useful 
correctness/latency tradeoff. If it trades off a lot of correctness, we 
probably want to leave it off by default and at that point we should delete the 
feature. If it trades off only a little correctness, then most people will want 
it and *disabling it won't significantly improve diagnostics correctness* as 
most errors come from non-configurable tradeoffs made elsewhere.

There's a sweet spot where this trades off just the critical amount of 
correctness to make it a difficult call, I just think it's unlikely we'll hit 
that. (I'm optimistic about this being mostly unnoticeable).

---

Regardless, if this is a long-lived option, it's **more** important to have a 
good name. `AllowStalePreamble` seems clearly better than `Mode` but maybe we 
could come up with something better - I just don't think it matters that much 
if the option is short-lived.



Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:555
+  EXPECT_TRUE(compileAndApply());
+  // Defaults to Strict.
+  EXPECT_EQ(Conf.Diagnostics.Mode, Config::DiagnosticsMode::Fast);

comment doesn't seem to apply here (and below)



Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:584
+
+std::vector mainFileDiagRanges(const ParsedAST &AST) {
+  std::vector Result;

hmm, this convert-then-assert-eq is harder to debug when there are unexpected 
diagnostics than matchers, I think. Might be better to follow what 
DiagnosticTests does here?



Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:628
+  AdditionalFiles["bar.h"] = "#pragma once";
+  llvm::StringLiteral BaselinePreamble("#define FOO 1\n[[#include 
\"foo.h\"]]");
+  {

nit: embedded newlines are hard to read and inconsistent with other tests. 
rawstrings?



Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:634
+NewCode.code(), AdditionalFiles);
+// FIXME: Should be pointing at the range inside the Newcode.
+EXPECT_THAT(mainFileDiagRanges(*AST), IsEmpty());

it's unclear what the behavior is here: do we get a diagnostic with no range, 
or a bad range that's being filtered out, or no diagnostic at all?

(The presence/absence of the diagnostics is important to test, independent of 
the locations)



Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:642
+auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles);
+// FIXME: This is just all sorts of wrong. We point at the FOO from 
baseline
+// claiming it's redefined and not point at the new include of "bar.h".

assert the behavior instead of describing it in a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142890

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

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

Alright, I've done a full reanalysis after a rebase. Overhead is not 
meaningfully measurable, even for complex TUs such as LLVM. The check is viable 
in C++ code as it finds cases (such as the one described in LLVM, but also 
Bitcoin is a primarily C++ project), so I won't rework the check to disable it 
in C++ mode explicitly. It seems the name lookup is implemented pretty well, 
helped by the fact that the names to match are short. No crashes had been 
observed in the test projects; let's hope it stays the same way; the matchers 
themselves are simple enough. The //Annex K.// matcher is only registered if in 
>= C11 mode.
I've also gone through the discussion earlier, and it looks to me as if all the 
concerns were either resolved or made obsolete due to the evolution of the 
check.

I did make several **NFC** changes to the patch in post-production, such as 
fixing the documentation here and there, ensuring that the `cert-` aliases are 
appropriately documented, and a little bit of refactoring so internal details 
of the check are genuinely internal. Thus, I'll be committing a version that is 
different from the one here.


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

https://reviews.llvm.org/D91000

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

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like the test fails on win: http://45.33.8.238/win/74290/step_7.txt

And Mac: http://45.33.8.238/macm1/54080/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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

[clang-tools-extra] f27c8ac - [clang-tidy] Add the `bugprone-unsafe-functions` check

[via cfe-commits]

Author: Gergely Fűtő
Date: 2023-02-02T14:11:42+01:00
New Revision: f27c8ac83e7cb945c8b3f9bf0092f8cf93278b5c

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

LOG: [clang-tidy] Add the `bugprone-unsafe-functions` check

Checks for unsafe functions, mostly those listed in the
SEI CERT C Coding Standard Recommendation `MSC24-C` and Rule `MSC33-C`.

For the listed functions, an alternative, more secure replacement is
suggested, if such is available. The checker heavily relies on the
functions from "Annex K" (Bounds-checking interfaces) from C11, but
there are several other recommendations not directly from Annex K.

Differential Revision: http://reviews.llvm.org/D91000

Reviewed-By: aaron.ballman, dkrupp, steakhal, whisperity

Co-Authored-By: Tamás Koller 
Co-Authored-By: Balázs Benics 
Co-Authored-By: Whisperity 

Added: 
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst
clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Modified: 
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a11db603d5ff0..11b4051668cef 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -71,6 +71,7 @@
 #include "UndelegatedConstructorCheck.h"
 #include "UnhandledExceptionAtNewCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
+#include "UnsafeFunctionsCheck.h"
 #include "UnusedRaiiCheck.h"
 #include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
@@ -200,6 +201,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-unhandled-self-assignment");
 CheckFactories.registerCheck(
 "bugprone-unhandled-exception-at-new");
+CheckFactories.registerCheck(
+"bugprone-unsafe-functions");
 CheckFactories.registerCheck("bugprone-unused-raii");
 CheckFactories.registerCheck(
 "bugprone-unused-return-value");

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 643c62132d875..a975c86fc3970 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -67,6 +67,7 @@ add_clang_library(clangTidyBugproneModule
   UndelegatedConstructorCheck.cpp
   UnhandledExceptionAtNewCheck.cpp
   UnhandledSelfAssignmentCheck.cpp
+  UnsafeFunctionsCheck.cpp
   UnusedRaiiCheck.cpp
   UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
new file mode 100644
index 0..ebb49645b5904
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -0,0 +1,238 @@
+//===--- UnsafeFunctionsCheck.cpp - clang-tidy 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UnsafeFunctionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+using namespace llvm;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static constexpr llvm::StringLiteral OptionNameReportMoreUnsafeFunctions =
+"ReportMoreUnsafeFunctions";
+
+static constexpr llvm::StringLiteral FunctionNamesWithAnnexKReplacementId =
+"FunctionNamesWithAnnexKReplacement";
+static constexpr llvm::StringLiteral FunctionNamesId = "FunctionsNames";
+static constexpr llvm::StringLiteral AdditionalFunctionNamesId =
+"AdditionalFunctionsNames";
+static constexpr llvm::StringLiteral DeclRefId = "DRE";
+
+static std::optional
+getAnnexKReplacementFor(StringRef FunctionName) {
+  return StringSwitch(FunctionName)
+  .Case("strlen", "strnlen_s")
+  .Case("wcslen", "wcsnlen_s")
+  .Default((

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf27c8ac83e7c: [clang-tidy] Add the 
`bugprone-unsafe-functions` check (authored by futogergely, committed by 
whisperity).

Changed prior to commit:
  https://reviews.llvm.org/D91000?vs=485774&id=494265#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst
  clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -0,0 +1,154 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef __WCHAR_TYPE__ wchar_t;
+
+char *gets(char *S);
+size_t strlen(const char *S);
+size_t wcslen(const wchar_t *S);
+
+void f1(char *S) {
+  gets(S);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' should be used instea
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'fgets' should be used instead
+
+  strlen(S);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+void f1w(wchar_t *S) {
+  wcslen(S);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *TimePtr);
+
+void f2(const struct tm *Time) {
+  asctime(Time);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*F1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*F2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K

[PATCH] D143099: [clang][lex] Expose findBeginningOfLine()

[Kyle Edwards via Phabricator via cfe-commits]

KyleFromKitware added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:493-494
 
 /// Returns the pointer that points to the beginning of line that contains
 /// the given offset, or null if the offset if invalid.
+const char *Lexer::findBeginningOfLine(StringRef Buffer, unsigned Offset) {

cor3ntin wrote:
> Should we remove the comment here?
Other `Lexer` methods follow a similar pattern of having doc comments both in 
the header and in the implementation. I think we can leave it.


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

https://reviews.llvm.org/D143099

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

[clang] 79571aa - Bump CINDEX_VERSION_MINOR due to additional APIs

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-02-02T08:49:12-05:00
New Revision: 79571aa2103c95760a07e3549d8636379e4948f0

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

LOG: Bump CINDEX_VERSION_MINOR due to additional APIs

0a51bc731bcc2c27e4fe97957a83642d93d989be added a new API to libclang
but forgot to bump the minor version number.

There is no reasonable way to test this change, hence the lack of test
coverage.

Added: 


Modified: 
clang/include/clang-c/Index.h

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 4e6de219bbac9..a17a97721de26 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -34,7 +34,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 62
+#define CINDEX_VERSION_MINOR 63
 
 #define CINDEX_VERSION_ENCODE(major, minor) (((major)*1) + ((minor)*1))
 



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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D140756#4099410 , @aaron.ballman 
wrote:

> In D140756#4099326 , @vedgy wrote:
>
>> Hello Luca and Aaron.
>> I have noticed that you recently implemented/reviewed multiple interesting 
>> libclang features, some of which can be used in KDevelop. However, 
>> `CINDEX_VERSION_MINOR` was last modified in Clang 13. This constant's 
>> documentation states:
>>
>>> `CINDEX_VERSION_MINOR` should increase when there are API additions.
>>
>> Can this constant be incremented in the upcoming Clang 16 release?
>
> That's a great catch, yes it can. I'll post a patch today to bump it on trunk 
> and then file an issue to cherry-pick it over to the 16.x branch. Thank you!

79571aa2103c95760a07e3549d8636379e4948f0 
 is the 
commit on main and https://github.com/llvm/llvm-project/issues/60478 is for the 
16.x cherry-pick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Oddly enough, one of the buildbots caught a not matching test that was working 
locally... on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Valentin Clement via Phabricator via cfe-commits]

clementval added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

TIFitis wrote:
> clementval wrote:
> > TIFitis wrote:
> > > clementval wrote:
> > > > TIFitis wrote:
> > > > > clementval wrote:
> > > > > > TIFitis wrote:
> > > > > > > clementval wrote:
> > > > > > > > kiranchandramohan wrote:
> > > > > > > > > clementval wrote:
> > > > > > > > > > Instead of copy pasting this from 
> > > > > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > > > > >  can you extract it and put it in a common shared file so 
> > > > > > > > > > bith translation can use the same code without duplication?
> > > > > > > > > @raghavendhra put up a patch some time back and he faced some 
> > > > > > > > > issues. It might be good to check with him or may be he can 
> > > > > > > > > comment here.
> > > > > > > > > https://reviews.llvm.org/D127037
> > > > > > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > > > > > Just moving the three functions should be trivial. I'm not 
> > > > > > > > talking about the processMapOperand.
> > > > > > > I've moved `getSizeInBytes`.
> > > > > > > 
> > > > > > > The other two functions make use of `mlir::Location` and thus 
> > > > > > > can't be moved trivially.
> > > > > > > 
> > > > > > > I can still try to move them by individually passing the elements 
> > > > > > > of `mlir::Location` but that might not be ideal. Is that what 
> > > > > > > you'd like?
> > > > > > What about a new header file in 
> > > > > > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > >  and 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`.
> > > > > >  That should be doable. 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > >  and 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp` 
> > > > > already have access to the common `mlir::Location` type.
> > > > > 
> > > > > Problem is that `OMPIRBuilder.cpp` is the only common file between 
> > > > > them  where I can move the two functions to. Currently there are no 
> > > > > `mlir/**` include files in `OMPIRBuilder.cpp` and it seems to me like 
> > > > > a strict design choice to have it that way.
> > > > The functions can be header only. Why do you need to put them in the 
> > > > OMPIRBuilder.cpp? I think it is better than duplicate the exact same 
> > > > code over. 
> > > Sorry, I misunderstood you earlier.
> > > I've added a new header file 
> > > `mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h`, this is my first 
> > > attempt at adding a new header file, please let me know if you find any 
> > > issues.
> > Thanks! That's what I had in mind. We might want to check with MLIR folks 
> > if `mlir::utils` is suited for that. I don't mind if it is 
> > `mlir::omp::builder` or smth similar since it is related to the 
> > OMPIRBuilder.
> Since the utils file is common to all the dialects I kept it as `mlir::utils`.
> 
> How do I get the opinion from people working in MLIR on this, can you suggest 
> some reviewers whom I can add?
It's only valid for translation to the `llvmir` dialect so that why 
`mlir::utils` seems to generic to me. 

Maybe @ftynse has some thoughts on this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

> 79571aa2103c95760a07e3549d8636379e4948f0 
>  is the 
> commit on main and https://github.com/llvm/llvm-project/issues/60478 is for 
> the 16.x cherry-pick.

Thank you Aaron! But note that this review's commit ended up in Clang 17, not 
Clang 16. Previous API additions have been implemented earlier and are included 
in Clang 16.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[clang] adf7ffd - Revert "[Clang] Add builtin_nondeterministic_value"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2023-02-02T08:59:27-05:00
New Revision: adf7ffd51ee34c3a72d3168f5aed8b946ba3d2cc

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

LOG: Revert "[Clang] Add builtin_nondeterministic_value"

This reverts commit 4a1832a5c801a61bf4c30891aaebe63993712fd9.
Test fail on (at least) macOS and Windows, see
https://reviews.llvm.org/D142388#4099441

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Builtins.def
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp

Removed: 
clang/test/CodeGen/builtins-nondeterministic-value.c



diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index d3a8d958dc7ca..0b533a2ff7596 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3080,32 +3080,6 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
-``__builtin_nondeterministic_value``
-
-
-``__builtin_nondeterministic_value`` returns a valid nondeterministic value of 
the same type as the provided argument.
-
-**Syntax**:
-
-.. code-block:: c++
-
-type __builtin_nondeterministic_value(type x)
-
-**Examples**:
-
-.. code-block:: c++
-
-int x = __builtin_nondeterministic_value(x);
-float y = __builtin_nondeterministic_value(y);
-__m256i a = __builtin_nondeterministic_value(a);
-
-**Description**
-
-Each call to ``__builtin_nondeterministic_value`` returns a valid value of the 
type given by the argument.
-
-The types currently supported are: integer types, floating-point types, vector 
types.
-
-Query for this feature with 
``__has_builtin(__builtin_nondeterministic_value)``.
 
 ``__builtin_sycl_unique_stable_name``
 -

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5f0f3247cf77d..9cc2a72f4c864 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -72,8 +72,6 @@ Non-comprehensive list of changes in this release
 - Clang now saves the address of ABI-indirect function parameters on the stack,
   improving the debug information available in programs compiled without
   optimizations.
-- Clang now supports ``__builtin_nondeterministic_value`` that returns a
-  nondeterministic value of the same type as the provided argument.
 
 New Compiler Flags
 --

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index f401a6b4c62b2..6b54ff7c40e82 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -655,7 +655,6 @@ BUILTIN(__builtin_alloca_uninitialized, "v*z", "Fn")
 BUILTIN(__builtin_alloca_with_align, "v*zIz", "Fn")
 BUILTIN(__builtin_alloca_with_align_uninitialized, "v*zIz", "Fn")
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
-BUILTIN(__builtin_nondeterministic_value, "v.", "nt")
 
 BUILTIN(__builtin_elementwise_abs, "v.", "nct")
 BUILTIN(__builtin_elementwise_max, "v.", "nct")

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 67d55ab3d8c6d..741c2503127af 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13571,8 +13571,6 @@ class Sema final {
   bool PrepareBuiltinElementwiseMathOneArgCall(CallExpr *TheCall);
   bool PrepareBuiltinReduceMathOneArgCall(CallExpr *TheCall);
 
-  bool SemaBuiltinNonDeterministicValue(CallExpr *TheCall);
-
   // Matrix builtin handling.
   ExprResult SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult);

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4a8ca1f918442..bf1c9acc7bec7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3060,15 +3060,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 return RValue::get(V);
   }
 
-  case Builtin::BI__builtin_nondeterministic_value: {
-llvm::Type *Ty = ConvertType(E->getArg(0)->getType());
-
-Value *Result = PoisonValue::get(Ty);
-Result = Builder.CreateFreeze(Result);
-
-return RValue::get(Result);
-  }
-
   case Builtin::BI__builtin_elementwise_abs: {
 Value *Result;
 QualType QT = E->getArg(0)->getType();

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2618b0cd64b20..4602284309491 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2584,12 +2584,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
 break;
   }
 
-  case Bui

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted in adf7ffd51ee34c3a72d3168f5aed8b946ba3d2cc 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

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



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

clementval wrote:
> TIFitis wrote:
> > clementval wrote:
> > > TIFitis wrote:
> > > > clementval wrote:
> > > > > TIFitis wrote:
> > > > > > clementval wrote:
> > > > > > > TIFitis wrote:
> > > > > > > > clementval wrote:
> > > > > > > > > kiranchandramohan wrote:
> > > > > > > > > > clementval wrote:
> > > > > > > > > > > Instead of copy pasting this from 
> > > > > > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > > > > > >  can you extract it and put it in a common shared file so 
> > > > > > > > > > > bith translation can use the same code without 
> > > > > > > > > > > duplication?
> > > > > > > > > > @raghavendhra put up a patch some time back and he faced 
> > > > > > > > > > some issues. It might be good to check with him or may be 
> > > > > > > > > > he can comment here.
> > > > > > > > > > https://reviews.llvm.org/D127037
> > > > > > > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > > > > > > Just moving the three functions should be trivial. I'm not 
> > > > > > > > > talking about the processMapOperand.
> > > > > > > > I've moved `getSizeInBytes`.
> > > > > > > > 
> > > > > > > > The other two functions make use of `mlir::Location` and thus 
> > > > > > > > can't be moved trivially.
> > > > > > > > 
> > > > > > > > I can still try to move them by individually passing the 
> > > > > > > > elements of `mlir::Location` but that might not be ideal. Is 
> > > > > > > > that what you'd like?
> > > > > > > What about a new header file in 
> > > > > > > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > >  and 
> > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`.
> > > > > > >  That should be doable. 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > >  and 
> > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`
> > > > > >  already have access to the common `mlir::Location` type.
> > > > > > 
> > > > > > Problem is that `OMPIRBuilder.cpp` is the only common file between 
> > > > > > them  where I can move the two functions to. Currently there are no 
> > > > > > `mlir/**` include files in `OMPIRBuilder.cpp` and it seems to me 
> > > > > > like a strict design choice to have it that way.
> > > > > The functions can be header only. Why do you need to put them in the 
> > > > > OMPIRBuilder.cpp? I think it is better than duplicate the exact same 
> > > > > code over. 
> > > > Sorry, I misunderstood you earlier.
> > > > I've added a new header file 
> > > > `mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h`, this is my first 
> > > > attempt at adding a new header file, please let me know if you find any 
> > > > issues.
> > > Thanks! That's what I had in mind. We might want to check with MLIR folks 
> > > if `mlir::utils` is suited for that. I don't mind if it is 
> > > `mlir::omp::builder` or smth similar since it is related to the 
> > > OMPIRBuilder.
> > Since the utils file is common to all the dialects I kept it as 
> > `mlir::utils`.
> > 
> > How do I get the opinion from people working in MLIR on this, can you 
> > suggest some reviewers whom I can add?
> It's only valid for translation to the `llvmir` dialect so that why 
> `mlir::utils` seems to generic to me. 
> 
> Maybe @ftynse has some thoughts on this. 
I agree with you on that, would perhaps renaming it to something like 
`mlir::dialect-utils` be a better option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D141389: [DFSAN] Add support for strnlen, strncat, strsep, sscanf and _tolower

[Tomasz Kuchta via Phabricator via cfe-commits]

tkuchta added inline comments.



Comment at: compiler-rt/lib/dfsan/dfsan_custom.cpp:213
+  char *res = strsep(s, delim);
+  s_label = dfsan_read_label(base, strlen(base));
+  if (res && (res != base)) {

browneee wrote:
> tkuchta wrote:
> > browneee wrote:
> > > tkuchta wrote:
> > > > browneee wrote:
> > > > > The `s_label` represents the taint label for `s` (the pointer).
> > > > > 
> > > > > This line would clobber the taint label of the pointer (`s`) with a 
> > > > > taint label from `s[0][0..n]`.
> > > > > 
> > > > > I think this line should be deleted.
> > > > Agree, s_label represents the taint associated with the **s pointer. 
> > > > However I am now wondering if that is the taint wich we would like to 
> > > > return.
> > > > For example, if we have
> > > > if (flags().strict_data_dependencies) {
> > > > *ret_label = res ? s_label : 0;
> > > > 
> > > > We would taint the return value with the value of the pointer, not the 
> > > > data. It means that if we operate on a string for which the characters 
> > > > are tainted, but the pointer itself isn't, we are likely going to 
> > > > return label 0. My understanding was that we are more concerned with 
> > > > the taint of the data, not the pointer, am I missing something?
> > > > 
> > > Yes, we are usually more concerned with the taint of the data, not the 
> > > pointer.
> > > 
> > > With strict dependencies:
> > > // If the input pointer is tainted, the output pointer would be tainted 
> > > (because it is derived from the input pointer - maybe the same value).
> > > taint(s[0]) == dfsan_read_label(s, sizeof(s)) > taint(ret) == 
> > > ret_label[0]
> > > 
> > > // If the input data is tainted, the output data would be tainted 
> > > (because it is derived from the input data).
> > > taint(s[0][0]) == MEM_TO_SHADOW(s[0])[0] > taint(ret[0]) == 
> > > MEM_TO_SHADOW(ret)[0]
> > > 
> > > Because s[0] == ret  (or ret==null), (for the non-null case) the output 
> > > shadow bytes are the same bytes as input shadow bytes and so these taint 
> > > labels for the string data in shadow memory do not need to be explicitly 
> > > propagated in this function. 
> > > 
> > > I think the only case actually changing/copying string data is writing a 
> > > delimiter byte to NULL, which you handled.
> > I am working on the changes and I came across a strange behavior that I 
> > would like to ask about.
> > 
> > It turned out that if we do
> > 
> > char *s = " ... ";
> > dfsan_set_label(label, &p_s, sizeof(&p_s));
> > 
> > Then, the s_label within the handler is 0, not "label". This is unexpected, 
> > as we would like the pointer itself to be labeled here.
> > I believe this has something to do with the fact that the input string in 
> > strsep is a double pointer. For example this works as expected for the 
> > delimiter string, which is a single pointer. 
> > It's either I'm doing something incorrectly or there is some issue with 
> > propagating labels for double pointers.
> > Have you perhaps come across this behavior before?
> I'm not sure what p_s is in your example. Could you provide a more complete 
> example?
> (and maybe all in one function, not half in __dfsw_strsep and half in another 
> function)
> 
> Here is an example showing taint labels at different levels of indirection:
> 
> ```
> #include 
> #include 
> #include 
> 
> int main(void) {
>   char *s = " ... ";
>   char **p_s = &s;
>   char ***pp_s = &p_s;
> 
>   dfsan_label i_label = 1 << 0;
>   dfsan_label j_label = 1 << 1;
>   dfsan_label k_label = 1 << 2;
>   dfsan_label m_label = 1 << 3;
> 
>   // string data
>   dfsan_set_label(i_label, s, strlen(s));
>   // pointer s
>   dfsan_set_label(j_label, &s, sizeof(s));
>   // pointer to pointer s
>   dfsan_set_label(k_label, &p_s, sizeof(p_s));
>   // pointer to pointer to pointer s
>   dfsan_set_label(m_label, &pp_s, sizeof(pp_s));
> 
>   assert(pp_s[0][0] == s);
> 
>   // string data
>   assert(dfsan_read_label(s, strlen(s)) == i_label);
>   // pointer s
>   assert(dfsan_read_label(&s, sizeof(s)) == j_label);
>   // pointer to pointer s
>   assert(dfsan_read_label(&p_s, sizeof(p_s)) == k_label);
>   // pointer to pointer to pointer s
>   assert(dfsan_read_label(&pp_s, sizeof(pp_s)) == m_label);
> 
>   return 0;
> }
> ```
Hello,

Thank you for the comment.

I should have provided a more complete example.
What I meant is a behavior I've found while working on the tests.
In the test file I have something like that:

```
char *s = "Hello world/";
char *delim = " /";
dfsan_set_label(label, &s, sizeof(&s));
char *rv = strep(&s, delim);
 ```

If I get this right, the line 
```
dfsan_set_label(label, &s, sizeof(&s));

``` 
should have applied a taint to the `s` pointer.
However, when inside `strsep`, if we check the `s_label`, it's `0`, not `label`

```
SANITIZER_INTERFACE_ATTRIBUTE char *__dfsw_strsep(char **s, const char *delim,
  dfsan_label s_label,
  

[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D142826#4098014 , @chapuni wrote:

> This discovers a warning in 
> https://reviews.llvm.org/rGa68d4b11465f5b3326be1dd820f59fac275b7581
> I think its checking is valid if size_t is uint32_t (eg. -m32)
>
> Could you guys teach me what the right fix for it? I don't know canonical 
> fixes for it.

This might be pointing out an issue with our existing `-Wtype-limits` 
implementation, as the way I would correct that still gets diagnosed: 
https://godbolt.org/z/sY9PodKbf The only time the comparison is tautological is 
when `size_t` has less width than `uint64_t`, but the extra test for that 
scenario doesn't silence the diagnostic. Even when I try really hard: 
https://godbolt.org/z/PbWcTYaz1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D140756#4099558 , @vedgy wrote:

>> 79571aa2103c95760a07e3549d8636379e4948f0 
>>  is the 
>> commit on main and https://github.com/llvm/llvm-project/issues/60478 is for 
>> the 16.x cherry-pick.
>
> Thank you Aaron! But note that this review's commit ended up in Clang 17, not 
> Clang 16. Previous API additions have been implemented earlier and are 
> included in Clang 16.

Yup, that's why I opened up the issue to backport it into Clang 16. We don't 
usually make edits directly to the branch, instead we put them on main and 
cherry-pick them over to the release branch. Our cherry-picking bot is doing 
the work (you can see it adding comments to 
https://github.com/llvm/llvm-project/issues/60478).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D143091: [clang-format] PackConstructorInitializers support PCIS_OnlyNextLine

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght updated this revision to Diff 494281.
Backl1ght added a comment.

adapt suggestion


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

https://reviews.llvm.org/D143091

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7289,6 +7289,14 @@
  Style);
 verifyFormat("Constructor() : a(a), b(b) {}", Style);
 
+Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+verifyFormat("Constructor()\n"
+ ": (a), b(b) {}",
+ Style);
+verifyFormat("Constructor()\n"
+ ": a(a), b(b) {}",
+ Style);
+
 Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
 Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor()\n"
@@ -7301,6 +7309,11 @@
  "  b(b) {}",
  Style);
 
+Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+verifyFormat("Constructor()\n"
+ ": (a), b(b) {}",
+ Style);
+
 Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
 Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor() :\n"
@@ -7312,6 +7325,11 @@
  "aa(a),\n"
  "b(b) {}",
  Style);
+
+Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+verifyFormat("Constructor() :\n"
+ "aa(a), b(b) {}",
+ Style);
   }
 
   // Test interactions between AllowAllParametersOfDeclarationOnNextLine and
@@ -7319,6 +7337,7 @@
   // BreakConstructorInitializers modes
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.AllowAllParametersOfDeclarationOnNextLine = true;
+  Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
"int , int b)\n"
": (a)\n"
@@ -7333,6 +7352,14 @@
": (a), b(b) {}",
Style);
 
+  Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+  verifyFormat("SomeClassWithALongName::Constructor(\n"
+   "int ,\n"
+   "int b,\n"
+   "int )\n"
+   ": (a), b(b) {}",
+   Style);
+
   Style.AllowAllParametersOfDeclarationOnNextLine = false;
   Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
@@ -7359,6 +7386,14 @@
": (a), b(b) {}",
Style);
 
+  Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+  verifyFormat("SomeClassWithALongName::Constructor(\n"
+   "int ,\n"
+   "int b,\n"
+   "int )\n"
+   ": (a), b(b) {}",
+   Style);
+
   Style.AllowAllParametersOfDeclarationOnNextLine = false;
   Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
@@ -7384,6 +7419,14 @@
"(a), b(b) {}",
Style);
 
+  Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+  verifyFormat("SomeClassWithALongName::Constructor(\n"
+   "int ,\n"
+   "int b,\n"
+   "int ) :\n"
+   "(a), b(b) {}",
+   Style);
+
   Style.AllowAllParametersOfDeclarationOnNextLine = false;
   Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
@@ -7587,6 +7630,16 @@
   "a(aa), aaa() {}",
   Style);
 
+  Style.PackConstructorInitializers = FormatStyle::PCIS_OnlyNextLine;
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  " 

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D142388#4099374 , @ManuelJBrito 
wrote:

> So this is getting some build failures:
>
> - https://lab.llvm.org/buildbot#builders/38/builds/9446
> - https://lab.llvm.org/buildbot#builders/245/builds/4189
> - https://lab.llvm.org/buildbot#builders/65/builds/7949
> - https://lab.llvm.org/buildbot#builders/188/builds/25538
>
> I suspect it's because of the way different ABIs handle the vectors.  I was 
> thinking that if we have the variable be a local variable and also not 
> returning the value can fix the tests.
>
> Does this make sense?

Yep, thats what it looks like, ABI differences.  Feel free to update this 
review with the new test now that you've been reverted, and I'll look to see if 
it seems sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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

[PATCH] D143091: [clang-format] PackConstructorInitializers support PCIS_OnlyNextLine

[Zhikai Zeng via Phabricator via cfe-commits]

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

In D143091#4097534 , 
@HazardyKnusperkeks wrote:

> An entry in the changelog would be nice.

It is already added I think.


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

https://reviews.llvm.org/D143091

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

[PATCH] D142933: Add -print-multi-selection-flags argument

[Joseph Faulls via Phabricator via cfe-commits]

Joe added a comment.



> Thanks for explaining. I found that when I was writing a regex to match flags 
> it was helpful to have a part of the string I could be sure would be there to 
> avoid matching the wrong type of flag, and `march=` helped with that. I also 
> worry that names from different types of flags could clash without some kind 
> of namespacing.

Understandable about the flag names clashing. The scope of riscv multilibs was 
just arch/abi, so there was no concern about clashing/matching the wrong type 
of flag. Would it be weird for one target to have the `march=` but anothers not?




Comment at: clang/lib/Driver/Driver.cpp:2213
+  if (C.getArgs().hasArg(options::OPT_print_multi_selection_flags)) {
+for (StringRef Attr : TC.getMultiSelectionFlags(C.getArgs()))
+  llvm::outs() << Attr << '\n';

Do we want to parse the multilib.yaml here so we can print out custom flags as 
well? It could help diagnose issues people have with them.



Comment at: clang/lib/Driver/ToolChain.cpp:204
+Result.push_back(
+clang::driver::getDriverOptTable().getOptionName(Option).str());
+  }

> 
>> the form `x=y` is already broken when you add the flags from the flag list.
> 
> Can you give an example of what you mean by that? Sounds like something that 
> might need fixing.
For example the option name for OPT_fexceptions is just `fexceptions`, and this 
is added directly to `Results`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Alright, right now, I have no meaningful idea why this failure appears 
,
 locally I'm trying every test command as they appear in the test, and all the 
tests are passing. It's as if on that system the whole //Annex K// support 
would not be allowed. Locally I added a few debug prints and I'm getting the 
right answers for //"Whether Annex K is allowed?"//.

@njames93 @aaron.ballman Do you have any idea what could make that test fail in 
that system?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

I meant that the commit message of 
https://reviews.llvm.org/rG79571aa2103c95760a07e3549d8636379e4948f0 
misleadingly refers to this review's commit. But `CINDEX_VERSION_MINOR == 63` 
is for previous commits. This commit will require incrementing 
`CINDEX_VERSION_MINOR` again to `64` in Clang 17. Hopefully my preamble-storage 
patches will also be included in Clang 17 and share the same minor version `64` 
:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D140415: [flang] stack arrays pass

[Tom Eccles via Phabricator via cfe-commits]

tblah updated this revision to Diff 494283.
tblah marked 8 inline comments as done.
tblah added a comment.

Changes: fix nits from review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140415

Files:
  clang/docs/tools/clang-formatted-files.txt
  flang/include/flang/Optimizer/Builder/MutableBox.h
  flang/include/flang/Optimizer/Dialect/FIRAttr.h
  flang/include/flang/Optimizer/Transforms/Passes.h
  flang/include/flang/Optimizer/Transforms/Passes.td
  flang/lib/Lower/Allocatable.cpp
  flang/lib/Optimizer/Builder/MutableBox.cpp
  flang/lib/Optimizer/Transforms/CMakeLists.txt
  flang/lib/Optimizer/Transforms/StackArrays.cpp
  flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90
  flang/test/Lower/Intrinsics/c_loc.f90
  flang/test/Lower/Intrinsics/system_clock.f90
  flang/test/Transforms/stack-arrays.f90
  flang/test/Transforms/stack-arrays.fir

Index: flang/test/Transforms/stack-arrays.fir
===
--- /dev/null
+++ flang/test/Transforms/stack-arrays.fir
@@ -0,0 +1,309 @@
+// RUN: fir-opt --stack-arrays %s | FileCheck %s
+
+// Simplest transformation
+func.func @simple() {
+  %0 = fir.allocmem !fir.array<42xi32>
+  fir.freemem %0 : !fir.heap>
+  return
+}
+// CHECK: func.func @simple() {
+// CHECK-NEXT: fir.alloca !fir.array<42xi32>
+// CHECK-NEXT: return
+// CHECK-NEXT: }
+
+// Check fir.must_be_heap allocations are not moved
+func.func @must_be_heap() {
+  %0 = fir.allocmem !fir.array<42xi32> {fir.must_be_heap = true}
+  fir.freemem %0 : !fir.heap>
+  return
+}
+// CHECK:  func.func @must_be_heap() {
+// CHECK-NEXT:   %[[ALLOC:.*]] = fir.allocmem !fir.array<42xi32> {fir.must_be_heap = true}
+// CHECK-NEXT:   fir.freemem %[[ALLOC]] : !fir.heap>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// Check the data-flow-analysis can detect cases where we aren't sure if memory
+// is freed by the end of the function
+func.func @dfa1(%arg0: !fir.ref> {fir.bindc_name = "cond"}) {
+  %7 = arith.constant 42 : index
+  %8 = fir.allocmem !fir.array, %7 {uniq_name = "_QFdfa1Earr.alloc"}
+  %9 = fir.load %arg0 : !fir.ref>
+  %10 = fir.convert %9 : (!fir.logical<4>) -> i1
+  fir.if %10 {
+fir.freemem %8 : !fir.heap>
+  } else {
+  }
+  return
+}
+// CHECK:  func.func @dfa1(%arg0: !fir.ref> {fir.bindc_name = "cond"}) {
+// CHECK-NEXT:   %[[C42:.*]] = arith.constant 42 : index
+// CHECK-NEXT:   %[[MEM:.*]] = fir.allocmem !fir.array, %[[C42]] {uniq_name = "_QFdfa1Earr.alloc"}
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.load %arg0 : !fir.ref>
+// CHECK-NEXT:   %[[BOOL:.*]] = fir.convert %[[LOGICAL]] : (!fir.logical<4>) -> i1
+// CHECK-NEXT:   fir.if %[[BOOL]] {
+// CHECK-NEXT: fir.freemem %[[MEM]] : !fir.heap>
+// CHECK-NEXT:   } else {
+// CHECK-NEXT:   }
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// Check scf.if (fir.if is not considered a branch operation)
+func.func @dfa2(%arg0: i1) {
+  %a = fir.allocmem !fir.array<1xi8>
+  scf.if %arg0 {
+fir.freemem %a : !fir.heap>
+  } else {
+  }
+  return
+}
+// CHECK: func.func @dfa2(%arg0: i1) {
+// CHECK-NEXT:  %[[MEM:.*]] = fir.allocmem !fir.array<1xi8>
+// CHECK-NEXT:  scf.if %arg0 {
+// CHECK-NEXT:fir.freemem %[[MEM]] : !fir.heap>
+// CHECK-NEXT:  } else {
+// CHECK-NEXT:  }
+// CHECK-NEXT:  return
+// CHECK-NEXT:  }
+
+// check the alloca is placed after all operands become available
+func.func @placement1() {
+  // do some stuff with other ssa values
+  %1 = arith.constant 1 : index
+  %2 = arith.constant 2 : index
+  %3 = arith.addi %1, %2 : index
+  // operand is now available
+  %4 = fir.allocmem !fir.array, %3
+  // ...
+  fir.freemem %4 : !fir.heap>
+  return
+}
+// CHECK:  func.func @placement1() {
+// CHECK-NEXT:   %[[ONE:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TWO:.*]] = arith.constant 2 : index
+// CHECK-NEXT:   %[[ARG:.*]] = arith.addi %[[ONE]], %[[TWO]] : index
+// CHECK-NEXT:   %[[MEM:.*]] = fir.alloca !fir.array, %[[ARG]]
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// check that if there are no operands, then the alloca is placed early
+func.func @placement2() {
+  // do some stuff with other ssa values
+  %1 = arith.constant 1 : index
+  %2 = arith.constant 2 : index
+  %3 = arith.addi %1, %2 : index
+  %4 = fir.allocmem !fir.array<42xi32>
+  // ...
+  fir.freemem %4 : !fir.heap>
+  return
+}
+// CHECK:  func.func @placement2() {
+// CHECK-NEXT:   %[[MEM:.*]] = fir.alloca !fir.array<42xi32>
+// CHECK-NEXT:   %[[ONE:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TWO:.*]] = arith.constant 2 : index
+// CHECK-NEXT:   %[[SUM:.*]] = arith.addi %[[ONE]], %[[TWO]] : index
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// check that stack allocations which must be placed in loops use stacksave
+func.func @placement3() {
+  %c1 = arith.constant 1 : index
+  %c1_i32 = fir.convert %c1 : (index) -> i32
+  %c2 = arith.constant 2 : index
+  %c10 = arith.constant 10 : in

[PATCH] D140415: [flang] stack arrays pass

[Tom Eccles via Phabricator via cfe-commits]

tblah added a comment.

In D140415#4098170 , 
@kiranchandramohan wrote:

> Looks OK. I have a few questions and some minor comments inline. It might be 
> good to pull in a bit of info from the RFC into the Summary, particularly 
> saying why a dataflow analysis is necessary, what operations are involved in 
> the analysis etc.
>
> Could we have used the Dominance and PostDominance information to find out 
> the Allocs and Frees that could have been replaced? I saw the following 
> functions for individual Ops but not for the case where a set of ops 
> dominates or post-dominates. So may be not with the existing infra.
>
>   bool DominanceInfo::properlyDominatesImpl(Operation *a, Operation *b
>   bool PostDominanceInfo::properlyPostDominates(Operation *a, Operation *b) {
>
> I guess, we are not capturing the following because of different values.
>
>   module {
> func.func @dfa2(%arg0: i1) {
>   cf.cond_br %arg0, ^bb1, ^bb2
> ^bb1:  // pred: ^bb0
>   %a = fir.allocmem !fir.array<1xi8>
>   cf.br ^bb3(%a : !fir.heap>)
> ^bb2:  // pred: ^bb0
>   %b = fir.allocmem !fir.array<1xi8>
>   cf.br ^bb3(%b : !fir.heap>)
> ^bb3(%0: !fir.heap>):  // 2 preds: ^bb1, ^bb2
>   fir.freemem %0 : !fir.heap>
>   return
> }
>   }

Yes we could have used Dominance and PostDominance information to find out if 
an allocation is always freed. I wasn't aware of `mlir::DominanceInfo` at the 
time I wrote this patch. As It is already written, I think the data flow 
analysis continues to be the correct approach because it will skip dead code 
(after constant propagation) and I suspect the worst case algorithmic 
complexity is better than computing dominance between each heap allocation and 
free.

Yes in that case we cannot detect that the allocation is freed because the free 
operates on a different SSA value to the allocations. This would have been a 
problem whether `mlir::DominanceInfo` or `mlir::DataFlowAnalysis` were used. I 
chose not to support allocations and frees using different SSA values as this 
would have added considerable complexity and is not necessary for the more 
common cases of Flang-generated allocations. See the RFC for details.




Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:532
+
+  // Find when the last operand value becomes available
+  mlir::Block *operandsBlock = nullptr;

kiranchandramohan wrote:
> Might be worth checking whether we have a function for this in MLIR core.
Not that I can find. The MLIR verifier checks that all operation arguments 
properly dominate the operation, but this is done by comparing each in turn 
against the operation: no last operand is found.

I could use mlir::DominanceInfo to find when the last operand becomes 
available, which I guess would better handle the case where operands are 
defined in different blocks. But dominance only provides a partial ordering so 
there might be cases where `domInfo.properlyDominates(arg1, arg2) == 
domInfo.properlyDominates(arg2, arg1) == false`. Looking at the direct 
operation ordering only within the same block (as I do here) guarantees a total 
ordering relationship.



Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:545-547
+  // Operation::isBeforeInBlock requires the operations to be in the same
+  // block. The best we can do is the location of the allocmem.
+  return checkReturn(oldAlloc.getOperation());

kiranchandramohan wrote:
> Theoretically speaking, we can use the dominance info to determine whether 
> one block dominates the other as well to handle cases like the following 
> where we are finding the operands of `func`. But I guess that is probably not 
> required.
> ```
> b1:
> x = opA
> br b2
> b2:
> y = opB
> br b3
> b3:
> z = func(x,y)
> ```
> 
Thank you for pointing out `mlir::DominanceInfo` - I was not aware of that 
analysis. I propose we keep this pass as it is for now, to avoid adding more 
complexity where we don't have a concrete example of flang-generated 
allocations which need to support alloca arguments defined in different blocks.



Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:560
+
lastOperand->getParentOfType();
+if (lastOpOmpRegion == oldOmpRegion)
+  return checkReturn(lastOperand);

kiranchandramohan wrote:
> Do we have a test for this, and in general for the OpenMP handling?
When writing the tests I discovered that the data flow analysis does not 
propagate lattices into or out of an omp.section, so currently no allocations 
inside of an openmp secton will be moved to the stack.

I intend to handle this in a subsequent patch. In the meantime I have added a 
test to make sure that allocations in an openmp region are not moved.



Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:732
+  mlir::applyPartialConversion(func, target

[clang-tools-extra] ed740e7 - [clang-tidy] Attempt fixing wrong caching result in `bugprone-unsafe-functions`

[via cfe-commits]

Author: Whisperity
Date: 2023-02-02T15:24:40+01:00
New Revision: ed740e741ec22f9aaea09bfc0b87d0801a7c492f

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

LOG: [clang-tidy] Attempt fixing wrong caching result in 
`bugprone-unsafe-functions`

There is a supposedly platform-specific crash related to not recognising
the availability of *Annex K.* properly? This patch is an attempt for
fixing this by moving the reset logic for the cache to a different
place.

It's really a coin-flip at this point whether this is really a fix...

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index ebb49645b5904..c069cc3ea33ad 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -226,10 +226,6 @@ void UnsafeFunctionsCheck::registerPPCallbacks(
 const SourceManager &SM, Preprocessor *PP,
 Preprocessor * /*ModuleExpanderPP*/) {
   this->PP = PP;
-}
-
-void UnsafeFunctionsCheck::onEndOfTranslationUnit() {
-  this->PP = nullptr;
   IsAnnexKAvailable.reset();
 }
 

diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
index 369ea25f693cc..f04a220fd12e2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
@@ -32,7 +32,6 @@ class UnsafeFunctionsCheck : public ClangTidyCheck {
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
-  void onEndOfTranslationUnit() override;
 
 private:
   /// If true, additional functions from widely used API-s (such as POSIX) are

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
index e1f8238fd4f3f..62754fa68111b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -15,8 +15,8 @@ size_t wcslen(const wchar_t *S);
 
 void f1(char *S) {
   gets(S);
-  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instead
-  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instea
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instead [bugprone-unsafe-functions]
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instead
   // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'fgets' should 
be used instead
 
   strlen(S);



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

[PATCH] D143093: [clangd] #undef macros inside preamble patch

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I can't understand from the description, code, or testcases what problem this 
is fixing. Can you clarify, ideally by improving the testcases?

It seems to introduce a false negative in the case that the preamble *does* 
contain a definition of an already-defined macro, which probably needs to be 
called out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143093

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

[PATCH] D143178: Add new clang-format alignment option

[Philip Herron via Phabricator via cfe-commits]

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

I work on a code base that aligns all declarations execpt parameters to
functions. This patch adds a new option to AlignConsecutiveDeclarations.

TODO I need to add unit-tests for this option but this would be really useful 
for me
but given its a new option I have not done this yet just to get feedback 
whether this 
option would get accepted or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143178

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp

Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -870,7 +870,7 @@
 
   AlignTokens(
   Style,
-  [](Change const &C) {
+  [&](Change const &C) {
 if (C.Tok->is(TT_FunctionDeclarationName))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
@@ -878,6 +878,15 @@
 if (C.Tok->Previous &&
 C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
   return false;
+
+if (C.Tok->is(TT_StartOfName)) {
+  bool isParam = C.Tok->Next && C.Tok->Next->is(clang::tok::comma);
+  bool shouldAlignParams =
+  Style.AlignConsecutiveDeclarations.AcrossParameterDeclarations;
+  if (isParam && !shouldAlignParams)
+return false;
+}
+
 // Check if there is a subsequent name that starts the same declaration.
 for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
   if (Next->is(tok::comment))
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -12,7 +12,6 @@
 ///
 //===--===//
 
-#include "clang/Format/Format.h"
 #include "AffectedRangeManager.h"
 #include "BreakableToken.h"
 #include "ContinuationIndenter.h"
@@ -33,6 +32,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
 #include "llvm/ADT/STLExtras.h"
@@ -65,29 +65,44 @@
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AcrossParameterDeclarations*/ true,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "Consecutive",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AcrossParameterDeclarations*/ true,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossEmptyLines",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/true,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AcrossParameterDeclarations*/ true,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/false,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
+FormatStyle::AlignConsecutiveStyle(
+{/*Enabled=*/true,
+ /*AcrossEmptyLines=*/false,
+ /*AcrossComments=*/true,
+ /*AlignCompound=*/false,
+ /*AcrossParameterDeclarations*/ true,
+ /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossEmptyLinesAndComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/true,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
+FormatStyle::AlignConsecutiveStyle(
+{/*Enabled=*/true,
+ /*AcrossEmptyLines=*/true,
+ /*AcrossComments=*/true,
+ /*

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

whisperity added a subscriber: dyung.
whisperity added a comment.

Ping @dyung, it looks like you're the owner of the relevant build-bot. I can't 
find any information what `x86_64-sie` is...

Meanwhile, it seems my attempt at fix is not actually fixing anything, I'm 
trying to figure out how to at least disable the check on this specific 
architecture. The rest of the architectures seem to be passing normally as 
expected. Something must be special within Clang's default environment or 
compiler configuration...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D141666: [RISCV] Proper support of extensions Zicsr and Zifencei

[Elena Lepilkina via Phabricator via cfe-commits]

eklepilkina added a comment.

Gently ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141666

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

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

It looks like this fixes up the location only of diagnostics attached to 
particular directives (`#include`) based on some "deep" idea about the content 
of the directive (the spelled header name).

Some shortcomings:

- This misses diagnostics attached to other directives / continued lines in 
macro definitions / etc
- it allows diagnostics to be translated across lines even if the directive 
spelling changed (in which case the ranges are incorrect since only line 
numbers are updated.)
- it requires modelling the directive content in some way that needs to be 
extended if we find another place that diagnostics can be attached

We discussed the idea of attaching to the text of the line, which seems pretty 
generic. Needs some handling of duplicate line content but seems like something 
pretty naive (closest line number with matching content?) would work well, be 
just as simple and more generic. Any reason this alternative was rejected?




Comment at: clang-tools-extra/clangd/Preamble.h:162
+  /// to an #include or #define directive.
+  std::vector patchedDiags() const { return PatchedDiags; }
   static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h";

this looks like a gratuitous copy - ParsedAST copies again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143096

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

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

[Corentin Jabot via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/Parser.cpp:1415
+//
+// FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+// and PopExpressionEvaluationContext().

ChuanqiXu wrote:
> cor3ntin wrote:
> > shafik wrote:
> > > It does seem a bit ad-hoc
> > could we use `ExitFunctionBodyRAII` here, maybe ? It already deals with 
> > lambdas
> > It does seem a bit ad-hoc
> 
> I agree. I feel the current explicit style for pushing and popping different 
> contexts looks not easy to maintain... I just want to prevent the crash for 
> now and I feel the patch wouldn't be a burden when someday we want to 
> refactor this.
> 
> > could we use ExitFunctionBodyRAII here, maybe ? It already deals with 
> > lambdas
> 
> Maybe. But it wouldn't be simpler. We need to move `ExitFunctionBodyRAII` and 
> we need to write:
> 
> ```
> ExitFunctionBodyRAII ExitRAII(Actions, 
> isLambdaCallOperator(dyn_cast_if_present(Res)));
> return Res;
> ```
> 
> which looks odd and unnecessary.
> 
> And if you're saying something like:
> 
> ```
> Decl *Res = Actions.ActOnStartOfFunctionDef(getCurScope(), D,
>   TemplateInfo.TemplateParams
>   ? 
> *TemplateInfo.TemplateParams
>   : MultiTemplateParamsArg(),
>   &SkipBody, BodyKind);
> 
> 
> ExitFunctionBodyRAII ExitRAII(Actions, 
> isLambdaCallOperator(dyn_cast_if_present(Res)));
> ```
> 
> and remove the ExitFunctionBodyRAII in `ActOnFinishFunctionBody`. I guess it 
> can't work in this way simply. Since the comment 
> (https://github.com/llvm/llvm-project/blob/c6795b1d37cee586d9b98dade64432f8f6bd004b/clang/lib/Sema/SemaDecl.cpp#L15811-L15818)
>  says we need to pop expression evaluation context before popping decl 
> context (and function scope info?).
I think you are right, your change is probably as good as anything without 
doing a major refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143053

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

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D140756#4099593 , @vedgy wrote:

> I meant that the commit message of 
> https://reviews.llvm.org/rG79571aa2103c95760a07e3549d8636379e4948f0 
> misleadingly refers to this review's commit. But `CINDEX_VERSION_MINOR == 63` 
> is for previous commits. This commit will require incrementing 
> `CINDEX_VERSION_MINOR` again to `64` in Clang 17. Hopefully my 
> preamble-storage patches will also be included in Clang 17 and share the same 
> minor version `64` :)

Hmmm, I thought 0a51bc731bcc2c27e4fe97957a83642d93d989be 
 landed 
before we did the branch (and so was the last change in Clang 16) and we don't 
have any new CIndex commits since then to bump it to 64 (yet). However, now I 
see that it almost made the branch but was reverted 
https://github.com/llvm/llvm-project/commit/1af716499d6bc29afd9ff2903200890df774859f
 (though we had other changes in Clang 16 justifying the version bump, such as 
5c67cf0a7fdc00c9b9c55578b770e768f5618bed 
).

So I think you're right, we need one more commit on main to bump to `64` 
because of 0a51bc731bcc2c27e4fe97957a83642d93d989be 
. And yes, 
I think the preamble storage patches will be in Clang 17 and share the same 
minor version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D91000#4099657 , @whisperity wrote:

> Ping @dyung, it looks like you're the owner of the relevant build-bot. I 
> can't find any information what `x86_64-sie` is...
>
> Meanwhile, it seems my attempt at fix is not actually fixing anything, I'm 
> trying to figure out how to at least disable the check on this specific 
> architecture. The rest of the architectures seem to be passing normally as 
> expected. Something must be special within Clang's default environment or 
> compiler configuration...

Sorry responses from me might be delayed today as I am on holiday, but that 
build bot builds with PS4 as the default target. This is the cmake line that is 
used:

  cmake ../llvm-project/llvm -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ 
-DCMAKE_BUILD_TYPE=Release -DCLANG_ENABLE_ARCMT=OFF -DCLANG_ENABLE_CLANGD=OFF 
-DLLVM_BUILD_RUNTIME=OFF -DLLVM_CCACHE_BUILD=ON -DLLVM_INCLUDE_EXAMPLES=OFF 
-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4 -DLLVM_ENABLE_ASSERTIONS=ON 
'-DLLVM_LIT_ARGS=--verbose -j100' -DLLVM_TARGETS_TO_BUILD=X86 
-DLLVM_USE_LINKER=gold 
'-DLLVM_ENABLE_PROJECTS=clang;cross-project-tests;llvm;clang-tools-extra;lld' 
-GNinja

Hopefully this can help you to reproduce the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D139704: [clang][RISCV] Added target attributes to runtime functions

[Elena Lepilkina via Phabricator via cfe-commits]

eklepilkina added a comment.

Gently ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139704

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D91000#4099669 , @dyung wrote:

>   -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4
>
> Hopefully this can help you to reproduce the problem.

Ah, thank you very much. And indeed, giving `--target=x86_64-scei-ps4` to 
Clang-Tidy will make the test case fail, but using `x86_64-linux-gnu` works 
perfectly.
Now knowing the exact triple I think I can delve into the preprocessor stack or 
something to figure out why the //Annex K.//-related flags, my best bet is that 
it's explicitly forbidden from being defined. 😉

  clang-tidy 
/tmp/LLVM-Build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/unsafe-functions.c.tmp.c
 --checks='-*,bugprone-unsafe-functions' -config={} -- 
--target=x86_64-linux-gnu -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 
-nostdinc++

My first wish is to figure out how to reliably disable the test case and have 
this quick-fix pushed immediately so the buildbots don't continue failing, and 
then we can figure out that this check should be disabled on this specific 
platform, or something like that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D91000#4099744 , @whisperity wrote:

> In D91000#4099669 , @dyung wrote:
>
>>   -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4
>>
>> Hopefully this can help you to reproduce the problem.
>
> Ah, thank you very much. And indeed, giving `--target=x86_64-scei-ps4` to 
> Clang-Tidy will make the test case fail, but using `x86_64-linux-gnu` works 
> perfectly.
> Now knowing the exact triple I think I can delve into the preprocessor stack 
> or something to figure out why the //Annex K.//-related flags, my best bet is 
> that it's explicitly forbidden from being defined. 😉
>
>   clang-tidy 
> /tmp/LLVM-Build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/unsafe-functions.c.tmp.c
>  --checks='-*,bugprone-unsafe-functions' -config={} -- 
> --target=x86_64-linux-gnu -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 
> -nostdinc++
>
> My first wish is to figure out how to reliably disable the test case and have 
> this quick-fix pushed immediately so the buildbots don't continue failing, 
> and then we can figure out that this check should be disabled on this 
> specific platform, or something like that...

One particular oddity of our platform is that we use a different default C++ 
and I think C standard, so if explicitly setting the C standard causes the test 
to pass, that is likely the case.

To XFAIL the test, grep for lines with "XFAIL" and "ps4" and you should find 
some examples. It was recently changed how it worked lately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D143094: [clang] Change AMX macros to match names from GCC

[Joe Loser via Phabricator via cfe-commits]

jloser added a comment.

In D143094#4098208 , @LuoYuanke wrote:

> LGTM, thanks

Thanks for the review.  Do I need to wait for another approver before landing 
this?  I mostly work on `libc++`, so not too familiar with expectations on 
Clang reviews, or whether one approver who's familiar/knowledgable in the area 
is sufficient.

I'm hoping to land this soon and request to cherry-pick it into LLVM 16 to 
avoid the macro value changing across 16/17 releases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143094

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

[PATCH] D143160: [include-mapping] Extend c-compatibility symbols in StdSymbolMap.inc

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 494299.
hokein added a comment.

update, separate out a new file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143160

Files:
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
  clang/tools/include-mapping/gen_std.py

Index: clang/tools/include-mapping/gen_std.py
===
--- clang/tools/include-mapping/gen_std.py
+++ clang/tools/include-mapping/gen_std.py
@@ -102,7 +102,30 @@
 exit("Path %s doesn't exist!" % symbol_index_root)
 
   symbols = cppreference_parser.GetSymbols(parse_pages)
-  
+  # C++ form of the C standard headers.
+  c_style_headers = {
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+  }
   # We don't have version information from the unzipped offline HTML files.
   # so we use the modified time of the symbol_index.html as the version.
   index_page_path = os.path.join(page_root, "index.html")
@@ -111,6 +134,10 @@
   print(CODE_PREFIX % (args.symbols.upper(), cppreference_modified_date))
   for symbol in symbols:
 if len(symbol.headers) == 1:
+  # Excluding the c-compatibility symbols, they are covered by the
+  # human-edit CXXSymbolMap.inc file.
+  if symbol.headers[0] in c_style_headers:
+continue
   # SYMBOL(unqualified_name, namespace, header)
   print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
 symbol.headers[0]))
Index: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
===
--- clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
+++ clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
@@ -9,15 +9,7 @@
 // Generated from cppreference offline HTML book (modified on 2022-07-30).
 //===--===//
 
-SYMBOL(FILE, std::, )
-SYMBOL(_Exit, std::, )
 SYMBOL(accumulate, std::, )
-SYMBOL(acos, std::, )
-SYMBOL(acosf, std::, )
-SYMBOL(acosh, std::, )
-SYMBOL(acoshf, std::, )
-SYMBOL(acoshl, std::, )
-SYMBOL(acosl, std::, )
 SYMBOL(add_const, std::, )
 SYMBOL(add_const_t, std::, )
 SYMBOL(add_cv, std::, )
@@ -38,7 +30,6 @@
 SYMBOL(advance, std::, )
 SYMBOL(align, std::, )
 SYMBOL(align_val_t, std::, )
-SYMBOL(aligned_alloc, std::, )
 SYMBOL(aligned_storage, std::, )
 SYMBOL(aligned_storage_t, std::, )
 SYMBOL(aligned_union, std::, )
@@ -62,37 +53,9 @@
 SYMBOL(as_bytes, std::, )
 SYMBOL(as_const, std::, )
 SYMBOL(as_writable_bytes, std::, )
-SYMBOL(asctime, std::, )
-SYMBOL(asin, std::, )
-SYMBOL(asinf, std::, )
-SYMBOL(asinh, std::, )
-SYMBOL(asinhf, std::, )
-SYMBOL(asinhl, std::, )
-SYMBOL(asinl, std::, )
 SYMBOL(assignable_from, std::, )
-SYMBOL(assoc_laguerre, std::, )
-SYMBOL(assoc_laguerref, std::, )
-SYMBOL(assoc_laguerrel, std::, )
-SYMBOL(assoc_legendre, std::, )
-SYMBOL(assoc_legendref, std::, )
-SYMBOL(assoc_legendrel, std::, )
 SYMBOL(assume_aligned, std::, )
 SYMBOL(async, std::, )
-SYMBOL(at_quick_exit, std::, )
-SYMBOL(atan, std::, )
-SYMBOL(atan2, std::, )
-SYMBOL(atan2f, std::, )
-SYMBOL(atan2l, std::, )
-SYMBOL(atanf, std::, )
-SYMBOL(atanh, std::, )
-SYMBOL(atanhf, std::, )
-SYMBOL(atanhl, std::, )
-SYMBOL(atanl, std::, )
-SYMBOL(atexit, std::, )
-SYMBOL(atof, std::, )
-SYMBOL(atoi, std::, )
-SYMBOL(atol, std::, )
-SYMBOL(atoll, std::, )
 SYMBOL(atomic_compare_exchange_strong, std::, )
 SYMBOL(atomic_compare_exchange_strong_explicit, std::, )
 SYMBOL(atomic_compare_exchange_weak, std::, )
@@ -176,9 +139,6 @@
 SYMBOL(basic_stringstream, std::, )
 SYMBOL(basic_syncbuf, std::, )
 SYMBOL(bernoulli_distribution, std::, )
-SYMBOL(beta, std::, )
-SYMBOL(betaf, std::, )
-SYMBOL(betal, std::, )
 SYMBOL(bidirectional_iterator, std::, )
 SYMBOL(bidirectional_iterator_tag, std::, )
 SYMBOL(binary_search, std::, )
@@ -200,22 +160,9 @@
 SYMBOL(boolalpha, std::, )
 SYMBOL(boyer_moore_horspool_searcher, std::, )
 SYMBOL(boyer_moore_searcher, std::, )
-SYMBOL(bsearch, std::, )
-SYMBOL(btowc, std::, )
-SYMBOL(byte, std::, )
 SYMBOL(byteswap, std::, )
-SYMBOL(c16rtomb, std::, )
-SYMBOL(c32rtomb, std::, )
-SYMBOL(c8rtomb, std::, )
 SYMBOL(call_once, std::, )
-SYMBOL(calloc, std::, )
 SYMBOL(cauchy_distribution, std::, )
-SYMBOL(cbrt, std::, )
-SYMBOL(cbrtf, std::, )
-SYMBOL(cbrtl, std::, )
-SYMBOL(ceil, std::, )
-SYMBOL(ceilf, std::, )
-SYMBOL(ceill, std::, )
 SYMBOL(centi, std::, )
 SYMBOL(cerr, std::, )
 SYMBOL(char_traits, std::, )
@@ -223,9 +170,6 @@
 SYMBOL(chi_squared_distribution, std::, )
 SYMBOL(cin, std::, )
 SYMBOL(clamp, std::, )
-SYMBOL(clearerr, std::, )
-SYMBOL(clock, std::, )
-SYMBOL(clock_t, std::, )
 SYMBOL(clog, std::, )
 SYMBOL(cmatch, std::, )
 

[PATCH] D143160: [include-mapping] Extend c-compatibility symbols in StdSymbolMap.inc

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 494300.
hokein added a comment.

upload the missing file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143160

Files:
  clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
  clang/tools/include-mapping/gen_std.py

Index: clang/tools/include-mapping/gen_std.py
===
--- clang/tools/include-mapping/gen_std.py
+++ clang/tools/include-mapping/gen_std.py
@@ -102,7 +102,30 @@
 exit("Path %s doesn't exist!" % symbol_index_root)
 
   symbols = cppreference_parser.GetSymbols(parse_pages)
-  
+  # C++ form of the C standard headers.
+  c_style_headers = {
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+'',
+  }
   # We don't have version information from the unzipped offline HTML files.
   # so we use the modified time of the symbol_index.html as the version.
   index_page_path = os.path.join(page_root, "index.html")
@@ -111,6 +134,10 @@
   print(CODE_PREFIX % (args.symbols.upper(), cppreference_modified_date))
   for symbol in symbols:
 if len(symbol.headers) == 1:
+  # Excluding the c-compatibility symbols, they are covered by the
+  # human-edit CXXSymbolMap.inc file.
+  if symbol.headers[0] in c_style_headers:
+continue
   # SYMBOL(unqualified_name, namespace, header)
   print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
 symbol.headers[0]))
Index: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
===
--- clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
+++ clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
@@ -9,15 +9,7 @@
 // Generated from cppreference offline HTML book (modified on 2022-07-30).
 //===--===//
 
-SYMBOL(FILE, std::, )
-SYMBOL(_Exit, std::, )
 SYMBOL(accumulate, std::, )
-SYMBOL(acos, std::, )
-SYMBOL(acosf, std::, )
-SYMBOL(acosh, std::, )
-SYMBOL(acoshf, std::, )
-SYMBOL(acoshl, std::, )
-SYMBOL(acosl, std::, )
 SYMBOL(add_const, std::, )
 SYMBOL(add_const_t, std::, )
 SYMBOL(add_cv, std::, )
@@ -38,7 +30,6 @@
 SYMBOL(advance, std::, )
 SYMBOL(align, std::, )
 SYMBOL(align_val_t, std::, )
-SYMBOL(aligned_alloc, std::, )
 SYMBOL(aligned_storage, std::, )
 SYMBOL(aligned_storage_t, std::, )
 SYMBOL(aligned_union, std::, )
@@ -62,37 +53,9 @@
 SYMBOL(as_bytes, std::, )
 SYMBOL(as_const, std::, )
 SYMBOL(as_writable_bytes, std::, )
-SYMBOL(asctime, std::, )
-SYMBOL(asin, std::, )
-SYMBOL(asinf, std::, )
-SYMBOL(asinh, std::, )
-SYMBOL(asinhf, std::, )
-SYMBOL(asinhl, std::, )
-SYMBOL(asinl, std::, )
 SYMBOL(assignable_from, std::, )
-SYMBOL(assoc_laguerre, std::, )
-SYMBOL(assoc_laguerref, std::, )
-SYMBOL(assoc_laguerrel, std::, )
-SYMBOL(assoc_legendre, std::, )
-SYMBOL(assoc_legendref, std::, )
-SYMBOL(assoc_legendrel, std::, )
 SYMBOL(assume_aligned, std::, )
 SYMBOL(async, std::, )
-SYMBOL(at_quick_exit, std::, )
-SYMBOL(atan, std::, )
-SYMBOL(atan2, std::, )
-SYMBOL(atan2f, std::, )
-SYMBOL(atan2l, std::, )
-SYMBOL(atanf, std::, )
-SYMBOL(atanh, std::, )
-SYMBOL(atanhf, std::, )
-SYMBOL(atanhl, std::, )
-SYMBOL(atanl, std::, )
-SYMBOL(atexit, std::, )
-SYMBOL(atof, std::, )
-SYMBOL(atoi, std::, )
-SYMBOL(atol, std::, )
-SYMBOL(atoll, std::, )
 SYMBOL(atomic_compare_exchange_strong, std::, )
 SYMBOL(atomic_compare_exchange_strong_explicit, std::, )
 SYMBOL(atomic_compare_exchange_weak, std::, )
@@ -176,9 +139,6 @@
 SYMBOL(basic_stringstream, std::, )
 SYMBOL(basic_syncbuf, std::, )
 SYMBOL(bernoulli_distribution, std::, )
-SYMBOL(beta, std::, )
-SYMBOL(betaf, std::, )
-SYMBOL(betal, std::, )
 SYMBOL(bidirectional_iterator, std::, )
 SYMBOL(bidirectional_iterator_tag, std::, )
 SYMBOL(binary_search, std::, )
@@ -200,22 +160,9 @@
 SYMBOL(boolalpha, std::, )
 SYMBOL(boyer_moore_horspool_searcher, std::, )
 SYMBOL(boyer_moore_searcher, std::, )
-SYMBOL(bsearch, std::, )
-SYMBOL(btowc, std::, )
-SYMBOL(byte, std::, )
 SYMBOL(byteswap, std::, )
-SYMBOL(c16rtomb, std::, )
-SYMBOL(c32rtomb, std::, )
-SYMBOL(c8rtomb, std::, )
 SYMBOL(call_once, std::, )
-SYMBOL(calloc, std::, )
 SYMBOL(cauchy_distribution, std::, )
-SYMBOL(cbrt, std::, )
-SYMBOL(cbrtf, std::, )
-SYMBOL(cbrtl, std::, )
-SYMBOL(ceil, std::, )
-SYMBOL(ceilf, std::, )
-SYMBOL(ceill, std::, )
 SYMBOL(centi, std::, )
 SYMBOL(cerr, std::, )
 SYMBOL(char_traits, std::, )
@@ -223,9 +170,6 @@
 SYMBOL(chi_squared_distribution, std::, )
 SYMBOL(cin, std::, )
 SYMBOL(clamp, std::, )
-SYMBOL(clearerr, std::, )
-SYMBOL(clock, std::, )
-SYMBOL(clock_t, std::, )

[clang] 5e01234 - Add a new modules test to ensure we dont rebreak diagnostic

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2023-02-02T07:16:41-08:00
New Revision: 5e01234df81885fa882c58e062ca0cb87ac4849d

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

LOG: Add a new modules test to ensure we dont rebreak diagnostic

Fixes: 60336

Seemingly the concepts sugaring patch caused us to not catch this
situation, which has been confirmed to be a valid error.  Make sure that
we catch this situation in the future, particularly if the concepts
sugaring patch gets re added.

Added: 
clang/test/Modules/GH60336.cpp

Modified: 


Removed: 




diff  --git a/clang/test/Modules/GH60336.cpp b/clang/test/Modules/GH60336.cpp
new file mode 100644
index 0..fefbd37b7926c
--- /dev/null
+++ b/clang/test/Modules/GH60336.cpp
@@ -0,0 +1,78 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -std=c++20 %s -verify -fmodules 
-fmodules-cache-path=%t
+#pragma clang module build std
+module std   [system] {
+  module concepts [system] {
+  module assignable [system] {
+}
+export *
+  }
+  module functional [system] {
+export *
+  }
+
+
+  module type_traits [system] {
+export *
+  }
+}
+
+#pragma clang module contents
+#pragma clang module begin std.type_traits
+namespace std {
+template
+concept same_as = __is_same(_Tp, _Up);
+
+template 
+struct common_reference;
+
+template  struct common_reference<_Tp, _Up>
+{
+using type = _Tp;
+};
+}
+#pragma clang module end // type_traits
+
+#pragma clang module begin std.concepts.assignable
+#pragma clang module import std.type_traits
+namespace std {
+template
+concept common_reference_with =
+  same_as::type, typename 
common_reference<_Up, _Tp>::type>;
+}
+namespace std {
+template
+concept assignable_from =
+  common_reference_with ;
+}
+#pragma clang module end // std.concepts.assignable
+
+#pragma clang module begin std.functional
+#pragma clang module import std.concepts.assignable
+namespace std {
+template
+concept sentinel_for = assignable_from<_Ip&, _Ip>;
+template 
+concept nothrow_sentinel_for = sentinel_for<_Sp, _Ip>;
+}
+#pragma clang module end   // std::functional
+#pragma clang module endbuild // contents
+
+
+#pragma clang module import std.functional
+constexpr bool ntsf_subsumes_sf(std::nothrow_sentinel_for auto) 
requires true {
+  return true;
+}
+constexpr bool ntsf_subsumes_sf(std::sentinel_for auto);
+static_assert(ntsf_subsumes_sf("foo"));
+
+// Note: Doing diagnostics verify lines in the individual modules isn't
+// permitted, and using 'bookmarks' in a module also doesn't work, so we're 
+// forced to diagnose this by line-number.
+//
+// Check to ensure that this error happens, prior to a revert of a concepts
+// sugaring patch, this diagnostic didn't happen correctly.
+
+// expected-error@* {{partial specialization of 'common_reference<_Tp, _Up>' 
must be imported from module 'std.type_traits' before it is required}}
+// expected-note@63 {{while substituting into concept arguments here}}
+// expected-note@*{{partial specialization declared here is not reachable}}



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

[PATCH] D140415: [flang] stack arrays pass

[Tom Eccles via Phabricator via cfe-commits]

tblah added inline comments.



Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:732
+  mlir::applyPartialConversion(func, target, std::move(patterns {
+mlir::emitError(func->getLoc(), "error in stack arrays optimization\n");
+signalPassFailure();

tblah wrote:
> kiranchandramohan wrote:
> > Nit: Is this error usually given in passes?
> Sorry I don't understand. What change are you requesting here?
I've checked some other FIR passes and they all follow the same pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140415

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

[PATCH] D142891: [clang-format] Recognize Verilog non-blocking assignment

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 494303.
sstwcw added a comment.

- add parentheses


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142891

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestVerilog.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1247,6 +1247,21 @@
   EXPECT_TOKEN(Tokens[5], tok::question, TT_ConditionalExpr);
   EXPECT_TOKEN(Tokens[7], tok::colon, TT_ConditionalExpr);
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GotoLabelColon);
+  // Non-blocking assignments.
+  Tokens = Annotate("a <= b;");
+  ASSERT_EQ(Tokens.size(), 5u);
+  EXPECT_TOKEN(Tokens[1], tok::lessequal, TT_BinaryOperator);
+  EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
+  Tokens = Annotate("if (a <= b) break;");
+  ASSERT_EQ(Tokens.size(), 9u);
+  EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
+  EXPECT_TOKEN_PRECEDENCE(Tokens[3], prec::Relational);
+  Tokens = Annotate("a <= b <= a;");
+  ASSERT_EQ(Tokens.size(), 7u);
+  EXPECT_TOKEN(Tokens[1], tok::lessequal, TT_BinaryOperator);
+  EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
+  EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
+  EXPECT_TOKEN_PRECEDENCE(Tokens[3], prec::Relational);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -45,6 +45,58 @@
   }
 };
 
+TEST_F(FormatTestVerilog, Align) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  verifyFormat("x<= x;\n"
+   "sfdbddfbdfbb <= x;\n"
+   "x = x;",
+   Style);
+  verifyFormat("x= x;\n"
+   "sfdbddfbdfbb = x;\n"
+   "x= x;",
+   Style);
+  // Compound assignments are not aligned by default. '<=' is not a compound
+  // assignment.
+  verifyFormat("x<= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x += x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x <<= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x <<<= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x >>= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x >>>= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  Style.AlignConsecutiveAssignments.AlignCompound = true;
+  verifyFormat("x<= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x+= x;\n"
+   "sfdbddfbdfbb <= x;",
+   Style);
+  verifyFormat("x<<= x;\n"
+   "sfdbddfbdfbb  <= x;",
+   Style);
+  verifyFormat("x<<<= x;\n"
+   "sfdbddfbdfbb   <= x;",
+   Style);
+  verifyFormat("x>>= x;\n"
+   "sfdbddfbdfbb  <= x;",
+   Style);
+  verifyFormat("x>>>= x;\n"
+   "sfdbddfbdfbb   <= x;",
+   Style);
+}
+
 TEST_F(FormatTestVerilog, BasedLiteral) {
   verifyFormat("x = '0;");
   verifyFormat("x = '1;");
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -838,7 +838,12 @@
 
 return Style.AlignConsecutiveAssignments.AlignCompound
? C.Tok->getPrecedence() == prec::Assignment
-   : C.Tok->is(tok::equal);
+   : (C.Tok->is(tok::equal) ||
+  // In Verilog the '<=' is not a compound assignment, thus
+  // it is aligned even when the AlignCompound option is not
+  // set.
+  (Style.isVerilog() && C.Tok->is(tok::lessequal) &&
+   C.Tok->getPrecedence() == prec::Assignment));
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments,
   /*RightJustify=*/true);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1625,6 +1625,7 @@
 bool CaretFound = false;
 bool InCpp11AttributeSpecifier = false;
 bool InCSharpAttributeSpecifier = false;
+bool AssignmentFound = false;
 en

[PATCH] D142891: [clang-format] Recognize Verilog non-blocking assignment

[sstwcw via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/WhitespaceManager.cpp:839-846
 return Style.AlignConsecutiveAssignments.AlignCompound
? C.Tok->getPrecedence() == prec::Assignment
-   : C.Tok->is(tok::equal);
+   : C.Tok->is(tok::equal) ||
+ // In Verilog the '<=' is not a compound assignment,
+ // thus it is aligned even when the AlignCompound
+ // option is not set.
+ (Style.isVerilog() && C.Tok->is(tok::lessequal) &&

HazardyKnusperkeks wrote:
> Do you need the extra case, or could you just activate `AlignCompound`?
> 
> If you do, can you please add parens, the precedence of `?` and `:` in 
> combination with `||` is at least for me not 100% clear.
If I get you right, you are suggesting that `AlignCompound: false` is there 
only for backward compatibility, and we can make it default to true since we 
don't have to maintain backward compatibility for new things.  I prefer 
consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142891

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

[PATCH] D142933: Add -print-multi-selection-flags argument

[Michael Platings via Phabricator via cfe-commits]

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

In D142933#4099587 , @Joe wrote:

> Would it be weird for one target to have the `march=` but anothers not?

Yes I think it would be weird. Potentially you could have a toolchain 
supporting Arm and other architectures so it would be unfortunate for that to 
be inconsistent. I'd very much like the API to work as well as possible across 
all architectures now, because it's going to be hard to change later. It's 
great getting your feedback on this.




Comment at: clang/lib/Driver/Driver.cpp:2213
+  if (C.getArgs().hasArg(options::OPT_print_multi_selection_flags)) {
+for (StringRef Attr : TC.getMultiSelectionFlags(C.getArgs()))
+  llvm::outs() << Attr << '\n';

Joe wrote:
> Do we want to parse the multilib.yaml here so we can print out custom flags 
> as well? It could help diagnose issues people have with them.
Yes, I think that would be an improvement. Might need to go in a later patch 
though.



Comment at: clang/lib/Driver/ToolChain.cpp:204
+Result.push_back(
+clang::driver::getDriverOptTable().getOptionName(Option).str());
+  }

Joe wrote:
> > 
> >> the form `x=y` is already broken when you add the flags from the flag list.
> > 
> > Can you give an example of what you mean by that? Sounds like something 
> > that might need fixing.
> For example the option name for OPT_fexceptions is just `fexceptions`, and 
> this is added directly to `Results`
OK, I was thrown by the word "broken".
The syntax for the multilib flags is derived from the command line arguments so 
I think this is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

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

[clang-tools-extra] 9225d08 - [NFC][clang-tidy] Disable test for `bugprone-unsafe-functions` for PlayStation

[via cfe-commits]

Author: Whisperity
Date: 2023-02-02T16:35:08+01:00
New Revision: 9225d08ccca5be900c07eb89e907c4092bbdd462

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

LOG: [NFC][clang-tidy] Disable test for `bugprone-unsafe-functions` for 
PlayStation

As discussed in [D91000](http://reviews.llvm.org/D91000) with @dyung, the
PlayStation-specific targets are using some custom standard library for
which the current written tests are not appropriate. Even though the
test code defines the `__STDC_LIB_EXT1__` and `__STDC_WANT_LIB_EXT1__`
macros and expected *Annex K.* support, the actual Clang
parser/preprocessor will report these macros as not existing, and thus
fail the tests.

The check reports the **non**-Annex K. functions as suggestions, such as
`fgets()` instead of `gets_s()` to replace `gets()`, so some safe
library suggestions are still there.

This patch is primarily done to unblock the relevant buildbot
[`llvm-clang-x86_64-sie-ubuntu-fast`](http://lab.llvm.org/buildbot/#/builders/139).

This commit partially reverts ed740e741ec22f9aaea09bfc0b87d0801a7c492f,
as the changes to the "caching logic" was not fixing anything.

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index c069cc3ea33ad..ebb49645b5904 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -226,6 +226,10 @@ void UnsafeFunctionsCheck::registerPPCallbacks(
 const SourceManager &SM, Preprocessor *PP,
 Preprocessor * /*ModuleExpanderPP*/) {
   this->PP = PP;
+}
+
+void UnsafeFunctionsCheck::onEndOfTranslationUnit() {
+  this->PP = nullptr;
   IsAnnexKAvailable.reset();
 }
 

diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
index f04a220fd12e2..369ea25f693cc 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
@@ -32,6 +32,7 @@ class UnsafeFunctionsCheck : public ClangTidyCheck {
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
+  void onEndOfTranslationUnit() override;
 
 private:
   /// If true, additional functions from widely used API-s (such as POSIX) are

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
index 62754fa68111b..9c8eb7d5cbf11 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -1,3 +1,10 @@
+// This test fails on "x86_64-sie" buildbot and "x86_64-scei-ps4" target.
+// According to @dyung, something related to the kind of standard library
+// availability is causing the failure. Even though we explicitly define
+// the relevant macros the check is hunting for in the invocation, the real
+// parsing and preprocessor state will not have that case.
+// UNSUPPORTED: target={{.*-(ps4|ps5)}}
+//
 // RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s 
bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 
-D__STDC_WANT_LIB_EXT1__=1
 // RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s 
bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   
-U__STDC_WANT_LIB_EXT1__
 // RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s 
bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 
-U__STDC_WANT_LIB_EXT1__
@@ -16,8 +23,11 @@ size_t wcslen(const wchar_t *S);
 void f1(char *S) {
   gets(S);
   // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instead [bugprone-unsafe-functions]
-  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' 
should be used instead
-  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 
'gets' is insecure, was deprecated and removed in C11 and C++14; 'fgets' should 
be used instead
+  // FIXME(?): On target=x86_64-scie-ps4, the above warning in the
+  // "-WITH-ANNEX-K" case will still report the suggestion to use 'fgets'
+  // instead of the expected 'get_s', as if "Annex K" was not available.
+  // CHE

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D91000#4099773 , @dyung wrote:

> To XFAIL the test, grep for lines with "XFAIL" and "ps4" and you should find 
> some examples. It was recently changed how it worked lately.

Thank you, yes I found an example. I went with `UNSUPPORTED` instead of `XFAIL` 
because the test implements 4 or 5 separate cases and `XFAIL`ing it all would 
make some cases become //"Unexpectedly passed"//... I did a quick commit with 
marking it as unsupported with a bit of an explanation in the code, and now the 
buildbots are coming back green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D142384: [C++20] Fix a crash with modules.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 494314.
usaxena95 marked an inline comment as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

Files:
  clang/lib/AST/Decl.cpp


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4769,7 +4769,10 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  // This is necessary for correctness for C++ with modules.
+  // FIXME: Come up with a test case that breaks without definition.
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4769,7 +4769,10 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  // This is necessary for correctness for C++ with modules.
+  // FIXME: Come up with a test case that breaks without definition.
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b612143 - [C++20] Fix a crash with modules.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2023-02-02T17:06:43+01:00
New Revision: b6121432da79c4b3d21f191864ff6c583e2e62eb

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

LOG: [C++20] Fix a crash with modules.

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

Added: 


Modified: 
clang/lib/AST/Decl.cpp

Removed: 




diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e60cc28f6e0f6..51cff6c54a6a5 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4770,7 +4770,10 @@ bool RecordDecl::isOrContainsUnion() const {
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  // This is necessary for correctness for C++ with modules.
+  // FIXME: Come up with a test case that breaks without definition.
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 



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

[PATCH] D140614: [C++20] Mark lambdas in lambda specifiers as dependent if necessary

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/SemaCXX/lambda-unevaluated.cpp:127
+auto foo(int t) {
+  int(*f)(int) = [](auto t) -> decltype([=] { return t; } ()) { return t; };
+  return f;

cor3ntin wrote:
> I thought unevaluated lambdas could not have captures. that might be the 
> issue here
[P0315](https://wg21.link/p0315) and says it should be allowed. 


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

https://reviews.llvm.org/D140614

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

[PATCH] D142384: [C++20] Fix a crash with modules.

[Utkarsh Saxena 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 rGb6121432da79: [C++20] Fix a crash with modules. (authored by 
usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

Files:
  clang/lib/AST/Decl.cpp


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4770,7 +4770,10 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  // This is necessary for correctness for C++ with modules.
+  // FIXME: Come up with a test case that breaks without definition.
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4770,7 +4770,10 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  // This is necessary for correctness for C++ with modules.
+  // FIXME: Come up with a test case that breaks without definition.
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141950: Use find_last_of when seraching for code in getRawCommentForDeclNoCacheImpl

[Kugan Vivekanandarajah via Phabricator via cfe-commits]

kuganv created this revision.
Herald added a subscriber: kadircet.
Herald added a project: All.
kuganv updated this revision to Diff 489882.
kuganv edited the summary of this revision.
kuganv added a comment.
kuganv added subscribers: DmitryPolukhin, 0x1eaf, ivanmurashko.
kuganv updated this revision to Diff 494242.
kuganv added reviewers: craig.topper, jkorous, rnk, gribozavr.
kuganv published this revision for review.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

Updating the summary.


kuganv added a comment.

Rebasing the diff to latest main


Especially in generated code with sparse comments, it takes longer
to bailout when there is code in-between. Searching from last
(with find_last_of) bails out faster.

This shows up in perf profiles with clangd in some auto-generated code.
ASTContext::getRawCommentForDeclNoCacheImpl showing as much as 18.2% in this
case. With find_last_of, this drops to 2.8%.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141950

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -348,7 +348,7 @@
 
   // There should be no other declarations or preprocessor directives between
   // comment and declaration.
-  if (Text.find_first_of(";{}#@") != StringRef::npos)
+  if (Text.find_last_of(";{}#@") != StringRef::npos)
 return nullptr;
 
   return CommentBeforeDecl;


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -348,7 +348,7 @@
 
   // There should be no other declarations or preprocessor directives between
   // comment and declaration.
-  if (Text.find_first_of(";{}#@") != StringRef::npos)
+  if (Text.find_last_of(";{}#@") != StringRef::npos)
 return nullptr;
 
   return CommentBeforeDecl;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added inline comments.



Comment at: clang/tools/libclang/libclang.map:419
 clang_getSymbolGraphForUSR;
+clang_CXXMethod_isExplicit;
 };

This line should be moved from under the `LLVM_16` tag to under a new `LLVM_17` 
tag. Alternatively this commit can be cherry-picked into the LLVM 16 branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140756

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

[PATCH] D139741: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

[Vladislav Dzhidzhoev via Phabricator via cfe-commits]

dzhidzhoev added inline comments.



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:370
+  const auto StorageAlignment = getAlignment(StorageType);
+  if (LayoutSize % StorageAlignment || Layout.getDataSize() % StorageAlignment)
 Packed = true;

efriedma wrote:
> Should this be `if (Layout.getSize() % StorageAlignment || 
> Layout.getDataSize() % StorageAlignment)`?  The dependency on 
> isNoUniqueAddress is a bit confusing.
For base class subobjects, base class DataSize is used to compute the whole 
object size in RecordLayoutBuilder.cpp, and base subobject LLVM type (with 
".base" suffix) is computed in addition to standard layout LLVM type in 
CGRecordLayoutBuilder.cpp.

Currently, both base subobject and standard layout LLVM types of the same class 
are supposed to agree on packedness, as stated in bb51300970b7. Thus, if one is 
packed, both are marked as packed, as done in 
CGRecordLowering::determinePacked. 

For data members of class/struct type, declared with [[no_unique_address]] 
attribute, DataSize is used to compute the whole object size in 
RecordLayoutBuilder.cpp, but standard layout LLVM type is used to represent the 
field in the whole class's LLVM type.
This patch proposes to use the base subobject LLVM type instead of the default 
LLVM type of a class to lay out a no_unique_address member of this class type.

Nextly, the patch proposes to create an unpadded LLVM type for unions, in 
addition to the default LLVM type. It is used to lay out the union members 
having no_unique_address attribute. Existing code for creating base subobject 
layout of classes are reused for unions, therefore, packedness of 
potentially-overlapping union LLVM type and of default union LLVM types must 
agree as well as for classes. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139741

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

[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Warnings and notes of checker alpha.unix.StdLibraryFunctionArgs are
improved. Previously one warning and one note was emitted for every
finding, now one warning is emitted only that contains a detailed
description of the found issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143194

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream-stdlibraryfunctionargs.c

Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===
--- clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -18,37 +18,31 @@
 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}}
+  fclose(fp); // stdargs-warning{{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}}
+  fclose(fp); // stdargs-warning{{should not be NULL}}
 }
 
 void test_fclose(void) {
   FILE *fp = tmpfile();
-  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
-  // stdargs-note{{The 1st argument should not be NULL}}
+  fclose(fp); // stdargs-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
 }
 
 void test_freopen(void) {
   FILE *fp = tmpfile();
-  fp = freopen("file", "w", fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
- // stdargs-note{{The 3rd argument should not be NULL}}
-  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
-  // stdargs-note{{The 1st argument should not be NULL}}
+  fp = freopen("file", "w", fp); // stdargs-warning{{should not be NULL}}
+  fclose(fp); // stdargs-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
 }
 
 void test_fread(void) {
   FILE *fp = tmpfile();
-  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
-// stdargs-note{{The 4th argument should not be NULL}}
+  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{The 4th argument of function 'fread' should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -58,8 +52,7 @@
 
 void test_fwrite(void) {
   FILE *fp = tmpfile();
-  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
-// stdargs-note{{The 4th argument should not be NULL}}
+  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument of function 'fwrite' should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
   clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
@@ -69,24 +62,21 @@
 
 void test_fseek(void) {
   FILE *fp = tmpfile();
-  fseek(fp, 0, 0); // stdargs-warning{{Function argument constraint is not satisfied}} \
-   // stdargs-note{{The 1st argument should not be NULL}}
+  fseek(fp, 0, 0); // stdargs-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
 
 void test_ftell(void) {
   FILE *fp = tmpfile();
-  ftell(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
- // stdargs-note{{The 1st argument should not be NULL}}
+  ftell(fp); // stdargs-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fcl

[PATCH] D139741: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

[Vladislav Dzhidzhoev via Phabricator via cfe-commits]

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

Existing code creating base subobject layout of classes is reused for unions *


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139741

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

[clang] fe08212 - [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2023-02-02T11:48:33-05:00
New Revision: fe082124faa8455cc9a68be5fdf10fc46a4d066c

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

LOG: [clang][driver] Emit an error for `/clang:-x`

`/clang:-x` emits an error instead of a warning. And if the error is suppressed,
`/clang:-x` takes no effect.
Considering that `/clang:` is a recent addition in 2018-11 and there are MSVC
style alternatives, therefore `/clang:-x` doesn't seem useful and we just reject
it since properly supporting it would add lots of complexity.

Fixes #59307

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/Driver.cpp
clang/test/Driver/x-args.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9cc2a72f4c86..4a86b379dda9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -60,6 +60,10 @@ Bug Fixes
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates. This fixes
   `Issue 60344 `_.
+- Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
+  driver mode and emit an error which suggests using ``/TC`` or ``/TP``
+  ``clang-cl`` options instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 98c0edc02ba6..6c0ff3d30e64 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@ void Driver::BuildInputs(const ToolChain &TC, 
DerivedArgList &Args,
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {

diff  --git a/clang/test/Driver/x-args.c b/clang/test/Driver/x-args.c
index b49f474babf0..daaa47bad892 100644
--- a/clang/test/Driver/x-args.c
+++ b/clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?



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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe082124faa8: [clang][driver] Emit an error for `/clang:-x` 
(authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
--implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,10 @@
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates. This fixes
   `Issue 60344 `_.
+- Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
+  driver mode and emit an error which suggests using ``/TC`` or ``/TP``
+  ``clang-cl`` options instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck --implicit-check-not="error:" -check-prefix=CL %s
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since