[clang] ebe0674 - [clang] Try to fix builders

2022-08-11 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2022-08-11T09:07:44+02:00
New Revision: ebe0674acb8bb3404d0e2a6b689d5e3cd02bb0b6

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

LOG: [clang] Try to fix builders

Try to fix these errors:

SemaDeclCXX.cpp:1:19: error: declaration of ‘const clang::Expr* 
clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)Expr’ changes meaning of ‘Expr’ [-fpermissive]
1 |   const Expr *Expr;
  |   ^~~~

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 41b7ff55d0de..13eb9acb0e5e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16663,7 +16663,7 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
   return;
 
 struct {
-  const Expr *Expr;
+  const clang::Expr *Expr;
   Expr::EvalResult Result;
   SmallString<12> ValueString;
   bool Print;



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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

This broke building with GCC (also noted by buildbot mails):

  ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
  ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of 
‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
clang::Expr*)Expr’ changes meaning of ‘Expr’ [-fpermissive]
  1 |   const Expr *Expr;
|   ^~~~
  In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
   from ../tools/clang/include/clang/AST/ASTLambda.h:18,
   from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
  ../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as 
‘class clang::Expr’
109 | class Expr : public ValueStmt {
|   ^~~~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D130894#3715124 , @mstorsjo wrote:

> This broke building with GCC (also noted by buildbot mails):
>
>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
> clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of 
> ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
> clang::Expr*)Expr’ changes meaning of ‘Expr’ 
> [-fpermissive]
>   1 |   const Expr *Expr;
> |   ^~~~
>   In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
>from ../tools/clang/include/clang/AST/ASTLambda.h:18,
>from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
>   ../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here 
> as ‘class clang::Expr’
> 109 | class Expr : public ValueStmt {
> |   ^~~~

It is kinda bad that GCC throws an error and Clang does not even print a 
warning.

How is it even possible? Clang does not implement strict(er) rules ? 
@aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D130969: [clang] [HLSL] Fix GCC warnings about virtual methods that are hidden

2022-08-11 Thread Martin Storsjö 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 rG5d89f9044392: [clang] [HLSL] Fix GCC warnings about virtual 
methods that are hidden (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130969

Files:
  clang/include/clang/Sema/HLSLExternalSemaSource.h


Index: clang/include/clang/Sema/HLSLExternalSemaSource.h
===
--- clang/include/clang/Sema/HLSLExternalSemaSource.h
+++ clang/include/clang/Sema/HLSLExternalSemaSource.h
@@ -45,6 +45,7 @@
   /// Inform the semantic consumer that Sema is no longer available.
   void ForgetSema() override { SemaPtr = nullptr; }
 
+  using ExternalASTSource::CompleteType;
   /// Complete an incomplete HLSL builtin type
   void CompleteType(TagDecl *Tag) override;
 };


Index: clang/include/clang/Sema/HLSLExternalSemaSource.h
===
--- clang/include/clang/Sema/HLSLExternalSemaSource.h
+++ clang/include/clang/Sema/HLSLExternalSemaSource.h
@@ -45,6 +45,7 @@
   /// Inform the semantic consumer that Sema is no longer available.
   void ForgetSema() override { SemaPtr = nullptr; }
 
+  using ExternalASTSource::CompleteType;
   /// Complete an incomplete HLSL builtin type
   void CompleteType(TagDecl *Tag) override;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 5d89f90 - [clang] [HLSL] Fix GCC warnings about virtual methods that are hidden

2022-08-11 Thread Martin Storsjö via cfe-commits

Author: Martin Storsjö
Date: 2022-08-11T10:15:13+03:00
New Revision: 5d89f9044392db3838348a3f44e2723e3436dd28

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

LOG: [clang] [HLSL] Fix GCC warnings about virtual methods that are hidden

This fixes the following warnings produced by GCC 9:

In file included from ../tools/clang/include/clang/Sema/ExternalSemaSource.h:15,
 from 
../tools/clang/include/clang/Sema/HLSLExternalSemaSource.h:17,
 from ../tools/clang/lib/Sema/HLSLExternalSemaSource.cpp:12:
../tools/clang/include/clang/AST/ExternalASTSource.h:211:16: warning: ‘virtual 
void clang::ExternalASTSource::CompleteType(clang::ObjCInterfaceDecl*)’ was 
hidden [-Woverloaded-virtual]
  211 |   virtual void CompleteType(ObjCInterfaceDecl *Class);
  |^~~~
In file included from ../tools/clang/lib/Sema/HLSLExternalSemaSource.cpp:12:
../tools/clang/include/clang/Sema/HLSLExternalSemaSource.h:49:8: warning:   by  
virtual void clang::HLSLExternalSemaSource::CompleteType(clang::TagDecl*)’ 
[-Woverloaded-virtual]
   49 |   void CompleteType(TagDecl *Tag) override;
  |^~~~

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

Added: 


Modified: 
clang/include/clang/Sema/HLSLExternalSemaSource.h

Removed: 




diff  --git a/clang/include/clang/Sema/HLSLExternalSemaSource.h 
b/clang/include/clang/Sema/HLSLExternalSemaSource.h
index a5aa21da1293..0560692f3d5b 100644
--- a/clang/include/clang/Sema/HLSLExternalSemaSource.h
+++ b/clang/include/clang/Sema/HLSLExternalSemaSource.h
@@ -45,6 +45,7 @@ class HLSLExternalSemaSource : public ExternalSemaSource {
   /// Inform the semantic consumer that Sema is no longer available.
   void ForgetSema() override { SemaPtr = nullptr; }
 
+  using ExternalASTSource::CompleteType;
   /// Complete an incomplete HLSL builtin type
   void CompleteType(TagDecl *Tag) override;
 };



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


[PATCH] D131388: [docs] Add "C++20 Modules"

2022-08-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 451750.
ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added a comment.

Address comments:

- Change `module file` to `BMI`.
- Replace `rm -f` with `rm`.
- Add a conclusion paragraph to the `How module speed up compilation` section.


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

https://reviews.llvm.org/D131388

Files:
  clang/docs/CPlusPlus20Modules.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -40,6 +40,7 @@
SafeStack
ShadowCallStack
SourceBasedCodeCoverage
+   CPlusPlus20Modules
Modules
MSVCCompatibility
MisExpect
Index: clang/docs/CPlusPlus20Modules.rst
===
--- /dev/null
+++ clang/docs/CPlusPlus20Modules.rst
@@ -0,0 +1,748 @@
+=
+C++20 Modules
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The term ``modules`` has a lot of meanings. For the users of Clang, modules may
+refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header Modules``,
+etc.) or C++20 modules. The implementation of all these kinds of modules in Clang 
+has a lot of shared code, but from the perspective of users, their semantics and
+command line interfaces are very different. This document focuses on
+an introduction of how to use C++20 modules in Clang.
+
+There is already a detailed document about `Clang modules `_, it
+should be helpful to read `Clang modules `_ if you want to know
+more about the general idea of modules. Since C++20 modules have different semantics
+(and work flows) from `Clang modules`, this page describes the background and use of
+Clang with C++20 modules
+
+Although the term ``modules`` has a unique meaning in C++20 Language Specification,
+when people talk about C++20 modules, they may refer to another C++20 feature:
+header units, which are also covered in this document.
+
+C++20 Modules
+=
+
+This document was intended to be a manual first and foremost, however, we consider it helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial; it will only introduce necessary concepts about the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term ``Modules``/``modules`` refers to C++20 modules
+feature if it is not decorated by ``Clang``.
+
+Clang Modules
+~
+
+In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
+c++ modules extension. These are also known as ``Clang header modules``,
+``Clang module map modules`` or ``Clang c++ modules``.
+
+Module and module unit
+~~
+
+A module consists of one or more module units. A module unit is a special
+translation unit. Every module unit must have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and ``partition_name``
+in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a literal dot ``.``
+in the name has no semantic meaning (e.g. implying a hierarchy).
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module interface partition unit.
+
+* Internal module partition unit.
+
+A primary module interface unit is a module unit whose module declaration is
+``export module module_name;``. The ``module_name`` here denotes the name of the
+module. A module should have one and only one primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+``module module_name;``. A module could have multiple module implementation
+units with the same declaration.
+
+A module interface partition unit is a module unit whose module declaration is
+``export module module_name:partition_name;``. The ``partition_name`` should be
+unique to the module.
+
+A internal module partition unit is a module unit whose module declaration
+is ``module module_name:partition_name;``. The ``partition_name`` should be
+unique to the module.
+
+In this document, we use the following umbrella terms:
+
+* A ``module interface unit`` refers to either a ``primary module interface unit``
+  or a ``module interface partition unit``.
+
+* An ``importable module unit`` refers to either a ``module interface unit``
+  or a ``internal module partition unit``.
+
+* A ``module partition unit`` refers to either a ``module interface partition unit``
+  or a ``internal module partition unit``.
+
+Module file
+~~~
+
+A module file stands for the precompiled result of an importable module unit.
+It is a

[PATCH] D131388: [docs] Add "C++20 Modules"

2022-08-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:665
+  # This is not allowed!
+  $ clang++ iostream.pcm -c -o iostream.o 
+

dblaikie wrote:
> could this use a strikethrough, perhaps? (not sure if you can strikethrough 
> inside a code block)
It looks like we can't do it in code blocks.. maybe we could only depend the 
comments above.



Comment at: clang/docs/CPlusPlus20Modules.rst:347
+  $ clang++ -std=c++20 M.cppm --precompile -o M.pcm
+  $ rm -f M.cppm
+  $ clang++ -std=c++20 Use.cpp -fmodule-file=M.pcm

dblaikie wrote:
> ChuanqiXu wrote:
> > dblaikie wrote:
> > > Could probably skip the `-f`?
> > In my system, when I run `rm M.cppm`, it asks `rm: remove regular file 
> > ‘M.cppm’?` and I need to enter `y` then it would remove the file actually. 
> > So it looks better to add `-f` to me.
> Perhaps your `rm` is aliased to `rm -i`? ( 
> https://superuser.com/questions/345923/remove-file-without-asking ) - `rm` 
> generally/without flags wouldn't prompt for removal of a writable file & 
> encouraging people to get in the habit of using `-f` (especially when they 
> probably don't need to - even if you do on your system) seems a bit 
> problematic. I think not including the input/output of `rm` in case some 
> users have it setup to require some interaction is OK - a plain `rm` is 
> probably enough for users to understand what to do on their system.
Oh, got it.



Comment at: clang/docs/CPlusPlus20Modules.rst:395-396
+
+Roughly, this theory is correct. But the problem is that it is too rough. 
Let's see what actually happens.
+For example, the behavior also depends on the optimization level, as we will 
illustrate below.
+

dblaikie wrote:
> ChuanqiXu wrote:
> > dblaikie wrote:
> > > I'm not sure I'm able to follow the example and how it justifies the 
> > > rough theory as inadequate to explain the motivation for modules - could 
> > > you clarify more directly (in comments, and then we can discuss how to 
> > > word it) what the motivation for this section is/what you're trying to 
> > > convey?
> > Let me answer the motivation first. The motivation comes from my personal 
> > experience. I feel like when most people heard modules, they would ask "how 
> > much speedup could we get"? And there are some other questions like "why 
> > does modules speedup the compilation?". So I guess the readers of the 
> > document may have similar questions and I try to answer it here.
> > 
> > The complexity theory is correct but it may be too abstract to our users. 
> > Since the complexity theory is about the scaling. But for certain users, 
> > the scales of their codes are temporarily fixed. So when they try to use 
> > modules but find the speedup doesn't meet their expectation in O2. They may 
> > feel frustrated. And it doesn't work if I say, "hey, you'll get much better 
> > speedup if the your codes get 10x longer." I guess they won't buy in. So 
> > what I try to do here is to manage the user's expectation to avoid any 
> > misunderstanding.
> > 
> > Following off is about the explanation. For example, there are `1` module 
> > interface and `10` users. There is a function `F` in the module interface 
> > and the function is used by every users. And let's say we need a `T` time 
> > to compile the function `F` and each users without the function `F`.
> > In O0, the function `F` will get compiled completely once and get involved 
> > in the Sema part 10 times. Due to the Sema part is relatively fast and 
> > let's say the Sema part would take `0.1T`. Given we compile them serially, 
> > we need `12T` to compile the project.
> > 
> > But if we are with optimizations, each function `F` will get involved in 
> > optimizations and IPO in every users. And these optimizations are most 
> > time-consuming. Let's say these optimizations will consume `0.8T`. And the 
> > time required will be `19T`. It is easy to say the we need `20T` to compile 
> > the project if we're using headers. So we could find the speedup with 
> > optimization is much slower.
> > 
> > BTW, if we write the required time with variables, it will be `nT + mT + 
> > T*m*additional_compilation_part`. The `additional_compilation_part ` here 
> > corresponds to the time percentage of `Sema` or `Optimizations`. And since 
> > `T` and `additional_compilation_part ` are both constant. So if we write 
> > them in `O()` form, it would be `O(n+m)`.
> > So the theory is still correct.
> > 
> > 
> I think the message is getting a bit lost in the text (both in the proposed 
> text, and the comment here).
> 
> "At -O0 implementations of non-inline functions defined in a module will not 
> impact module users, but at higher optimization levels the definitions of 
> such functions are provided to user compilations for the purposes of 
> optimization (but definitions of these functions are still not included in 
> the use's object fil

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-08-11 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment.

Having looked around in the Firefox codebase for cases that now fail to build 
because of this, and looking at the clarification in DR2338, I wonder if enums 
in `extern "C"` sections should be treated as if they had an explicit type of 
`int` as if it were e.g. `enum Foo: int { ... }`.


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

https://reviews.llvm.org/D130058

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


[PATCH] D131655: [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

2022-08-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: t-rasmud, usama54321, ziqingluo-90, malavikasamak, 
xazax.hun.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
Herald added a project: All.
NoQ requested review of this revision.

Patch by Paul Peltz! I'm going to be addressing comments. The overall idea 
sounds great and the code seems to have all the necessary parts in place but I 
haven't looked at it //very// carefully so please comment.

//Author's original description://

The NullabilityChecker has a very early policy decision that non-inlined 
property accesses will be inferred as returning nonnull, despite nullability 
annotations to the contrary.  This decision eliminates false positives related 
to very common code patterns that look like this:

  if (foo.prop) {
  [bar doStuffWithNonnull:foo.prop];
  }

While this probably represents a correct nil-check, the analyzer can't 
determine correctness without gaining visibility into the property 
implementation.

Unfortunately, inferring nullable properties as nonnull comes at the cost of 
significantly reduced code coverage. My goal here is to enable detection of 
many property-related nullability violations without a large increase in false 
positives.

The approach is to introduce a heuristic: after accessing the value of a 
property, if the analyzer at any time proves that the property value is nonnull 
(which would happen in particular due to a nil-check conditional), then 
subsequent property accesses on that code path will be *inferred* as nonnull. 
This captures the pattern described above, which I believe to be the dominant 
source of false positives in real code.


Repository:
  rC Clang

https://reviews.llvm.org/D131655

Files:
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/nullability.mm

Index: clang/test/Analysis/nullability.mm
===
--- clang/test/Analysis/nullability.mm
+++ clang/test/Analysis/nullability.mm
@@ -34,6 +34,13 @@
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+  unsigned int __line, __const char *__function)
+__attribute__((__noreturn__));
+
+#define assert(expr) \
+  ((expr) ? (void)(0) : __assert_fail(#expr, __FILE__, __LINE__, __func__))
+
 @interface TestObject : NSObject
 - (int *_Nonnull)returnsNonnull;
 - (int *_Nullable)returnsNullable;
@@ -42,6 +49,9 @@
 - (void)takesNullable:(int *_Nullable)p;
 - (void)takesUnspecified:(int *)p;
 @property(readonly, strong) NSString *stuff;
+@property(readonly, nonnull) int *propReturnsNonnull;
+@property(readonly, nullable) int *propReturnsNullable;
+@property(readonly) int *propReturnsUnspecified;
 @end
 
 TestObject * getUnspecifiedTestObject();
@@ -182,6 +192,53 @@
   }
 }
 
+void testObjCPropertyReadNullability() {
+  TestObject *o = getNonnullTestObject();
+  switch (getRandom()) {
+  case 0:
+[o takesNonnull:o.propReturnsNonnull]; // no-warning
+break;
+  case 1:
+[o takesNonnull:o.propReturnsUnspecified]; // no-warning
+break;
+  case 2:
+[o takesNonnull:o.propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+break;
+  case 3:
+// If a property is constrained nonnull, assume it remains nonnull
+if (o.propReturnsNullable) {
+  [o takesNonnull:o.propReturnsNullable]; // no-warning
+  [o takesNonnull:o.propReturnsNullable]; // no-warning
+}
+break;
+  case 4:
+if (!o.propReturnsNullable) {
+  [o takesNonnull:o.propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+}
+break;
+  case 5:
+if (!o.propReturnsNullable) {
+  if (o.propReturnsNullable) {
+// Nonnull constraint from the more recent call wins
+[o takesNonnull:o.propReturnsNullable]; // no-warning
+  }
+}
+break;
+  case 6:
+// Constraints on property return values are receiver-qualified
+if (o.propReturnsNullable) {
+  [o takesNonnull:o.propReturnsNullable];  // no-warning
+  [o takesNonnull:getNonnullTestObject().propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+}
+break;
+  case 7:
+// Assertions must be effective at suppressing warnings
+assert(o.propReturnsNullable);
+[o takesNonnull:o.propReturnsNullable]; // no-warning
+break;
+  }
+}
+
 Dummy * _Nonnull testDirectCastNullableToNonnull() {
   Dummy *p = returnsNullable();
   takesNonnull((Dummy * _Nonnull)p);  // no-warning
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

2022-08-11 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 451754.
wyt added a comment.

Initialise DeclCtx field with nullptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
   ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
@@ -168,9 +168,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -185,7 +187,7 @@
 ReturnLoc = &createStorageLocation(ReturnType);
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -210,6 +212,9 @@
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
+
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -252,12 +257,12 @@
 
 void Environment::popCall(const Environment &CalleeEnv) {
   // We ignore `DACtx` because it's already the same in both. We don't want the
-  // callee's `ReturnLoc` or `ThisPointeeLoc`. We don't bring back `DeclToLoc`
-  // and `ExprToLoc` because we want to be able to later analyze the same callee
-  // in a different context, and `setStorageLocation` requires there to not
-  // already be a storage location assigned. Conceptually, these maps capture
-  // information from the local scope, so when popping that scope, we do not
-  // propagate the maps.
+  // callee's `DeclCtx`, `ReturnLoc` or `ThisPointeeLoc`. We don't bring back
+  // `DeclToLoc` and `ExprToLoc` because we want to be able to later analyze the
+  // same callee in a different context, and `setStorageLocation` requires there
+  // to not already be a storage location assigned. Conceptually, these maps
+  // capture information from the local scope, so when popping that scope, we do
+  // not propagate the maps.
   this->LocToVal = std::move(CalleeEnv.LocToVal);
   this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
   this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
@@ -304,11 +309,13 @@
   assert(DACtx == Other.DACtx);
   assert(ReturnLoc == Other.ReturnLoc);
   assert(ThisPointeeLoc == Other.ThisPointeeLoc);
+  assert(DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
   JoinedEnv.ReturnLoc = ReturnLoc;
   JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
 
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -347,6 +347,13 @@
   /// imply that `Val` is true.
   bool flowConditionImplies(BoolValue &Val) const;
 
+  /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
+  /// returns null.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
@@ -377,6 +384,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx = nullptr;
+
   // In a properly initialized `Environment`, `ReturnLoc` should only be null if
   // its `DeclContext` could not be cast to a `FunctionDecl`.
   StorageLocatio

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

2022-08-11 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

There are still some cases that were broken by D131307 
, that aren't fixed by this patch. Building 
https://martin.st/temp/qt-enum.cpp with `clang -target i686-w64-mingw32 -c 
-std=c++17 qt-enum.cpp -Wno-ignored-attributes -Wno-user-defined-literals` 
succeeded before the change to make those errors downgradable, and those are 
still an error now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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


[PATCH] D123967: Disable update_cc_test_checks.py tests in stand-alone builds

2022-08-11 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123967

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


[clang] 2cb5144 - [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

2022-08-11 Thread Wei Yi Tee via cfe-commits

Author: Wei Yi Tee
Date: 2022-08-11T07:36:57Z
New Revision: 2cb51449f0d9ed06de87b4a47b5074eb6eec2e23

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

LOG: [clang][dataflow] Store DeclContext of block being analysed in Environment 
if available.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index fc43b6b43575f..94fe2be37b2eb 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -347,6 +347,13 @@ class Environment {
   /// imply that `Val` is true.
   bool flowConditionImplies(BoolValue &Val) const;
 
+  /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
+  /// returns null.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
@@ -377,6 +384,9 @@ class Environment {
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx = nullptr;
+
   // In a properly initialized `Environment`, `ReturnLoc` should only be null 
if
   // its `DeclContext` could not be cast to a `FunctionDecl`.
   StorageLocation *ReturnLoc = nullptr;

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index ff27a2a45179b..16c83cad9d9e3 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
   ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
@@ -168,9 +168,11 @@ Environment &Environment::operator=(const Environment 
&Other) {
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -185,7 +187,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 ReturnLoc = &createStorageLocation(ReturnType);
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -210,6 +212,9 @@ Environment Environment::pushCall(const CallExpr *Call) 
const {
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
+
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -252,12 +257,12 @@ Environment Environment::pushCall(const CallExpr *Call) 
const {
 
 void Environment::popCall(const Environment &CalleeEnv) {
   // We ignore `DACtx` because it's already the same in both. We don't want the
-  // callee's `ReturnLoc` or `ThisPointeeLoc`. We don't bring back `DeclToLoc`
-  // and `ExprToLoc` because we want to be able to later analyze the same 
callee
-  // in a 
diff erent context, and `setStorageLocation` requires there to not
-  // already be a storage location assigned. Conceptually, these maps capture
-  // information from the local scope, so when popping that scope, we do not
-  // propagate the maps.
+  // callee's `DeclCtx`, `ReturnLoc` or `ThisPointeeLoc`. We don't bring back
+  // `DeclToLoc` and `ExprToLoc` because we want to be able to later analyze 
the
+  // same callee in a 
diff erent context, and `setSto

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

2022-08-11 Thread weiyi 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 rG2cb51449f0d9: [clang][dataflow] Store DeclContext of block 
being analysed in Environment if… (authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
   ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
@@ -168,9 +168,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -185,7 +187,7 @@
 ReturnLoc = &createStorageLocation(ReturnType);
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -210,6 +212,9 @@
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
+
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -252,12 +257,12 @@
 
 void Environment::popCall(const Environment &CalleeEnv) {
   // We ignore `DACtx` because it's already the same in both. We don't want the
-  // callee's `ReturnLoc` or `ThisPointeeLoc`. We don't bring back `DeclToLoc`
-  // and `ExprToLoc` because we want to be able to later analyze the same callee
-  // in a different context, and `setStorageLocation` requires there to not
-  // already be a storage location assigned. Conceptually, these maps capture
-  // information from the local scope, so when popping that scope, we do not
-  // propagate the maps.
+  // callee's `DeclCtx`, `ReturnLoc` or `ThisPointeeLoc`. We don't bring back
+  // `DeclToLoc` and `ExprToLoc` because we want to be able to later analyze the
+  // same callee in a different context, and `setStorageLocation` requires there
+  // to not already be a storage location assigned. Conceptually, these maps
+  // capture information from the local scope, so when popping that scope, we do
+  // not propagate the maps.
   this->LocToVal = std::move(CalleeEnv.LocToVal);
   this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
   this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
@@ -304,11 +309,13 @@
   assert(DACtx == Other.DACtx);
   assert(ReturnLoc == Other.ReturnLoc);
   assert(ThisPointeeLoc == Other.ThisPointeeLoc);
+  assert(DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
   JoinedEnv.ReturnLoc = ReturnLoc;
   JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
 
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -347,6 +347,13 @@
   /// imply that `Val` is true.
   bool flowConditionImplies(BoolValue &Val) const;
 
+  /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
+  /// returns null.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
@@ -377,6 +384,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx = nullptr;
+
   /

[PATCH] D131134: [X86] Report error if the amx enabled on the non-64-bits target

2022-08-11 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment.

In D131134#3715079 , @craig.topper 
wrote:

> In D131134#3715062 , @pengfei wrote:
>
>> In D131134#3715024 , @pengfei 
>> wrote:
>>
>>> In D131134#3714860 , 
>>> @craig.topper wrote:
>>>
 Yes AMX is only supported in 64-bit mode but I doubt CPUID checks which 
 mode the program is running in.
>>>
>>> I guess `HasAMXSave` is `false` on 32-bit mode 
>>> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Host.cpp#L1800.
>>>  But I don't have the environment to check it.
>>
>> I found it. The `XFEATURE_MASK_XTILE` in Kernel is the same as `HasAMXSave` 
>> in compiler: 
>> https://lwn.net/ml/linux-kernel/20210710130313.5072-20-chang.seok@intel.com/
>> So these features bits are `true` only on 64-bit mode.
>
> Is it cleared when running a 32-bit binary on a 64-bit kernel?

No, a 32-bit binary gets the same CR0 as 64-bit on a 64-bit kernel. Actually, 
it's compiler to get the CR0 rather than the binary it builds. It should be 
rare to run 32-bit compiler on 64-bit kernel.
So you are right. No matter 32 or 64 bits compiler, when it builds a binary 
with `-m32` on 64-bit kernel with `-march=native`, it always sets AMX features. 
Maybe we should reset them to false when compile for 32-bit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131134

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


[PATCH] D131448: Introduce iterator sentinel to make graph traversal implementation more efficient and cleaner

2022-08-11 Thread Roman Rusyaev via Phabricator via cfe-commits
rusyaev-roman added a comment.

First of all, thank you for your feedback! I've tried to address all your 
comments.




Comment at: llvm/include/llvm/ADT/BreadthFirstIterator.h:128-131
+  bool operator==(iterator_sentinel) const { return VisitQueue.empty(); }
+
+  bool operator!=(iterator_sentinel RHS) const { return !(*this == RHS); }
+

dblaikie wrote:
> Generally any operator that can be a non-member should be a non-member (but 
> can still be a friend) so there's equal conversion handling for LHS and RHS. 
> Could you make these non-members? (maybe a separate patch to do the same to 
> the existing op overloads, so the new ones don't look weird)
> 
> do you need the inverse operators too, so the sentinel can appear on either 
> side of the comparison? 
Absolutely agree with all your points!

But I didn't want to make the code inconsistent and complicated in this patch. 
So, I suggest making all these operators 'friend' in a separate patch, 
otherwise it can lead to some boilerplate code like this:
```
  friend bool operator==(const scc_iterator &SCCI, iterator_sentinel) {
return SCCI.isAtEnd();
  }

  friend bool operator==(iterator_sentinel IS, const scc_iterator &SCCI) {
return SCCI == IS;
  }

  friend bool operator!=(const scc_iterator &SCCI, iterator_sentinel IS) {
return !(SCCI == IS);
  }

  friend bool operator!=(const scc_iterator &SCCI, iterator_sentinel IS) {
return !(IS == SCCI);
  }
```
This boilerplate code can be avoided using special helper classes, but I 
wouldn't like to implement them in this patch in order to keep it simple.

What do you think?



Comment at: llvm/include/llvm/ADT/SCCIterator.h:49-50
   using SccTy = std::vector;
-  using reference = typename scc_iterator::reference;
+  using reference = const SccTy &;
+  using pointer = const SccTy *;
 

dblaikie wrote:
> does this need a `value` type too? (& then define the `reference` and 
> `pointer` types relative to that)
Thanks, good point! I forgot to add additional types and member functions to 
satisfy `forward iterator` requirements when I removed `iterator_facade` base 
class. I'll update the patch (maybe get iterator_facade back)



Comment at: llvm/include/llvm/ADT/SCCIterator.h:152-162
+bool hasCycle() const {
+  assert(!SCC.empty() && "Dereferencing END SCC iterator!");
+  if (SCC.size() > 1)
+return true;
+  NodeRef N = SCC.front();
+  for (ChildItTy CI = GT::child_begin(N), CE = GT::child_end(N); CI != CE;
+   ++CI)

dblaikie wrote:
> I'm not quite following why this requires the proxy object - even after 
> reading the comment above. It looks like this function is entirely in terms 
> of the `SCC` object that's returned from `operator*` - so maybe this could be 
> a free function, called with `hasCycle(*some_iterator)`?
> maybe this could be a free function, called with hasCycle(*some_iterator)?

This was my initial intention.

But in the case of free function (or maybe static function of scc_iterator 
class) a user should write the following code:
```
 for (const auto& SCC : scc_traversal(Graph))
   if (hasCycle(SCC)) // or in more complicated case when 
GraphTraits cannot be deduced from Graph type -- hasCycle(SCC))
  ...
```

This is the main reason of SCCProxy introduction -- to make it possible to 
write like this:
```
 for (const auto& SCC : scc_traversal(Graph))
   if (SCC.hasCycle())
  ...
```



Comment at: llvm/include/llvm/ADT/SCCIterator.h:165-170
+  SCCProxy operator*() const {
 assert(!CurrentSCC.empty() && "Dereferencing END SCC iterator!");
 return CurrentSCC;
   }
 
+  SCCProxy operator->() const { return operator*(); }

dblaikie wrote:
> I always forget in which cases you're allowed to return a proxy object from 
> an iterator - I thought some iterator concepts (maybe random access is the 
> level at which this kicks in?) that required something that amounts to "there 
> has to be a real object that outlives the iterator"
> 
> Could you refresh my memory on that/on why proxy objects are acceptable for 
> this iterator type? (where/how does this iterator declare what concept it 
> models anyway, since this removed the facade helper?)
A proxy object is allowed to be returned while dereferencing an `input 
iterator` (https://en.cppreference.com/w/cpp/named_req/InputIterator#Notes)

```
The reference type for an input iterator that is not also a 
LegacyForwardIterator does not have to be a reference type: dereferencing an 
input iterator may return a proxy object or value_type itself by value
```

For our case (that's `forward iterator`) we need to satisfy the following thing:
```
 The type std::iterator_traits::reference must be exactly 
   ...
   * const T& otherwise (It is constant), 

(where T is the type denoted by std::iterator_traits::value_type) 
```
I'll also

[PATCH] D131657: Remove redundant condition check, NFC

2022-08-11 Thread Jun Zhang via Phabricator via cfe-commits
junaire created this revision.
Herald added a project: All.
junaire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Jun Zhang 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131657

Files:
  clang/lib/Parse/ParseDecl.cpp


Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2078,10 +2078,8 @@
   << (Fixit ? FixItHint::CreateInsertion(D.getBeginLoc(), "_Noreturn ")
 : FixItHint());
 }
-  }
 
-  // Check to see if we have a function *definition* which must have a body.
-  if (D.isFunctionDeclarator()) {
+// Check to see if we have a function *definition* which must have a body.
 if (Tok.is(tok::equal) && NextToken().is(tok::code_completion)) {
   cutOffParsing();
   Actions.CodeCompleteAfterFunctionEquals(D);


Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2078,10 +2078,8 @@
   << (Fixit ? FixItHint::CreateInsertion(D.getBeginLoc(), "_Noreturn ")
 : FixItHint());
 }
-  }
 
-  // Check to see if we have a function *definition* which must have a body.
-  if (D.isFunctionDeclarator()) {
+// Check to see if we have a function *definition* which must have a body.
 if (Tok.is(tok::equal) && NextToken().is(tok::code_completion)) {
   cutOffParsing();
   Actions.CodeCompleteAfterFunctionEquals(D);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131563: [WIP] [clang] Fix clang multiarch isssue with musl

2022-08-11 Thread Brahmajit via Phabricator via cfe-commits
listout updated this revision to Diff 451757.
listout added a comment.

Using `getEnvironmentTypeName()` instead of `isMusl()`


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

https://reviews.llvm.org/D131563

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -46,6 +46,9 @@
   bool IsMipsR6 = TargetTriple.getSubArch() == llvm::Triple::MipsSubArch_r6;
   bool IsMipsN32Abi = TargetTriple.getEnvironment() == llvm::Triple::GNUABIN32;
 
+  llvm::StringRef EnvName =
+  llvm::Triple::getEnvironmentTypeName(TargetTriple.getEnvironment());
+
   // For most architectures, just use whatever we have rather than trying to be
   // clever.
   switch (TargetTriple.getArch()) {
@@ -62,34 +65,35 @@
   return "arm-linux-androideabi";
 if (TargetEnvironment == llvm::Triple::GNUEABIHF)
   return "arm-linux-gnueabihf";
-return "arm-linux-gnueabi";
+return "arm-linux-" + llvm::Twine(EnvName).str() + "eabi";
   case llvm::Triple::armeb:
   case llvm::Triple::thumbeb:
 if (TargetEnvironment == llvm::Triple::GNUEABIHF)
   return "armeb-linux-gnueabihf";
-return "armeb-linux-gnueabi";
+return "armeb-linux-" + llvm::Twine(EnvName).str() + "eabi";
   case llvm::Triple::x86:
 if (IsAndroid)
   return "i686-linux-android";
-return "i386-linux-gnu";
+return "i386-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::x86_64:
 if (IsAndroid)
   return "x86_64-linux-android";
 if (TargetEnvironment == llvm::Triple::GNUX32)
   return "x86_64-linux-gnux32";
-return "x86_64-linux-gnu";
+return "x86_64-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::aarch64:
 if (IsAndroid)
   return "aarch64-linux-android";
-return "aarch64-linux-gnu";
+return "aarch64-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::aarch64_be:
-return "aarch64_be-linux-gnu";
+return "aarch64_be-linux-" + llvm::Twine(EnvName).str();
 
   case llvm::Triple::m68k:
-return "m68k-linux-gnu";
+return "m68k-linux-" + llvm::Twine(EnvName).str();
 
   case llvm::Triple::mips:
-return IsMipsR6 ? "mipsisa32r6-linux-gnu" : "mips-linux-gnu";
+return IsMipsR6 ? "mipsisa32r6-linux-gnu"
+: "mips-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::mipsel:
 if (IsAndroid)
   return "mipsel-linux-android";
@@ -117,19 +121,19 @@
   case llvm::Triple::ppc:
 if (D.getVFS().exists(concat(SysRoot, "/lib/powerpc-linux-gnuspe")))
   return "powerpc-linux-gnuspe";
-return "powerpc-linux-gnu";
+return "powerpc-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::ppcle:
-return "powerpcle-linux-gnu";
+return "powerpcle-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::ppc64:
-return "powerpc64-linux-gnu";
+return "powerpc64-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::ppc64le:
-return "powerpc64le-linux-gnu";
+return "powerpc64le-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::riscv64:
-return "riscv64-linux-gnu";
+return "riscv64-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::sparc:
-return "sparc-linux-gnu";
+return "sparc-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::sparcv9:
-return "sparc64-linux-gnu";
+return "sparc64-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::systemz:
 return "s390x-linux-gnu";
   }


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -46,6 +46,9 @@
   bool IsMipsR6 = TargetTriple.getSubArch() == llvm::Triple::MipsSubArch_r6;
   bool IsMipsN32Abi = TargetTriple.getEnvironment() == llvm::Triple::GNUABIN32;
 
+  llvm::StringRef EnvName =
+  llvm::Triple::getEnvironmentTypeName(TargetTriple.getEnvironment());
+
   // For most architectures, just use whatever we have rather than trying to be
   // clever.
   switch (TargetTriple.getArch()) {
@@ -62,34 +65,35 @@
   return "arm-linux-androideabi";
 if (TargetEnvironment == llvm::Triple::GNUEABIHF)
   return "arm-linux-gnueabihf";
-return "arm-linux-gnueabi";
+return "arm-linux-" + llvm::Twine(EnvName).str() + "eabi";
   case llvm::Triple::armeb:
   case llvm::Triple::thumbeb:
 if (TargetEnvironment == llvm::Triple::GNUEABIHF)
   return "armeb-linux-gnueabihf";
-return "armeb-linux-gnueabi";
+return "armeb-linux-" + llvm::Twine(EnvName).str() + "eabi";
   case llvm::Triple::x86:
 if (IsAndroid)
   return "i686-linux-android";
-return "i386-linux-gnu";
+return "i386-linux-" + llvm::Twine(EnvName).str();
   case llvm::Triple::x86_64:
 if (IsAndroid)
   return "x86_64-linux-androi

[PATCH] D131563: [WIP] [clang] Fix clang multiarch isssue with musl

2022-08-11 Thread Brahmajit via Phabricator via cfe-commits
listout marked an inline comment as done.
listout added a comment.

@barannikov88 made some changes. What do you think?




Comment at: clang/lib/Driver/ToolChains/Linux.cpp:49
 
+  std::string EnvName = TargetTriple.isMusl() ? "musl" : "gnu";
+

barannikov88 wrote:
> * `!isMusl()` does not mean it is "gnu".
> * Why don't just use `Triple::getEnvironmentTypeName`?
> 
Sure, I'll change the patch


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

https://reviews.llvm.org/D131563

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


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

2022-08-11 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 451760.
carlosgalvezp added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,169 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template 
+struct unique_ptr {};
+
+template 
+struct shared_ptr {};
+} // namespace std
+
+namespace gsl {
+template 
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference
+};
+
+struct ConstRefMember {
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference
+};
+
+struct RvalueRefMember {
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference
+};
+
+struct ConstAndRefMembers {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified
+};
+
+struct LvalueRefMember2 {
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'ConstType' (aka 'const int') is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' of type 'RefType' (aka 'int &') is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' of type 'ConstRefType' (aka 'const int &') is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' of type 'RefRefType' (aka 'int &&') is a reference
+};
+
+template 
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' of type 'const Array<1>' (aka 'const int[1]') is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' of type 'Array<2> &' (aka 'int (&)[2]') is a reference
+};
+
+struct ConstLvalueRefArrayMember {
+  const Array<3> &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' of type 'const Array<3> &' (aka 'co

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

2022-08-11 Thread Dave Brown via Phabricator via cfe-commits
bigdavedev accepted this revision.
bigdavedev added a comment.

LGTM. All comments have been addressed. Any improvements to the diagnostics can 
be made down the line depending on how users feel about them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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


[clang-tools-extra] 9ae5896 - [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

2022-08-11 Thread Carlos Galvez via cfe-commits

Author: Carlos Galvez
Date: 2022-08-11T07:46:04Z
New Revision: 9ae5896d9673a54f4a6cf656624891bafc192857

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

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

Flags uses of const-qualified and reference data members in structs.
Implements rule C.12 of C++ Core Guidelines.

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

Added: 

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Modified: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
new file mode 100644
index 0..5f340736c8dde
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
@@ -0,0 +1,38 @@
+//===--- AvoidConstOrRefDataMembersCheck.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 "AvoidConstOrRefDataMembersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidConstOrRefDataMembersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  fieldDecl(hasType(hasCanonicalType(referenceType(.bind("ref"), this);
+  Finder->addMatcher(
+  fieldDecl(hasType(qualType(isConstQualified(.bind("const"), this);
+}
+
+void AvoidConstOrRefDataMembersCheck::check(
+const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs("ref"))
+diag(MatchedDecl->getLocation(), "member %0 of type %1 is a reference")
+<< MatchedDecl << MatchedDecl->getType();
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs("const"))
+diag(MatchedDecl->getLocation(), "member %0 of type %1 is const qualified")
+<< MatchedDecl << MatchedDecl->getType();
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
new file mode 100644
index 0..7fd94aa7daa26
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
@@ -0,0 +1,38 @@
+//===--- AvoidConstOrRefDataMembersCheck.h - clang-tidy -*- C++ 
-*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Const-qualified or reference data members in classes should be avoided, as
+/// they make the class non-copy-assignable.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.html
+class AvoidConstOrRefDataMembersCheck : public ClangTidyCheck {
+public:
+  AvoidConstOrRefDataMembersCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // 

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

2022-08-11 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ae5896d9673: [clang-tidy] Add 
cppcoreguidelines-avoid-const-or-ref-data-members check (authored by 
carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,169 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template 
+struct unique_ptr {};
+
+template 
+struct shared_ptr {};
+} // namespace std
+
+namespace gsl {
+template 
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference
+};
+
+struct ConstRefMember {
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference
+};
+
+struct RvalueRefMember {
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference
+};
+
+struct ConstAndRefMembers {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified
+};
+
+struct LvalueRefMember2 {
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'ConstType' (aka 'const int') is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' of type 'RefType' (aka 'int &') is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' of type 'ConstRefType' (aka 'const int &') is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' of type 'RefRefType' (aka 'int &&') is a reference
+};
+
+template 
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' of type 'const Array<1>' (aka 'const int[1]') is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> &lr;
+  // CHECK-MESSA

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-08-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment.

In D126907#3712363 , @erichkeane 
wrote:

> In D126907#3711956 , @ChuanqiXu 
> wrote:
>
>> In D126907#3710656 , @erichkeane 
>> wrote:
>>
>>> Ok, fixed the test failure in clang, AND it managed to fix 
>>> all the failures in libcxx!
>>>
>>> HOWEVER, it appears that libcxx/test/libcxx/modules_include.sh.cpp
>>> is now hanging?
>>>
>>> I don't know much about the modules implementation (perhaps someone
>>> like @ChuanqiXu  can help out?), so I'm at least somewhat stuck until
>>> I can figure out how to get it to give me more info.
>>
>> I may not be able to look into the details recently. What's the error 
>> message from the modules?
>
> Looking deeper... this test is a monstrosity 
> (https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/modules_include.sh.cpp),
>  and I'm not necessarily sure it is necessarily modules-related, other than 
> enabling it on every header.  So it just seems like one of the STL headers is 
> taking significantly longer to compile?  I have no idea how to move forward 
> with this... it AT LEAST "finishes", but it takes a very long time to get 
> through now.

I tried to reproduce and I am not sure if reproduced. With this patch, 
`modules_include.sh.cpp` takes 562s to complete. And without this patch, it 
takes a 557s. So it looks not your fault.


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

https://reviews.llvm.org/D126907

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


[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Ties Stuij via Phabricator via cfe-commits
stuij updated this revision to Diff 451781.
stuij added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/ConstantEmitter.h
  clang/test/CodeGen/const-init.c


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +181,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+// CHECK: @.compoundliteral = internal global [1 x float] [float 
0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,15 @@
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
+static ConstantAddress tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
   const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1970,7 +1968,9 @@
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2214,7 +2214,8 @@
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this, nullptr);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +181,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4
 }
+
+// CHECK: @.compoundliteral = internal global [1 x float] [float 0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstan

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Ties Stuij via Phabricator via cfe-commits
stuij updated this revision to Diff 451783.
stuij marked an inline comment as done.
stuij added a comment.

also added a comment above the runline. Initially thought it wouldn't be 
necessary as we're now doing a check, but I do agree that it's more clear to be 
specific about the cmdline arg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/ConstantEmitter.h
  clang/test/CodeGen/const-init.c


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+// CHECK: @.compoundliteral = internal global [1 x float] [float 
0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,15 @@
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
+static ConstantAddress tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
   const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1970,7 +1968,9 @@
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2214,7 +2214,8 @@
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this, nullptr);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s | FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, al

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

2022-08-11 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added a reviewer: aaron.ballman.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider:

  constexpr int a;

the error message for this is currently:

  error: default initialization of an object of const type 'const int'
  constexpr int a;
^
  = 0

which makes very little sense to me. With this patch, the output is instead:

  error: constexpr variable 'a' must be initialized by a constant expression
  constexpr int a;
^
  = 0

which is much better. Tells the user exactly what is missing.

For

  constexpr int a[];

before this patch, the output is:

  error: definition of variable with array type needs an explicit size or an 
initializer
  constexpr int a[];
^

The error message is not even true in this case, since _only_ passing a size 
won't work. It must be initialized by a constant expression, so now the error 
message is:

  error: constexpr variable 'a' must be initialized by a constant expression
  constexpr int a[];
^

and a fixit hint if a size was provided:

  error: constexpr variable 'a' must be initialized by a constant expression
  constexpr int a[2];
^
 = {}

(not sure about that part)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131662

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaFixItUtils.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -27,6 +27,10 @@
   constexpr int zero() const { return 0; }
 };
 
+constexpr int arr[]; // expected-error {{constexpr variable 'arr' must be initialized by a constant expression}}
+constexpr int arr2[2]; // expected-error {{constexpr variable 'arr2' must be initialized by a constant expression}}
+constexpr int arr3[2] = {};
+
 namespace DerivedToVBaseCast {
 
   struct U { int n; };
@@ -1298,7 +1302,7 @@
   void f() {
 extern constexpr int i; // expected-error {{constexpr variable declaration must be a definition}}
 constexpr int j = 0;
-constexpr int k; // expected-error {{default initialization of an object of const type}}
+constexpr int k; // expected-error {{constexpr variable 'k' must be initialized by a constant expression}}
   }
 
   extern const int q;
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
@@ -18,7 +18,7 @@
 
 // A variable declaration which uses the constexpr specifier shall have an
 // initializer and shall be initialized by a constant expression.
-constexpr int ni1; // expected-error {{default initialization of an object of const type 'const int'}}
+constexpr int ni1; // expected-error {{constexpr variable 'ni1' must be initialized by a constant expression}}
 constexpr struct C { C(); } ni2; // expected-error {{cannot have non-literal type 'const struct C'}} expected-note 3{{has no constexpr constructors}}
 constexpr double &ni3; // expected-error {{declaration of reference variable 'ni3' requires an initializer}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
@@ -37,7 +37,7 @@
 #if __cplusplus <= 201402L && !defined(MS_ABI)
   // expected-error@-2 {{requires an initializer}}
 #else
-  // expected-error@-4 {{default initialization of an object of const}}
+  // expected-error@-4 {{constexpr variable 'mi2' must be initialized by a constant expression}}
 #endif
   mutable constexpr int mi3 = 3; // expected-error-re {{non-static data member cannot be constexpr{{$ expected-error {{'mutable' and 'const' cannot be mixed}}
 };
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8053,19 +8053,29 @@
 return ExprError();
   }
   if (!ZeroInitializationFixit.empty()) {
-unsigned DiagID = diag::err_default_init_const;
-if (Decl *D = Entity.getDecl())
-  if (S.getLangOpts().MSVCCompat && D->hasAttr())
-DiagID = diag::ext_default_init_const;
+const Decl *D = Entity.getDecl();
+const VarDecl *VD = dyn_cast_or_null(D);
+QualType DestType = Entity.getType();
 
 // The initialization would have succeeded with this fixit.

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Victor Campos via Phabricator via cfe-commits
vhscampos added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:2217
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this, nullptr);
+  return tryEmitGlobalCompoundLiteral(emitter, E);

This constructor has the second parameter optional anyway. I suggest you omit 
the nullptr here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

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


[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Victor Campos via Phabricator via cfe-commits
vhscampos added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:2217
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this, nullptr);
+  return tryEmitGlobalCompoundLiteral(emitter, E);

vhscampos wrote:
> This constructor has the second parameter optional anyway. I suggest you omit 
> the nullptr here.
To clarify, when I say optional, I mean it has a default value with is already 
nullptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

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


[PATCH] D130701: [clang-tidy] Rename a local cmake variables to match the new tool name. NFC.

2022-08-11 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

Ping @sammccall  (although I guess I could just go ahead and commit this as it 
is quite trivial, but as I sent it out for review in the first place...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130701

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 451785.
dongjunduo added a comment.

[Clang] add -### for debug


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,9 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4736,6 +4736,79 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile = C.getArgs()
+   .getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  if (TimeTrace || TimeTraceFile) {
+StringRef LinkingResParentPath;
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(SmallString<128>(
+  J.getOutputFilenames()[0].c_str(
+  LinkingResParentPath = llvm::sys::path::parent_path(SmallString<128>(
+ J.getOutputFilenames()[0].c_str()));
+else
+  LinkingResParentPath = ".";
+  }
+}
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (LinkingResParentPath.empty()){
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+SmallString<128> TracePath(LinkingResParentPath);
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+llvm::sys::path::replace_extension(TracePath, "json");
+arg += std::string(TracePath.c_str());
+  }
+} else {
+  // /full_file_path_specified or /path_specified/yyy.json
+  SmallString<128> TracePath(
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+  llvm::sys::path::replace_extension(TracePath, "json");
+  arg += std::string(TracePath.c_str());
+}
+
+const std::string::size_type siz

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

2022-08-11 Thread mgabka via Phabricator via cfe-commits
mgabka updated this revision to Diff 451791.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

Files:
  clang/utils/TableGen/SveEmitter.cpp


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,8 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
+  OS << "#undef __aio\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,8 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
+  OS << "#undef __aio\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

2022-08-11 Thread mgabka via Phabricator via cfe-commits
mgabka marked an inline comment as done.
mgabka added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:1285
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";

paulwalker-arm wrote:
> Can you also do this for `__aio`?
sure, I didn't notice that one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

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


[PATCH] D90275: [clang][IR] Add support for leaf attribute

2022-08-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Was there any follow up? It seems that this hint (attribute) is currently not 
used by LLVM passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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


[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Ties Stuij via Phabricator via cfe-commits
stuij updated this revision to Diff 451794.
stuij added a comment.

addressed review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/ConstantEmitter.h
  clang/test/CodeGen/const-init.c


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+// CHECK: @.compoundliteral = internal global [1 x float] [float 
0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,15 @@
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
+static ConstantAddress tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
   const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1970,7 +1968,9 @@
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2214,7 +2214,8 @@
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s | FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,8 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4
 }
+
+// CHECK: @.compoundliteral = internal global [1 x float] [float 0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

2022-08-11 Thread Ties Stuij via Phabricator via cfe-commits
stuij marked 2 inline comments as done.
stuij added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:2217
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this, nullptr);
+  return tryEmitGlobalCompoundLiteral(emitter, E);

vhscampos wrote:
> vhscampos wrote:
> > This constructor has the second parameter optional anyway. I suggest you 
> > omit the nullptr here.
> To clarify, when I say optional, I mean it has a default value with is 
> already nullptr
done. thanks!



Comment at: clang/test/CodeGen/const-init.c:1
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 

simon_tatham wrote:
> I think some kind of a comment would be useful saying what this option is 
> doing there -- at least, which one of the tests further down the file it's 
> supposed to apply to. Otherwise I could easily imagine someone throwing it 
> out again, and since the test would pass anyway, not noticing that it's no 
> longer testing what it's meant to test.
thanks, yes is done. I also added a check on generation of the float value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

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


[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-08-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:57
+
+* GCC >= 7.1
+* Clang >= 5.0

Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 5.0 
and MSVC 16.7.

It is bad to promise something and then breakages here and there, for example: 
https://github.com/llvm/llvm-project/issues/57057 so probably no such bots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 451796.
dongjunduo added a comment.

Add more debug info


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,9 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4736,6 +4736,83 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile = C.getArgs()
+   .getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  if (TimeTrace || TimeTraceFile) {
+StringRef LinkingResParentPath;
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(SmallString<128>(
+  J.getOutputFilenames()[0].c_str(
+  LinkingResParentPath = llvm::sys::path::parent_path(SmallString<128>(
+ J.getOutputFilenames()[0].c_str()));
+else
+  LinkingResParentPath = ".";
+  }
+}
+printf("LinkingPath: %s\n", LinkingResParentPath.str().c_str());
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (LinkingResParentPath.empty()){
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+SmallString<128> TracePath(LinkingResParentPath);
+printf("TracePath: %s\n", TracePath.c_str());
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+printf("TracePath: %s\n", TracePath.c_str());
+llvm::sys::path::replace_extension(TracePath, "json");
+printf("TracePath: %s\n", TracePath.c_str());
+arg += std::string(TracePath.c_str());
+  }
+} else {
+  // /full_file_path_specified or /path_specified/yyy.json
+  SmallString<128> TracePath(
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath,
+  

[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-11 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: andrewng, phosek, beanz, russell.gallop.
CarlosAlbertoEnciso added a project: LLVM.
Herald added a subscriber: mgorny.
Herald added a project: All.
CarlosAlbertoEnciso requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For bootstrap builds (CLANG_ENABLE_BOOTSTRAP=ON) allow 
arguments to be passed to the native tool used in CMake 
for the stage2 step.

Can be used to pass extra arguments for enhanced versions 
of build tools, e.g. distributed build options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131665

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-08-11 Thread Tobias Hieta via Phabricator via cfe-commits
thieta added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:57
+
+* GCC >= 7.1
+* Clang >= 5.0

xbolva00 wrote:
> Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 
> 5.0 and MSVC 16.7.
> 
> It is bad to promise something and then breakages here and there, for 
> example: https://github.com/llvm/llvm-project/issues/57057 so probably no 
> such bots.
I am not sure. I think it would be good if you posted that to discourse so that 
bot owners can reply to that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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


[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  virtual void transferCFGElement(const CFGElement *Element, Lattice &L,
+  Environment &Env) {}
+

Instead of adding virtual function, please continue following the CRTP pattern. 
Add the expected function declarations in the comment above the class.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
 Lattice &L = llvm::any_cast(E.Value);
-static_cast(this)->transfer(Stmt, L, Env);
+transferCFGElement(Element, L, Env);
+





Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:112
+if (Element->getKind() == CFGElement::Statement) {
+  transfer(Element->getAs()->getStmt(), L, Env);
+}

Ditto.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:134
 /// the dataflow analysis cannot be performed successfully. Otherwise, calls
-/// `PostVisitStmt` on each statement with the final analysis results at that
-/// program point.
+/// `PostVisit` on each element with the final analysis results at that program
+/// point.





Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
+   typename AnalysisT::Lattice> &)>
+PostVisit = nullptr) {
+  std::function(), State);
+  }

PTAL at the intended usage of getAs() here: 
llvm-project/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp

That is, it should be used like dyn_cast in if statements.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:260
 
-/// Transfers `State` by evaluating `CfgStmt` in the context of `Analysis`.
-/// `HandleTransferredStmt` (if provided) will be applied to `CfgStmt`, after 
it
-/// is evaluated.
-static void transferCFGStmt(
-const ControlFlowContext &CFCtx,
-llvm::ArrayRef> 
BlockStates,
-const CFGStmt &CfgStmt, TypeErasedDataflowAnalysis &Analysis,
-TypeErasedDataflowAnalysisState &State,
-std::function
-HandleTransferredStmt) {
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt &CfgStmt,





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:261
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt &CfgStmt,
+ TypeErasedDataflowAnalysisState &InputState,

"CfgStmt" is violating the style guide casing rules.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:270-271
 
-/// Transfers `State` by evaluating `CfgInit`.
-static void transferCFGInitializer(const CFGInitializer &CfgInit,
-   TypeErasedDataflowAnalysisState &State) {
-  const auto &ThisLoc = *cast(
-  State.Env.getThisPointeeStorageLocation());
+/// Built-in transfer function for `CfgInit`.
+void builtInTransfer(const CFGInitializer &CfgInit,
+ TypeErasedDataflowAnalysisState &InputState) {

Ditto.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:325
+  case CFGElement::Statement: {
+builtInTransfer(*Element.getAs(), State, TC);
+break;

Should be using castAs().



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+  case CFGElement::Initializer: {
+builtInTransfer(*Element.getAs(), State);
+break;

castAs()



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:438
 } // namespace dataflow
-} // namespace clang
+} // namespace clang

Please add the newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-08-11 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov edited the summary of this revision.
mizvekov updated this revision to Diff 451799.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaTemplate/deduction.cpp
  libcxx/DELETE.ME

Index: libcxx/DELETE.ME
===
--- /dev/null
+++ libcxx/DELETE.ME
@@ -0,0 +1 @@
+D111283
Index: clang/test/SemaTemplate/deduction.cpp
===
--- clang/test/SemaTemplate/deduction.cpp
+++ clang/test/SemaTemplate/deduction.cpp
@@ -162,6 +162,15 @@
 
 } // namespace test4
 
+namespace test5 {
+
+template  class a {};
+template  void c(b, b);
+template  void c(a, a);
+void d() { c(a(), a()); }
+
+} // namespace test5
+
 // Verify that we can deduce enum-typed arguments correctly.
 namespace test14 {
   enum E { E0, E1 };
Index: clang/test/SemaCXX/sugared-auto.cpp
===
--- clang/test/SemaCXX/sugared-auto.cpp
+++ clang/test/SemaCXX/sugared-auto.cpp
@@ -1,4 +1,12 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20
+// RUN: %clang_cc1 -fsyntax-only -verify -xobjective-c++ %s -std=c++20 -fms-extensions -fblocks -fobjc-arc -fobjc-runtime-has-weak -fenable-matrix -Wno-dynamic-exception-spec -Wno-c++17-compat-mangling
+// RUN: %clang_cc1 -fsyntax-only -verify -xobjective-c++ %s -std=c++14 -fms-extensions -fblocks -fobjc-arc -fobjc-runtime-has-weak -fenable-matrix -Wno-dynamic-exception-spec -Wno-c++17-compat-mangling
+
+namespace std {
+template struct initializer_list {
+  const T *begin, *end;
+  initializer_list();
+};
+} // namespace std
 
 enum class N {};
 
@@ -9,6 +17,26 @@
 using Man = Animal;
 using Dog = Animal;
 
+using ManPtr = Man *;
+using DogPtr = Dog *;
+
+using SocratesPtr = ManPtr;
+
+using ConstMan = const Man;
+using ConstDog = const Dog;
+
+using Virus = void;
+using SARS = Virus;
+using Ebola = Virus;
+
+using Bacteria = float;
+using Bacilli = Bacteria;
+using Vibrio = Bacteria;
+
+struct Plant;
+using Gymnosperm = Plant;
+using Angiosperm = Plant;
+
 namespace variable {
 
 auto x1 = Animal();
@@ -25,6 +53,9 @@
 N t4 = x4; // expected-error {{lvalue of type 'Man' (aka 'int')}}
 N t5 = x5; // expected-error {{lvalue of type 'Dog' (aka 'int')}}
 
+auto x6 = { Man(), Dog() };
+N t6 = x6; // expected-error {{from 'std::initializer_list' (aka 'initializer_list')}}
+
 } // namespace variable
 
 namespace function_basic {
@@ -41,3 +72,160 @@
 N t3 = x3; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
 
 } // namespace function_basic
+
+namespace function_multiple_basic {
+
+N t1 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t2 = []() -> decltype(auto) { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t3 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Dog();
+  auto x = Man();
+  return x;
+}();
+
+N t4 = [] { // expected-error {{rvalue of type 'int'}}
+  if (true)
+return Dog();
+  return 1;
+}();
+
+N t5 = [] { // expected-error {{rvalue of type 'Virus' (aka 'void')}}
+  if (true)
+return Ebola();
+  return SARS();
+}();
+
+N t6 = [] { // expected-error {{rvalue of type 'void'}}
+  if (true)
+return SARS();
+  return;
+}();
+
+} // namespace function_multiple_basic
+
+#define TEST_AUTO(X, A, B) \
+  static_assert(__is_same(A, B), ""); \
+  auto X(A a, B b) {   \
+if (0) \
+  return a;\
+if (0) \
+  return b;\
+return N();\
+  }
+#define TEST_DAUTO(X, A, B) \
+  static_assert(__is_same(A, B), ""); \
+  decltype(auto) X(A a, B b) {  \
+if (0)  \
+  return static_cast(a); \
+if (0)  \
+  return static_cast(b); \
+return N(); \
+  }
+
+namespace misc {
+
+TEST_AUTO(t1, ManPtr, DogPtr)  // expected-error {{but deduced as 'Animal *' (aka 'int *')}}
+TEST_AUTO(t2, ManPtr, int *)   // expected-error {{but deduced as 'int *'}}
+TEST_AUTO(t3, SocratesPtr, ManPtr) // expected-error {{but deduced as 'ManPtr' (aka 'int *')}}
+
+TEST_AU

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-08-11 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

In D111283#3712348 , @erichkeane 
wrote:

> This is a sizable patch, and it isn't really clear from context what I'm 
> looking at.  Can you please improve the description/commit message to better 
> clarify what you are trying to do, what you did, and how you are trying to 
> accomplish it?

Updated description, also, for reference, the other patches that continue this 
work are D111509  and D130308 
.

Thanks for taking a look!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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


[clang-tools-extra] e935f7f - [pseudo] Fix a bug in checking the duplicated grammar rules.

2022-08-11 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-08-11T13:16:01+02:00
New Revision: e935f7fd0cbc698438bdabc3b836ab25baa9a174

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

LOG: [pseudo] Fix a bug in checking the duplicated grammar rules.

Added: 


Modified: 
clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
clang-tools-extra/pseudo/unittests/GrammarTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp 
b/clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
index 410b151ff2145..9706f17eab848 100644
--- a/clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
+++ b/clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
@@ -327,8 +327,13 @@ class GrammarBuilder {
 "Token-like name {0} is used as a nonterminal", 
G.symbolName(SID)));
   }
 }
-for (RuleID RID = 0; RID + 1u < T.Rules.size(); ++RID) {
-  if (T.Rules[RID] == T.Rules[RID + 1])
+llvm::DenseSet VisitedRules;
+for (RuleID RID = 0; RID < T.Rules.size(); ++RID) {
+  const auto &R = T.Rules[RID];
+  auto Code = llvm::hash_combine(
+  R.Target, llvm::hash_combine_range(R.seq().begin(), R.seq().end()));
+  auto [_, New] = VisitedRules.insert(Code);
+  if (!New)
 Diagnostics.push_back(
 llvm::formatv("Duplicate rule: `{0}`", G.dumpRule(RID)));
 }

diff  --git a/clang-tools-extra/pseudo/unittests/GrammarTest.cpp 
b/clang-tools-extra/pseudo/unittests/GrammarTest.cpp
index e8641f8edf6f1..6b6b47b8a2dbe 100644
--- a/clang-tools-extra/pseudo/unittests/GrammarTest.cpp
+++ b/clang-tools-extra/pseudo/unittests/GrammarTest.cpp
@@ -136,6 +136,18 @@ TEST_F(GrammarTest, Diagnostics) {
  "Unknown attribute 'unknown'"));
 }
 
+TEST_F(GrammarTest, DuplicatedDiagnostics) {
+  build(R"cpp(
+_ := test
+
+test := INT
+test := DOUBLE
+test := INT
+  )cpp");
+
+  EXPECT_THAT(Diags, UnorderedElementsAre("Duplicate rule: `test := INT`"));
+}
+
 TEST_F(GrammarTest, FirstAndFollowSets) {
   build(
   R"bnf(



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


[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

2022-08-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm! let me know if i should land this for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130363

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


[clang] ef110a4 - [Builtins] Do not claim most libfuncs are readnone with trapping math.

2022-08-11 Thread Florian Hahn via cfe-commits

Author: Florian Hahn
Date: 2022-08-11T12:29:01+01:00
New Revision: ef110a491f702e84d3f97e7cde6dacea99fd238d

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

LOG: [Builtins] Do not claim most libfuncs are readnone with trapping math.

At the moment, Clang only considers errno when deciding if a builtin
is const. This ignores the fact that some library functions may raise
floating point exceptions, which may modify global state, e.g. when
updating FP status registers.

To model the fact that some library functions/builtins may raise
floating point exceptions, this patch adds a new 'g' modifier for
builtins. If a builtin is marked with 'g', it cannot be considered
const, unless FP exceptions are ignored.

So far I've not added CHECK lines for all calls in math-libcalls.c. I'll
do that once we agree on the overall direction.

A consequence seems to be that we fail to select some of the constrained
math builtins now, but I am not entirely sure what's going on there.

Reviewed By: john.brawn

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

Added: 


Modified: 
clang/include/clang/Basic/Builtins.def
clang/include/clang/Basic/Builtins.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGen/math-libcalls.c

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index c67f0dfe9ab04..249a3913dc0b6 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -97,7 +97,8 @@
 //  S:N: -> similar to the s:N: attribute, but the function is like vscanf
 //  in that it accepts its arguments as a va_list rather than
 //  through an ellipsis
-//  e -> const, but only when -fno-math-errno
+//  e -> const, but only when -fno-math-errno and FP exceptions are ignored
+//  g -> const when FP exceptions are ignored
 //  j -> returns_twice (like setjmp)
 //  u -> arguments are not evaluated for their side-effects
 //  V:N: -> requires vectors of at least N bits to be legal
@@ -1340,7 +1341,7 @@ LIBBUILTIN(ilogbf, "if", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(ilogbl, "iLd", "fne", "math.h", ALL_LANGUAGES)
 
 // POSIX math.h declares a global, signgam, that lgamma writes to, so these
-// shouldn't have "e" or "c" attributes
+// shouldn't have "e", "c" or "g" attributes
 LIBBUILTIN(lgamma, "dd", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(lgammaf, "ff", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(lgammal, "LdLd", "fn", "math.h", ALL_LANGUAGES)
@@ -1401,9 +1402,9 @@ LIBBUILTIN(remquo, "dddi*", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(remquof, "fffi*", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(remquol, "LdLdLdi*", "fn", "math.h", ALL_LANGUAGES)
 
-LIBBUILTIN(rint, "dd", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(rintf, "ff", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(rintl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(rint, "dd", "fng", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(rintf, "ff", "fng", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(rintl, "LdLd", "fng", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(round, "dd", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(roundf, "ff", "fnc", "math.h", ALL_LANGUAGES)

diff  --git a/clang/include/clang/Basic/Builtins.h 
b/clang/include/clang/Basic/Builtins.h
index 2761f5d282ac0..ae5a2dd340bce 100644
--- a/clang/include/clang/Basic/Builtins.h
+++ b/clang/include/clang/Basic/Builtins.h
@@ -228,13 +228,18 @@ class Context {
 llvm::SmallVectorImpl &Encoding) const;
 
   /// Return true if this function has no side effects and doesn't
-  /// read memory, except for possibly errno.
+  /// read memory, except for possibly errno or raising FP exceptions.
   ///
-  /// Such functions can be const when the MathErrno lang option is disabled.
-  bool isConstWithoutErrno(unsigned ID) const {
+  /// Such functions can be const when the MathErrno lang option and FP
+  /// exceptions are disabled.
+  bool isConstWithoutErrnoAndExceptions(unsigned ID) const {
 return strchr(getRecord(ID).Attributes, 'e') != nullptr;
   }
 
+  bool isConstWithoutExceptions(unsigned ID) const {
+return strchr(getRecord(ID).Attributes, 'g') != nullptr;
+  }
+
   const char *getRequiredFeatures(unsigned ID) const {
 return getRecord(ID).Features;
   }

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 559a778f9fdd2..fff9fb44e5ce0 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2204,7 +2204,15 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counte

[PATCH] D129231: [Builtins] Do not claim all libfuncs are readnone with trapping math.

2022-08-11 Thread Florian Hahn via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef110a491f70: [Builtins] Do not claim most libfuncs are 
readnone with trapping math. (authored by fhahn).

Changed prior to commit:
  https://reviews.llvm.org/D129231?vs=448628&id=451800#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129231

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/Builtins.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/math-libcalls.c

Index: clang/test/CodeGen/math-libcalls.c
===
--- clang/test/CodeGen/math-libcalls.c
+++ clang/test/CodeGen/math-libcalls.c
@@ -27,9 +27,9 @@
   // HAS_ERRNO: declare double @atan2(double noundef, double noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare float @atan2f(float noundef, float noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare x86_fp80 @atan2l(x86_fp80 noundef, x86_fp80 noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare double @atan2(double noundef, double noundef) [[READNONE:#[0-9]+]]
-  // HAS_MAYTRAP: declare float @atan2f(float noundef, float noundef) [[READNONE]]
-  // HAS_MAYTRAP: declare x86_fp80 @atan2l(x86_fp80 noundef, x86_fp80 noundef) [[READNONE]]
+  // HAS_MAYTRAP: declare double @atan2(double noundef, double noundef) [[NOT_READNONE:#[0-9]+]]
+  // HAS_MAYTRAP: declare float @atan2f(float noundef, float noundef) [[NOT_READNONE]]
+  // HAS_MAYTRAP: declare x86_fp80 @atan2l(x86_fp80 noundef, x86_fp80 noundef) [[NOT_READNONE]]
 
   copysign(f,f); copysignf(f,f);copysignl(f,f);
 
@@ -63,7 +63,7 @@
   // HAS_ERRNO: declare double @frexp(double noundef, i32* noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare float @frexpf(float noundef, i32* noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare x86_fp80 @frexpl(x86_fp80 noundef, i32* noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare double @frexp(double noundef, i32* noundef) [[NOT_READNONE:#[0-9]+]]
+  // HAS_MAYTRAP: declare double @frexp(double noundef, i32* noundef) [[NOT_READNONE]]
   // HAS_MAYTRAP: declare float @frexpf(float noundef, i32* noundef) [[NOT_READNONE]]
   // HAS_MAYTRAP: declare x86_fp80 @frexpl(x86_fp80 noundef, i32* noundef) [[NOT_READNONE]]
 
@@ -75,9 +75,9 @@
   // HAS_ERRNO: declare double @ldexp(double noundef, i32 noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare float @ldexpf(float noundef, i32 noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare x86_fp80 @ldexpl(x86_fp80 noundef, i32 noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare double @ldexp(double noundef, i32 noundef) [[READNONE]]
-  // HAS_MAYTRAP: declare float @ldexpf(float noundef, i32 noundef) [[READNONE]]
-  // HAS_MAYTRAP: declare x86_fp80 @ldexpl(x86_fp80 noundef, i32 noundef) [[READNONE]]
+  // HAS_MAYTRAP: declare double @ldexp(double noundef, i32 noundef) [[NOT_READNONE]]
+  // HAS_MAYTRAP: declare float @ldexpf(float noundef, i32 noundef) [[NOT_READNONE]]
+  // HAS_MAYTRAP: declare x86_fp80 @ldexpl(x86_fp80 noundef, i32 noundef) [[NOT_READNONE]]
 
   modf(f,d);   modff(f,fp);  modfl(f,l);
 
@@ -125,9 +125,9 @@
 // HAS_ERRNO: declare double @acos(double noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @acosf(float noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @acosl(x86_fp80 noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare double @acos(double noundef) [[READNONE]]
-// HAS_MAYTRAP: declare float @acosf(float noundef) [[READNONE]]
-// HAS_MAYTRAP: declare x86_fp80 @acosl(x86_fp80 noundef) [[READNONE]]
+// HAS_MAYTRAP: declare double @acos(double noundef) [[NOT_READNONE]]
+// HAS_MAYTRAP: declare float @acosf(float noundef) [[NOT_READNONE]]
+// HAS_MAYTRAP: declare x86_fp80 @acosl(x86_fp80 noundef) [[NOT_READNONE]]
 
 
   acosh(f);  acoshf(f); acoshl(f);
@@ -138,9 +138,9 @@
 // HAS_ERRNO: declare double @acosh(double noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @acoshf(float noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @acoshl(x86_fp80 noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare double @acosh(double noundef) [[READNONE]]
-// HAS_MAYTRAP: declare float @acoshf(float noundef) [[READNONE]]
-// HAS_MAYTRAP: declare x86_fp80 @acoshl(x86_fp80 noundef) [[READNONE]]
+// HAS_MAYTRAP: declare double @acosh(double noundef) [[NOT_READNONE]]
+// HAS_MAYTRAP: declare float @acoshf(float noundef) [[NOT_READNONE]]
+// HAS_MAYTRAP: declare x86_fp80 @acoshl(x86_fp80 noundef) [[NOT_READNONE]]
 
   asin(f);   asinf(f);  asinl(f);
 
@@ -150,9 +150,9 @@
 // HAS_ERRNO: declare double @asin(double noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @asinf(float noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @asinl(x86_fp80 noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare double @asin(double noundef) [[READNONE]]
-// HAS_MAYTRAP: declare float @asinf(float noundef) [[READNONE]]
-// HAS_MAYTRAP: declare x86_fp80

[PATCH] D131657: Remove redundant condition check, NFC

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LTGM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131657

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


[PATCH] D129231: [Builtins] Do not claim all libfuncs are readnone with trapping math.

2022-08-11 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments.



Comment at: clang/include/clang/Basic/Builtins.def:1409
 
-LIBBUILTIN(round, "dd", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(roundf, "ff", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(roundl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(round, "dd", "fng", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(roundf, "ff", "fng", "math.h", ALL_LANGUAGES)

john.brawn wrote:
> scanon wrote:
> > `round` is just like `trunc` or `floor` or `ceil`, and should be "fnc" if 
> > they are.
> I think they all should be the same, but I'm note sure if they should all be 
> "fnc" or "fng".
> 
> Looking at the C17 standard, in section F.10.6 (IEE754/IEC60559 behaviour of 
> nearest integer functions) round, trunc, floor, and ceil are all described in 
> the same way: "The xyz functions may, but are not required to, raise the 
> "inexact" floating-point exception for finite non-integer arguments". I don't 
> know if our policy should be "These functions may raise an exception, 
> therefore we should assume they do and treat that as a side-effect", or 
> "These functions are not required to raise an exception, therefore a program 
> can't rely on it and so we don't need to treat it as a side-effect".
> 
> Looking at implementations of these functions, it looks like GNU libm doesn't 
> raise inexact, but the bionic libm does. I think I'm leaning towards marking 
> all of them as "fng" as it's the more cautious of the two.
> Looking at implementations of these functions, it looks like GNU libm doesn't 
> raise inexact, but the bionic libm does. I think I'm leaning towards marking 
> all of them as "fng" as it's the more cautious of the two.

Hmm, bionic's behavior sounds a bit surprising. In the committed version, I 
kept them `fnc` to keep the previous behavior for now. I'll double-check with 
@scanon and potentially upload a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129231

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


[clang] 7309e8c - Do not use the same identifier for a data member as a type; NFC

2022-08-11 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-11T07:32:32-04:00
New Revision: 7309e8cfbe59844b4e0128c3f48eb1a68d8bad68

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

LOG: Do not use the same identifier for a data member as a type; NFC

This should help address a build failure found after
09117b21890c652994f7ada0229d309b35b44259

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of ‘const 
clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
clang::Expr*)Expr’ changes meaning of ‘Expr’ [-fpermissive]
1 |   const Expr *Expr;
  |   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as 
‘class clang::Expr’
  109 | class Expr : public ValueStmt {
  |   ^~~~

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 13eb9acb0e5e..ab3af1298be5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16663,14 +16663,14 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) 
{
   return;
 
 struct {
-  const clang::Expr *Expr;
+  const clang::Expr *Cond;
   Expr::EvalResult Result;
   SmallString<12> ValueString;
   bool Print;
 } DiagSide[2] = {{LHS, Expr::EvalResult(), {}, false},
  {RHS, Expr::EvalResult(), {}, false}};
 for (unsigned I = 0; I < 2; I++) {
-  const Expr *Side = DiagSide[I].Expr;
+  const Expr *Side = DiagSide[I].Cond;
 
   Side->EvaluateAsRValue(DiagSide[I].Result, Context, true);
 



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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D130894#3715143 , @xbolva00 wrote:

> In D130894#3715124 , @mstorsjo 
> wrote:
>
>> This broke building with GCC (also noted by buildbot mails):
>>
>>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
>> clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
>>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of 
>> ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
>> clang::Expr*)Expr’ changes meaning of ‘Expr’ 
>> [-fpermissive]
>>   1 |   const Expr *Expr;
>> |   ^~~~
>>   In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
>>from ../tools/clang/include/clang/AST/ASTLambda.h:18,
>>from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
>>   ../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here 
>> as ‘class clang::Expr’
>> 109 | class Expr : public ValueStmt {
>> |   ^~~~
>
> It is kinda bad that GCC throws an error and Clang does not even print a 
> warning.
>
> How is it even possible? Clang does not implement strict(er) rules ? 
> @aaron.ballman

I pushed up a change to "fix" this, but I believe GCC is wrong to give an error 
here. At least, nobody else gives one and GCC is inconsistent about what it 
diagnoses: https://godbolt.org/z/hTqM8MPch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 451804.
dongjunduo added a comment.

Add more debug info


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,9 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4736,6 +4736,90 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile = C.getArgs()
+   .getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  if (TimeTrace || TimeTraceFile) {
+StringRef LinkingResParentPath;
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(SmallString<128>(
+  J.getOutputFilenames()[0].c_str(
+  LinkingResParentPath = llvm::sys::path::parent_path(SmallString<128>(
+ J.getOutputFilenames()[0].c_str()));
+else
+  LinkingResParentPath = ".";
+  }
+}
+printf("LinkingPath: %s\n", LinkingResParentPath.str().c_str());
+printf("LinkingPathD: %s\n", LinkingResParentPath.data());
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+printf("LinkingPath: %s\n", LinkingResParentPath.str().c_str());
+printf("LinkingPathD: %s\n", LinkingResParentPath.data());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (LinkingResParentPath.empty()){
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+printf("LinkingPath: %s\n", LinkingResParentPath.str().c_str());
+printf("LinkingPathD: %s\n", LinkingResParentPath.data());
+SmallString<128> TracePath(LinkingResParentPath);
+printf("LinkingPath: %s\n", LinkingResParentPath.str().c_str());
+printf("LinkingPathD: %s\n", LinkingResParentPath.data());
+printf("TracePath: %s\n", TracePath.c_str());
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+printf("TracePath: %s\n", TracePath.c_str());
+ 

[PATCH] D131194: [C++20] Fix crash-on-valid with consteval temporary construction through list initialization

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Ping


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

https://reviews.llvm.org/D131194

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In D130894#3715709 , @aaron.ballman 
wrote:

> In D130894#3715143 , @xbolva00 
> wrote:
>
>> In D130894#3715124 , @mstorsjo 
>> wrote:
>>
>>> This broke building with GCC (also noted by buildbot mails):
>>>
>>>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
>>> clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
>>>   ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of 
>>> ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
>>> clang::Expr*)Expr’ changes meaning of ‘Expr’ 
>>> [-fpermissive]
>>>   1 |   const Expr *Expr;
>>> |   ^~~~
>>>   In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
>>>from ../tools/clang/include/clang/AST/ASTLambda.h:18,
>>>from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
>>>   ../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here 
>>> as ‘class clang::Expr’
>>> 109 | class Expr : public ValueStmt {
>>> |   ^~~~
>>
>> It is kinda bad that GCC throws an error and Clang does not even print a 
>> warning.
>>
>> How is it even possible? Clang does not implement strict(er) rules ? 
>> @aaron.ballman
>
> I pushed up a change to "fix" this

FWIW, the same issue actually already was fixed by 
ebe0674acb8bb3404d0e2a6b689d5e3cd02bb0b6 
 - 
although your commit made it even clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D130894#3715725 , @mstorsjo wrote:

> In D130894#3715709 , @aaron.ballman 
> wrote:
>
>> In D130894#3715143 , @xbolva00 
>> wrote:
>>
>>> In D130894#3715124 , @mstorsjo 
>>> wrote:
>>>
 This broke building with GCC (also noted by buildbot mails):

   ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void 
 clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
   ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of 
 ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const 
 clang::Expr*)Expr’ changes meaning of ‘Expr’ 
 [-fpermissive]
   1 |   const Expr *Expr;
 |   ^~~~
   In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
from ../tools/clang/include/clang/AST/ASTLambda.h:18,
from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
   ../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared 
 here as ‘class clang::Expr’
 109 | class Expr : public ValueStmt {
 |   ^~~~
>>>
>>> It is kinda bad that GCC throws an error and Clang does not even print a 
>>> warning.
>>>
>>> How is it even possible? Clang does not implement strict(er) rules ? 
>>> @aaron.ballman
>>
>> I pushed up a change to "fix" this
>
> FWIW, the same issue actually already was fixed by 
> ebe0674acb8bb3404d0e2a6b689d5e3cd02bb0b6 
>  - 
> although your commit made it even clearer.

Oh neat, thanks for pointing that out! I reflexively rename declarations when 
they share the name of a type because I have run into similar issues to this 
before, but it's usually things like IDE "go to definition" behavior that winds 
up breaking, not the build. (I run into this all the time with people doing 
`const Attr *Attr`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D130058#3714880 , @glandium wrote:

> In D130058#3714672 , @glandium 
> wrote:
>
>> This catches 
>> https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/third_party/rust/glslopt/glsl-optimizer/src/compiler/glsl/ir_reader.cpp#732
>>  but not 
>> https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/third_party/rust/glslopt/glsl-optimizer/src/compiler/glsl/ir.cpp#654.
>>  Is that expected?
>
> Maybe something changed? A couple days ago it was catching 
> https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/gfx/cairo/cairo/src/win32/cairo-dwrite-font.cpp#238
>  but apparently not anymore.

Correct, the diagnostic was previously over-eager, but that's been fixed.

In D130058#3714882 , @glandium wrote:

> Also not caught: a cast of 0 when 0 is not a valid value in the enum.

I don't think that situation will ever be UB. When the underlying type of the 
enum is not fixed, the range of values it can represent is whatever values fit 
into a hypothetical bit-field that is large enough to cover the full range of 
stated values (https://eel.is/c++draft/enum#dcl.enum-8.sentence-2). `0` is 
something that can always be represented in such a bit-field (there's a special 
provision for empty enumerations or one that can only store 0).

In D130058#3715183 , @glandium wrote:

> Having looked around in the Firefox codebase for cases that now fail to build 
> because of this, and looking at the clarification in DR2338, I wonder if 
> enums in `extern "C"` sections should be treated as if they had an explicit 
> type of `int` as if it were e.g. `enum Foo: int { ... }`.

That doesn't match the C++ language rules for constant expression evaluation, 
so I don't think we should go that route unless WG21 decides they want to.


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

https://reviews.llvm.org/D130058

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


[PATCH] D131479: Handle explicitly defaulted consteval special members.

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:805
+void func() {
+  default_ctor fail0; // expected-error-re {{call to consteval function 
'{{.*::default_ctor<.*::foo>}}::default_ctor' is not a constant expression}} \
+  expected-note {{in call to 'default_ctor()'}}

usaxena95 wrote:
> aaron.ballman wrote:
> > Why do we need to use the regex here (and elsewhere)?
> I wanted to omit the namespace name in the error messages and make the 
> namespace more descriptive. See the `.*` in the error messages.
I think it's better to skip the regex and spell out the diagnostic fully (even 
with the descriptive namespace name). Regular expression matching is going to 
be slower than regular matching (generally) and there's no real need for it 
here except brevity.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:812
+  fail1 = good0;  // expected-error-re {{call to consteval function 
'{{.*::copy<.*::foo>}}::operator=' is not a constant expression}} \
+ expected-note {{in call to 
'&fail1->operator=(good0)'}}
+

shafik wrote:
> aaron.ballman wrote:
> > This likely has nothing to do with your changes here, but what the heck is 
> > with that leading `&`  in this already-pretty-surprisingly-weird note? Is 
> > that aiming for `(&fail1)->operator=(good0)` for some reason?
> Note, we also see a similar diagnostic in 
> `SemaCXX/constant-expression-cxx11.cpp` so this looks like it existed before.
Good catch! I filed https://github.com/llvm/llvm-project/issues/57081 so that 
hopefully we go back to make that wording better some day.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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


[PATCH] D131396: [pseudo] Use C++17 variant to simplify the DirectiveTree::Chunk class, NFC.

2022-08-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 451811.
hokein marked 2 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131396

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
  clang-tools-extra/pseudo/lib/DirectiveTree.cpp
  clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp

Index: clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
===
--- clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
+++ clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
@@ -44,7 +44,15 @@
   return testing::Matches(tokens(Tokens))(TS.tokens(arg.Tokens));
 }
 
-MATCHER_P(chunkKind, K, "") { return arg.kind() == K; }
+MATCHER(directiveChunk, "") {
+  return std::holds_alternative(arg);
+}
+MATCHER(codeChunk, "") {
+  return std::holds_alternative(arg);
+}
+MATCHER(conditionalChunk, "") {
+  return std::holds_alternative(arg);
+}
 
 TEST(DirectiveTree, Parse) {
   LangOptions Opts;
@@ -66,19 +74,16 @@
 
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
+  ASSERT_THAT(PP.Chunks, ElementsAre(directiveChunk(), codeChunk(),
+ conditionalChunk(), codeChunk()));
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Directive),
- chunkKind(Chunk::K_Code),
- chunkKind(Chunk::K_Conditional),
- chunkKind(Chunk::K_Code)));
-
-  EXPECT_THAT((const DirectiveTree::Directive &)PP.Chunks[0],
+  EXPECT_THAT(std::get(PP.Chunks[0]),
   tokensAre(S, "# include < foo . h >"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[1],
+  EXPECT_THAT(std::get(PP.Chunks[1]),
   tokensAre(S, "int main ( ) {"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[3], tokensAre(S, "}"));
+  EXPECT_THAT(std::get(PP.Chunks[3]), tokensAre(S, "}"));
 
-  const DirectiveTree::Conditional &Ifdef(PP.Chunks[2]);
+  const auto &Ifdef = std::get(PP.Chunks[2]);
   EXPECT_THAT(Ifdef.Branches,
   ElementsAre(Pair(tokensAre(S, "# ifdef HAS_FOO"), _),
   Pair(tokensAre(S, "# elif NEEDS_FOO"), _)));
@@ -87,17 +92,15 @@
   const DirectiveTree &HasFoo(Ifdef.Branches[0].second);
   const DirectiveTree &NeedsFoo(Ifdef.Branches[1].second);
 
-  EXPECT_THAT(HasFoo.Chunks, ElementsAre(chunkKind(Chunk::K_Conditional)));
-  const DirectiveTree::Conditional &If(HasFoo.Chunks[0]);
+  EXPECT_THAT(HasFoo.Chunks, ElementsAre(conditionalChunk()));
+  const auto &If = std::get(HasFoo.Chunks[0]);
   EXPECT_THAT(If.Branches, ElementsAre(Pair(tokensAre(S, "# if HAS_BAR"), _),
Pair(tokensAre(S, "# else"), _)));
-  EXPECT_THAT(If.Branches[0].second.Chunks,
-  ElementsAre(chunkKind(Chunk::K_Code)));
-  EXPECT_THAT(If.Branches[1].second.Chunks,
-  ElementsAre(chunkKind(Chunk::K_Code)));
+  EXPECT_THAT(If.Branches[0].second.Chunks, ElementsAre(codeChunk()));
+  EXPECT_THAT(If.Branches[1].second.Chunks, ElementsAre(codeChunk()));
 
-  EXPECT_THAT(NeedsFoo.Chunks, ElementsAre(chunkKind(Chunk::K_Directive)));
-  const DirectiveTree::Directive &Error(NeedsFoo.Chunks[0]);
+  EXPECT_THAT(NeedsFoo.Chunks, ElementsAre(directiveChunk()));
+  const auto &Error = std::get(NeedsFoo.Chunks[0]);
   EXPECT_THAT(Error, tokensAre(S, "# error missing_foo"));
   EXPECT_EQ(Error.Kind, tok::pp_error);
 }
@@ -114,14 +117,15 @@
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Code),
- chunkKind(Chunk::K_Directive),
- chunkKind(Chunk::K_Code)));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[0], tokensAre(S, "/*A*/"));
-  const DirectiveTree::Directive &Define(PP.Chunks[1]);
+  ASSERT_THAT(PP.Chunks,
+  ElementsAre(codeChunk(), directiveChunk(), codeChunk()));
+  EXPECT_THAT(std::get(PP.Chunks[0]),
+  tokensAre(S, "/*A*/"));
+  const auto &Define = std::get(PP.Chunks[1]);
   EXPECT_EQ(Define.Kind, tok::pp_define);
   EXPECT_THAT(Define, tokensAre(S, "# /*B*/ /*C*/ define BAR /*D*/"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[2], tokensAre(S, "/*E*/"));
+  EXPECT_THAT(std::get(PP.Chunks[2]),
+  tokensAre(S, "/*E*/"));
 }
 
 TEST(DirectiveTree, ParseBroken) {
@@ -135,20 +139,18 @@
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Code),
- chunkKind(Chunk::K_Directive),
- chunkKind(Chunk::K_Conditional)));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[0

[PATCH] D131569: [clangd] Allow updates to be canceled after compile flags retrieval

2022-08-11 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:898
+if (isCancelled()) {
+  log("ASTWorker skipping update {0} for file {1}", Inputs.Version,
+  FileName);

I'd add something like "due to cancellation". I know you can tell that from the 
line number, but at first glance someone might think it's due to lack of 
changes or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131569

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


[clang-tools-extra] bf0e219 - [pseudo] Use C++17 variant to simplify the DirectiveTree::Chunk class, NFC.

2022-08-11 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-08-11T14:27:38+02:00
New Revision: bf0e219d0481212ad12667262ed0b27e5e69f5f2

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

LOG: [pseudo] Use C++17 variant to simplify the DirectiveTree::Chunk class, NFC.

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
clang-tools-extra/pseudo/lib/DirectiveTree.cpp
clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
index e8220537649f9..2c6b1d2805697 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
@@ -30,6 +30,7 @@
 
 #include "clang-pseudo/Token.h"
 #include "clang/Basic/TokenKinds.h"
+#include 
 #include 
 
 namespace clang {
@@ -86,7 +87,7 @@ struct DirectiveTree {
   };
 
   /// Some piece of the file. {One of Code, Directive, Conditional}.
-  class Chunk; // Defined below.
+  using Chunk = std::variant;
   std::vector Chunks;
 
   /// Extract preprocessor structure by examining the raw tokens.
@@ -99,7 +100,6 @@ struct DirectiveTree {
   TokenStream stripDirectives(const TokenStream &) const;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DirectiveTree &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DirectiveTree::Chunk 
&);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DirectiveTree::Code 
&);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &,
   const DirectiveTree::Directive &);
@@ -124,48 +124,6 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &,
 /// The choices are stored in Conditional::Taken nodes.
 void chooseConditionalBranches(DirectiveTree &, const TokenStream &Code);
 
-// FIXME: This approximates std::variant.
-// Switch once we can use C++17.
-class DirectiveTree::Chunk {
-public:
-  enum Kind { K_Empty, K_Code, K_Directive, K_Conditional };
-  Kind kind() const {
-return CodeVariant  ? K_Code
-   : DirectiveVariant   ? K_Directive
-   : ConditionalVariant ? K_Conditional
-: K_Empty;
-  }
-
-  Chunk() = delete;
-  Chunk(const Chunk &) = delete;
-  Chunk(Chunk &&) = default;
-  Chunk &operator=(const Chunk &) = delete;
-  Chunk &operator=(Chunk &&) = default;
-  ~Chunk() = default;
-
-  // T => Chunk constructor.
-  Chunk(Code C) : CodeVariant(std::move(C)) {}
-  Chunk(Directive C) : DirectiveVariant(std::move(C)) {}
-  Chunk(Conditional C) : ConditionalVariant(std::move(C)) {}
-
-  // Chunk => T& and const T& conversions.
-#define CONVERSION(CONST, V)   
\
-  explicit operator CONST V &() CONST { return *V##Variant; }
-  CONVERSION(const, Code);
-  CONVERSION(, Code);
-  CONVERSION(const, Directive);
-  CONVERSION(, Directive);
-  CONVERSION(const, Conditional);
-  CONVERSION(, Conditional);
-#undef CONVERSION
-
-private:
-  // Wasteful, a union variant would be better!
-  llvm::Optional CodeVariant;
-  llvm::Optional DirectiveVariant;
-  llvm::Optional ConditionalVariant;
-};
-
 } // namespace pseudo
 } // namespace clang
 

diff  --git a/clang-tools-extra/pseudo/lib/DirectiveTree.cpp 
b/clang-tools-extra/pseudo/lib/DirectiveTree.cpp
index d5f97455685ed..c9e0198227fa4 100644
--- a/clang-tools-extra/pseudo/lib/DirectiveTree.cpp
+++ b/clang-tools-extra/pseudo/lib/DirectiveTree.cpp
@@ -10,6 +10,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/Support/FormatVariadic.h"
+#include 
 
 namespace clang {
 namespace pseudo {
@@ -141,6 +142,36 @@ class DirectiveParser {
   clang::IdentifierTable PPKeywords;
 };
 
+struct Dumper {
+  llvm::raw_ostream &OS;
+  unsigned Indent = 0;
+
+  Dumper(llvm::raw_ostream& OS) : OS(OS) {}
+  void operator()(const DirectiveTree& Tree) {
+for (const auto& Chunk : Tree.Chunks)
+  std::visit(*this, Chunk);
+  }
+  void operator()(const DirectiveTree::Conditional &Conditional) {
+for (unsigned I = 0; I < Conditional.Branches.size(); ++I) {
+  const auto &Branch = Conditional.Branches[I];
+  (*this)(Branch.first, Conditional.Taken == I);
+  Indent += 2;
+  (*this)(Branch.second);
+  Indent -= 2;
+}
+(*this)(Conditional.End);
+  }
+  void operator()(const DirectiveTree::Directive &Directive,
+  bool Taken = false) {
+OS.indent(Indent) << llvm::formatv(
+"#{0} ({1} tokens){2}\n", tok::getPPKeywordSpelling(Directive.Kind),
+Directive.Tokens.size(), Taken ? " TAKEN" : "");
+  }
+  void operator()(const DirectiveTree:

[PATCH] D131396: [pseudo] Use C++17 variant to simplify the DirectiveTree::Chunk class, NFC.

2022-08-11 Thread Haojian Wu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rGbf0e219d0481: [pseudo] Use C++17 variant to simplify the 
DirectiveTree::Chunk class, NFC. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131396

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/DirectiveTree.h
  clang-tools-extra/pseudo/lib/DirectiveTree.cpp
  clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp

Index: clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
===
--- clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
+++ clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp
@@ -44,7 +44,15 @@
   return testing::Matches(tokens(Tokens))(TS.tokens(arg.Tokens));
 }
 
-MATCHER_P(chunkKind, K, "") { return arg.kind() == K; }
+MATCHER(directiveChunk, "") {
+  return std::holds_alternative(arg);
+}
+MATCHER(codeChunk, "") {
+  return std::holds_alternative(arg);
+}
+MATCHER(conditionalChunk, "") {
+  return std::holds_alternative(arg);
+}
 
 TEST(DirectiveTree, Parse) {
   LangOptions Opts;
@@ -66,19 +74,16 @@
 
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
+  ASSERT_THAT(PP.Chunks, ElementsAre(directiveChunk(), codeChunk(),
+ conditionalChunk(), codeChunk()));
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Directive),
- chunkKind(Chunk::K_Code),
- chunkKind(Chunk::K_Conditional),
- chunkKind(Chunk::K_Code)));
-
-  EXPECT_THAT((const DirectiveTree::Directive &)PP.Chunks[0],
+  EXPECT_THAT(std::get(PP.Chunks[0]),
   tokensAre(S, "# include < foo . h >"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[1],
+  EXPECT_THAT(std::get(PP.Chunks[1]),
   tokensAre(S, "int main ( ) {"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[3], tokensAre(S, "}"));
+  EXPECT_THAT(std::get(PP.Chunks[3]), tokensAre(S, "}"));
 
-  const DirectiveTree::Conditional &Ifdef(PP.Chunks[2]);
+  const auto &Ifdef = std::get(PP.Chunks[2]);
   EXPECT_THAT(Ifdef.Branches,
   ElementsAre(Pair(tokensAre(S, "# ifdef HAS_FOO"), _),
   Pair(tokensAre(S, "# elif NEEDS_FOO"), _)));
@@ -87,17 +92,15 @@
   const DirectiveTree &HasFoo(Ifdef.Branches[0].second);
   const DirectiveTree &NeedsFoo(Ifdef.Branches[1].second);
 
-  EXPECT_THAT(HasFoo.Chunks, ElementsAre(chunkKind(Chunk::K_Conditional)));
-  const DirectiveTree::Conditional &If(HasFoo.Chunks[0]);
+  EXPECT_THAT(HasFoo.Chunks, ElementsAre(conditionalChunk()));
+  const auto &If = std::get(HasFoo.Chunks[0]);
   EXPECT_THAT(If.Branches, ElementsAre(Pair(tokensAre(S, "# if HAS_BAR"), _),
Pair(tokensAre(S, "# else"), _)));
-  EXPECT_THAT(If.Branches[0].second.Chunks,
-  ElementsAre(chunkKind(Chunk::K_Code)));
-  EXPECT_THAT(If.Branches[1].second.Chunks,
-  ElementsAre(chunkKind(Chunk::K_Code)));
+  EXPECT_THAT(If.Branches[0].second.Chunks, ElementsAre(codeChunk()));
+  EXPECT_THAT(If.Branches[1].second.Chunks, ElementsAre(codeChunk()));
 
-  EXPECT_THAT(NeedsFoo.Chunks, ElementsAre(chunkKind(Chunk::K_Directive)));
-  const DirectiveTree::Directive &Error(NeedsFoo.Chunks[0]);
+  EXPECT_THAT(NeedsFoo.Chunks, ElementsAre(directiveChunk()));
+  const auto &Error = std::get(NeedsFoo.Chunks[0]);
   EXPECT_THAT(Error, tokensAre(S, "# error missing_foo"));
   EXPECT_EQ(Error.Kind, tok::pp_error);
 }
@@ -114,14 +117,15 @@
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Code),
- chunkKind(Chunk::K_Directive),
- chunkKind(Chunk::K_Code)));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[0], tokensAre(S, "/*A*/"));
-  const DirectiveTree::Directive &Define(PP.Chunks[1]);
+  ASSERT_THAT(PP.Chunks,
+  ElementsAre(codeChunk(), directiveChunk(), codeChunk()));
+  EXPECT_THAT(std::get(PP.Chunks[0]),
+  tokensAre(S, "/*A*/"));
+  const auto &Define = std::get(PP.Chunks[1]);
   EXPECT_EQ(Define.Kind, tok::pp_define);
   EXPECT_THAT(Define, tokensAre(S, "# /*B*/ /*C*/ define BAR /*D*/"));
-  EXPECT_THAT((const DirectiveTree::Code &)PP.Chunks[2], tokensAre(S, "/*E*/"));
+  EXPECT_THAT(std::get(PP.Chunks[2]),
+  tokensAre(S, "/*E*/"));
 }
 
 TEST(DirectiveTree, ParseBroken) {
@@ -135,20 +139,18 @@
   TokenStream S = cook(lex(Code, Opts), Opts);
   DirectiveTree PP = DirectiveTree::parse(S);
 
-  ASSERT_THAT(PP.Chunks, ElementsAre(chunkKind(Chunk::K_Code),
-   

[PATCH] D131091: [clang][index] Index unresolved member expression as reference

2022-08-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131091

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


[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

2022-08-11 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

We're also still seeing the diag fire after this: 
https://ci.chromium.org/p/chromium/builders/ci/ToTLinux

(And we cleaned up our codebase back when it was still an error.)

Our bots have been red since the change to turn this into a warning landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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


[PATCH] D131646: [clang][dataflow] Restructure loops to call widen on back edges

2022-08-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

Nice!




Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:180
+  assert(Block->pred_size() == 2);
+  BackEdge = Pred;
+}

Might it be worth simply returning the backedge when you find it? Or is the 
assertion (above) sufficiently important to keep it as is?



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:275
+  )";
+  auto BlockStates = llvm::cantFail(runAnalysis(
+  Code, [](ASTContext &C) { return ConvergesOnWidenAnalysis(C); }));

Might a (googletest) assertion here be better than `llvm::cantFail`? I would 
think that this line is the crux of checking whether it converges or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131646

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


[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131351#3714968 , @brooksmoses 
wrote:

> For the record, so far we've seen this showing up in the following:
>
> - A case in a notoriously warning-heavy third-party library where we'd 
> backported a file from a newer version and didn't quite fix all the internal 
> API mismatches.
> - A ten-year-old bug in a local patch to another third-party library, where a 
> function-pointer parameter was defined as returning a `void` and then 
> assigned to a function-pointer that returns a `void *`.
> - A probably-innocuous bug in a local patch to yet another third-party 
> library, where we were passing an `int foo(char *, char *)` function to 
> GLIBC's `qsort`, which expects a function with a signature of `int foo(void 
> *, void *)`.
> - A case where 
> https://gitlab.freedesktop.org/pixman/pixman/-/commit/e0d4403e78a7af8f4be4110ae25e9c3d1ac93a78
>  wasn't applied to our local version.  This is also probably an innocuous 
> case, not a "real" bug.
> - A case where SciPy's extension has a function that uses `void *` for a 
> `FILE *` pointer 
> (https://github.com/scipy/scipy/blob/main/scipy/spatial/_qhull.pyx#L187, 
> second argument) while the corresponding C code's function has a real `FILE 
> *` pointer 
> (https://github.com/qhull/qhull/blob/master/src/libqhull_r/io_r.h#L97).  The 
> SciPy function also uses a `void *` for an argument of a struct type, which 
> seems rather odd to me given that it just defined the type two lines earlier.
>
> So, real bugs in 40% of cases, I'd say.

I double-checked these scenarios against the standard (where possible) and all 
of them are undefined behavior per the spec (and would be things I would expect 
a type sanitizer to report on), but I agree that not all of them seem likely to 
lead to a security vulnerability.

In D131351#3714993 , @MaskRay wrote:

> Nit: I think it is useful mentioning 
> `-Wincompatible-pointer-types``-Wincompatible-function-pointer-types` in the 
> commit message, not just in `clang/docs/ReleaseNotes.rst`, so that `git log 
> --grep incompatible-function-pointer-types` will reveal something when the 
> user knows incompatible-function-pointer-types but not "implicit function 
> pointer conversions diagnostic".

Good point, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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


[PATCH] D131645: [clang][dataflow] Allow user-provided lattices to provide a widen operator

2022-08-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:119
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs("int x = 0;", {"-std=c++11"});
+  HasWidenAnalysis Analysis(AST->getASTContext(),

nit: why c++11 (vs something later)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131645

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Btw,

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

2022-08-11 Thread mgabka via Phabricator via cfe-commits
mgabka marked an inline comment as done.
mgabka added a comment.

It is a fix for https://github.com/llvm/llvm-project/issues/57083


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D130894#3715884 , @xbolva00 wrote:

> Btw,
>
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html

Yup, that paper is currently closed in the WG21 paper tracker FWIW.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130894

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


[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

2022-08-11 Thread Furkan via Phabricator via cfe-commits
furkanusta marked an inline comment as not done.
furkanusta added a comment.

That would be great, thanks




Comment at: clang/test/CodeCompletion/overrides.cpp:41
+// Runs completion at empty line on line 13.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:13:1 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: COMPLETION: Pattern : void vfunc(bool param) override{{$}}

kadircet wrote:
> no need for `-code-completion-patterns`. can you also move the new test case 
> below (after `CHECK-CC3-NOT`) that way we get to keep the line numbers for 
> old tests the same.
I put the test in between because anything after vf on line 14 gives an error. 

error: unknown type name 'vf'
  vf
  ^

It still outputs the correct completions but since the return code is non-zero, 
FileCheck fails 



Comment at: clang/test/CodeCompletion/overrides.cpp:41
+// Runs completion at empty line on line 13.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:13:1 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: COMPLETION: Pattern : void vfunc(bool param) override{{$}}

furkanusta wrote:
> kadircet wrote:
> > no need for `-code-completion-patterns`. can you also move the new test 
> > case below (after `CHECK-CC3-NOT`) that way we get to keep the line numbers 
> > for old tests the same.
> I put the test in between because anything after vf on line 14 gives an 
> error. 
> 
> error: unknown type name 'vf'
>   vf
>   ^
> 
> It still outputs the correct completions but since the return code is 
> non-zero, FileCheck fails 
I've found from one of the others tests it is possible to invert the return 
code from clang.
This way I  was able to move the newly added test to bottom.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130363

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


[clang] eb91fd5 - [clang][dataflow] Analyze constructor bodies

2022-08-11 Thread Wei Yi Tee via cfe-commits

Author: Sam Estep
Date: 2022-08-11T12:46:20Z
New Revision: eb91fd5cbc6995099bd38c04c839686cbdd18b92

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

LOG: [clang][dataflow] Analyze constructor bodies

This patch adds the ability to context-sensitively analyze constructor bodies, 
by changing `pushCall` to allow both `CallExpr` and `CXXConstructExpr`, and 
extracting the main context-sensitive logic out of `VisitCallExpr` into a new 
`transferInlineCall` method which is now also called at the end of 
`VisitCXXConstructExpr`.

Reviewed By: ymandel, sgatev, xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 94fe2be37b2eb..c30a76267716d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -135,7 +135,7 @@ class Environment {
   ///
   /// Requirements:
   ///
-  ///  The callee of `Call` must be a `FunctionDecl` with a body.
+  ///  The callee of `Call` must be a `FunctionDecl`.
   ///
   ///  The body of the callee must not reference globals.
   ///
@@ -143,6 +143,7 @@ class Environment {
   ///
   ///  Each argument of `Call` must already have a `StorageLocation`.
   Environment pushCall(const CallExpr *Call) const;
+  Environment pushCall(const CXXConstructExpr *Call) const;
 
   /// Moves gathered information back into `this` from a `CalleeEnv` created 
via
   /// `pushCall`.
@@ -381,6 +382,12 @@ class Environment {
   StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
 
+  /// Shared implementation of `pushCall` overloads. Note that unlike
+  /// `pushCall`, this member is invoked on the environment of the callee, not
+  /// of the caller.
+  void pushCallInternal(const FunctionDecl *FuncDecl,
+ArrayRef Args);
+
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 16c83cad9d9e3..e4af68e53e14e 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -207,52 +207,68 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 
 Environment Environment::pushCall(const CallExpr *Call) const {
   Environment Env(*this);
-  // FIXME: Support references here.
-  Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
-
-  const auto *FuncDecl = Call->getDirectCallee();
-  assert(FuncDecl != nullptr);
 
-  Env.setDeclCtx(FuncDecl);
-
-  // FIXME: In order to allow the callee to reference globals, we probably need
-  // to call `initGlobalVars` here in some way.
+  // FIXME: Support references here.
+  Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
 
   if (const auto *MethodCall = dyn_cast(Call)) {
 if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
-  Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+  Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
 }
   }
 
+  Env.pushCallInternal(Call->getDirectCallee(),
+   llvm::makeArrayRef(Call->getArgs(), 
Call->getNumArgs()));
+
+  return Env;
+}
+
+Environment Environment::pushCall(const CXXConstructExpr *Call) const {
+  Environment Env(*this);
+
+  // FIXME: Support references here.
+  Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
+
+  Env.ThisPointeeLoc = Env.ReturnLoc;
+
+  Env.pushCallInternal(Call->getConstructor(),
+   llvm::makeArrayRef(Call->getArgs(), 
Call->getNumArgs()));
+
+  return Env;
+}
+
+void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
+   ArrayRef Args) {
+  setDeclCtx(FuncDecl);
+
+  // FIXME: In order to allow the callee to reference globals, we probably need
+  // to call `initGlobalVars` here in some way.
+
   auto ParamIt = FuncDecl->param_begin();
-  auto ArgIt = Call->arg_begin();
-  auto ArgEnd = Call->arg_end();
 
   // FIXME: Parameters don't always map to arguments 1:1; examples include
   // overloaded operators implemented as member functions, and parameter packs.
-  for (; ArgIt != ArgEnd; ++ParamIt, ++ArgI

[PATCH] D131675: [clang] SIGSEGV fix at clang::ASTContext::getRawCommentForDeclNoCacheImpl

2022-08-11 Thread Ivan Murashko via Phabricator via cfe-commits
ivanmurashko created this revision.
ivanmurashko added reviewers: sammccall, aaron.ballman.
Herald added subscribers: usaxena95, kadircet.
Herald added a project: All.
ivanmurashko requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

The `File` might point to an invalid `FileID` at the case of broken AST. That 
leads to clang/clangd crash while processing comments. Relevant part of the 
crash is below

   #4 0x7f1d7fbf95bc std::_Rb_tree, std::_Select1st>, std::less, 
std::allocator>>::_M_lower_bound(std::_Rb_tree_node> const*, std::_Rb_tree_node_base const*, 
unsigned int const&) const /usr/include/c++/8/bits/stl_tree.h:1911:2
   #5 0x7f1d7fbf95bc std::_Rb_tree, std::_Select1st>, std::less, 
std::allocator>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_tree.h:1214:56
   #6 0x7f1d7fbf95bc std::map, std::allocator>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_map.h:1264:3
  6
   #7 0x7f1d7fbf95bc 
clang::ASTContext::getRawCommentForDeclNoCacheImpl(clang::Decl const*, 
clang::SourceLocation, std::map, std::allocator>> const&) const 
/home/ivanmurashko/local/llvm-project/clang/lib/AST/ASTContext.cpp:226:57

The corresponding lit test that reproduces that crash was also added


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131675

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/AST/ast-crash-doc.cpp


Index: clang/test/AST/ast-crash-doc.cpp
===
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa 
%t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I 
%t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+header "a.h"
+header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
 return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
 return nullptr;


Index: clang/test/AST/ast-crash-doc.cpp
===
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa %t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I %t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+header "a.h"
+header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
 return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
 return nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] d09d4bd - [clang][dataflow] Don't crash when caller args are missing storage locations

2022-08-11 Thread Wei Yi Tee via cfe-commits

Author: Sam Estep
Date: 2022-08-11T13:00:42Z
New Revision: d09d4bd66c864d58b29d74918a4a164f3ad905de

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

LOG: [clang][dataflow] Don't crash when caller args are missing storage 
locations

This patch modifies `Environment`'s `pushCall` method to pass over arguments 
that are missing storage locations, instead of crashing.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index c30a76267716d..5b29915e368ed 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -140,8 +140,6 @@ class Environment {
   ///  The body of the callee must not reference globals.
   ///
   ///  The arguments of `Call` must map 1:1 to the callee's parameters.
-  ///
-  ///  Each argument of `Call` must already have a `StorageLocation`.
   Environment pushCall(const CallExpr *Call) const;
   Environment pushCall(const CXXConstructExpr *Call) const;
 

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e4af68e53e14e..119ef337c6319 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -253,7 +253,8 @@ void Environment::pushCallInternal(const FunctionDecl 
*FuncDecl,
 
 const Expr *Arg = Args[ArgIndex];
 auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
-assert(ArgLoc != nullptr);
+if (ArgLoc == nullptr)
+  continue;
 
 const VarDecl *Param = *ParamIt;
 auto &Loc = createStorageLocation(*Param);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index af06021abccfd..0e33df3a38008 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4229,6 +4229,27 @@ TEST(TransferTest, ContextSensitiveReturnArg) {
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
 }
 
+TEST(TransferTest, ContextSensitiveReturnInt) {
+  std::string Code = R"(
+int identity(int x) { return x; }
+
+void target() {
+  int y = identity(42);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+// This just tests that the analysis doesn't crash.
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
 TEST(TransferTest, ContextSensitiveMethodLiteral) {
   std::string Code = R"(
 class MyClass {



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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread Jamie Schmeiser via Phabricator via cfe-commits
jamieschmeiser added a comment.

You should not have debugging information in code that is up for review.  If 
this is debugging information that you plan to leave in for future purposes 
(which I doubt is the case here), you need to protect it so that it isn't 
active unless some option is set.  For example, see LLVM_DEBUG code that lives 
in various opt routines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.



> In D130058#3714882 , @glandium 
> wrote:
>
>> Also not caught: a cast of 0 when 0 is not a valid value in the enum.
>
> I don't think that situation will ever be UB. When the underlying type of the 
> enum is not fixed, the range of values it can represent is whatever values 
> fit into a hypothetical bit-field that is large enough to cover the full 
> range of stated values (https://eel.is/c++draft/enum#dcl.enum-8.sentence-2). 
> `0` is something that can always be represented in such a bit-field (there's 
> a special provision for empty enumerations or one that can only store 0).

Correct here.  A 'valid value' of an enum is NOT just the list of values 
listed.  It is every value that would be represent-able by the 
smallest-bit-sized bitfield required to represent all of the possible 
enumerators.  Since `0` is represent-able by all 1+ bit integers, zero is 
always valid.


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

https://reviews.llvm.org/D130058

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


[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D119051#3714645 , @dblaikie wrote:

> Realized maybe we don't need a separate driver flag for this at all, and rely 
> only on the abi-compat flag? That seems to be how (at least some) other ABI 
> compat changes have been handled, looking at other uses of `ClangABI` enum 
> values.

Agreed, I think this is the better approach.

> There could be more testing than only the indirect result of the packing 
> problem that first inspired this patch. Any suggestions on what might be the 
> most direct way to test whether the type's been considered pod in this sense?

I would have thought use of `__is_pod` would tell us, but I'm not seeing the 
behavior described in the test case when using that: 
https://godbolt.org/z/1vr3MK4KW Oddly, it seems that 
`QualType::isCXX11PODType()` doesn't look at `PlainOldData` at all! What is 
your expectation as to how the type trait should be behaving?




Comment at: clang/test/SemaCXX/class-layout.cpp:2-9
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions
+// RUN: %clang_cc1 -triple x86_64-apple-darwin%s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -DCLANG_ABI_COMPAT=14
+// RUN: %clang_cc1 -triple x86_64-scei-ps4%s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-sie-ps5 %s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -fclang-abi-compat=6 
-DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -fclang-abi-compat=14 
-DCLANG_ABI_COMPAT=14
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify 
-std=c++11 -Wno-inaccessible-base -Wno-c++11-extensions -fclang-abi-compat=15 
-DCLANG_ABI_COMPAT=15

No need for `-Wno-c++11-extensions` on these RUN lines, right (they already 
specify c++11 specifically, so there are no extensions to warn about)?



Comment at: clang/test/SemaCXX/class-layout.cpp:663-665
+_Static_assert(_Alignof(t2) == 4, "");
+#else
+_Static_assert(_Alignof(t2) == 1, "");




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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


[PATCH] D131651: [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I think this is fine.  I vaguely remember doing something like this in the 
past, but I can't seem to figure out what it was...




Comment at: clang/lib/AST/DeclBase.cpp:1037
+  assert(getDeclContext());
+  return getDeclContext()->getNonTransparentContext();
+}

so why are we calling this on our `DeclContext` here, which is the 'parent' of 
the current `Decl`?  Shouldn't this be casting itself to `DeclContext` and 
calling this function on it, so we don't end up 'skipping' the first parent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131651

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


[clang] bbb30bd - [clang][AArch64][SVE] Clarify documentation for sizeof operator on SVE

2022-08-11 Thread David Truby via cfe-commits

Author: David Truby
Date: 2022-08-11T13:22:23Z
New Revision: bbb30bd54a6447702f9f59a2ae4c478eb7133ae0

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

LOG: [clang][AArch64][SVE] Clarify documentation for sizeof operator on SVE

Previously the table in LanguageExtensions said that sizeof worked on
SVE types but this is only correct for fixed-length vectors so a
clarification has been added.

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 3b80289fd5fe..3adf0a12fc4c 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -552,7 +552,7 @@ bitwise operators &,|,^,~yes yes   yes  
   yesyes
 ==, !=, >, <, >=, <= yes yes   yes yesyes
 =yes yes   yes yesyes
 ?: [#]_  yes --yes yesyes
-sizeof   yes yes   yes yesyes
+sizeof   yes yes   yes yesyes [#]_
 C-style cast yes yes   yes no no
 reinterpret_cast yes noyes no no
 static_cast  yes noyes no no
@@ -568,6 +568,7 @@ See also :ref:`langext-__builtin_shufflevector`, 
:ref:`langext-__builtin_convert
   conversions (that is, != 0).
   If it's an extension (OpenCL) vector, it's only available in C and OpenCL C.
   And it selects base on signedness of the condition operands (OpenCL v1.1 
s6.3.9).
+.. [#] sizeof can only be used on vector length specific SVE types.
 .. [#] Clang does not allow the address of an element to be taken while GCC
allows this. This is intentional for vectors with a boolean element type and
not implemented otherwise.



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


[PATCH] D131625: [HLSL] Entry functions require param annotation

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Attr.h:193
 
+class HLSLAnnotationAttr : public InheritableAttr {
+protected:

Is this intended to be used only for parameters (that's how I read the summary 
for the patch)? If so, why is this not inheriting from `InheritableParamAttr`?



Comment at: clang/include/clang/AST/Attr.h:195-197
+  HLSLAnnotationAttr(ASTContext &Context,
+   const AttributeCommonInfo &CommonInfo, attr::Kind AK,
+   bool IsLateParsed, bool InheritEvenIfAlreadyPresent)

Formatting looks off here, you should run the patch through clang-format.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11635
 def err_hlsl_attribute_param_mismatch : Error<"%0 attribute parameters do not 
match the previous declaration">;
+def err_hlsl_missing_parameter_annotation : Error<"entry function parameter %0 
missing annotation">;
 

Will users know how to fix the issue when they get this diagnostic? "missing 
annotation" sounds like "slap any old annotation on there, it'll be fine".



Comment at: clang/lib/Sema/SemaDecl.cpp:11874
+
+  for (const auto Param : FD->parameters()) {
+if (!Param->hasAttr()) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131625

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


[PATCH] D131651: [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang/lib/AST/DeclBase.cpp:1037
+  assert(getDeclContext());
+  return getDeclContext()->getNonTransparentContext();
+}

erichkeane wrote:
> so why are we calling this on our `DeclContext` here, which is the 'parent' 
> of the current `Decl`?  Shouldn't this be casting itself to `DeclContext` and 
> calling this function on it, so we don't end up 'skipping' the first parent?
Ah, I see... `getNonTransparentContext` works differently from the other 
`getDeclContext` type functions in that it does NOT get the 1st parent, it will 
return the 'current' `DeclContext` if necessary.  So casting it would be 
incorrect here. Strange that this works opposite of the rest, but looks like 
you're right :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131651

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


[PATCH] D131657: Remove redundant condition check, NFC

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.

LGTM!  Let us know if you need this committed for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131657

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


[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D126907#3715436 , @ChuanqiXu wrote:

> In D126907#3712363 , @erichkeane 
> wrote:
>
>> In D126907#3711956 , @ChuanqiXu 
>> wrote:
>>
>>> In D126907#3710656 , @erichkeane 
>>> wrote:
>>>
 Ok, fixed the test failure in clang, AND it managed to fix 
 all the failures in libcxx!

 HOWEVER, it appears that libcxx/test/libcxx/modules_include.sh.cpp
 is now hanging?

 I don't know much about the modules implementation (perhaps someone
 like @ChuanqiXu  can help out?), so I'm at least somewhat stuck until
 I can figure out how to get it to give me more info.
>>>
>>> I may not be able to look into the details recently. What's the error 
>>> message from the modules?
>>
>> Looking deeper... this test is a monstrosity 
>> (https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/modules_include.sh.cpp),
>>  and I'm not necessarily sure it is necessarily modules-related, other than 
>> enabling it on every header.  So it just seems like one of the STL headers 
>> is taking significantly longer to compile?  I have no idea how to move 
>> forward with this... it AT LEAST "finishes", but it takes a very long time 
>> to get through now.
>
> I tried to reproduce and I am not sure if reproduced. With this patch, 
> `modules_include.sh.cpp` takes 562s to complete. And without this patch, it 
> takes a 557s. So it looks not your fault.

Hmm... ok, thanks for checking!  I'm going to try to prove to myself that it 
isn't too bad then.  This might just be a case of the Debug version of this 
taking a horribly long-time and me not noticing it normally.

If that is the case, I think this is ready for review!


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

https://reviews.llvm.org/D126907

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


[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D131528#3715841 , @thakis wrote:

> We're also still seeing the diag fire after this: 
> https://ci.chromium.org/p/chromium/builders/ci/ToTLinux
>
> (And we cleaned up our codebase back when it was still an error.)
>
> Our bots have been red since the change to turn this into a warning landed.

Are you sure that is because of THIS patch?  That appears to be the bitfield 
conversion diagnostic that was recently mucked with, and no longer this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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


[PATCH] D131573: [clang][AArch64][SVE] Change SVE_VECTOR_OPERATORS macro for VLA vectors

2022-08-11 Thread David Truby via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
DavidTruby marked an inline comment as done.
Closed by commit rG13a784f368ef: [clang][AArch64][SVE] Change 
SVE_VECTOR_OPERATORS macro for VLA vectors (authored by DavidTruby).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131573

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -150,6 +150,7 @@
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE: __ARM_FEATURE_SVE_VECTOR_OPERATORS 2
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve+bf16 -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-SVE-BF16 %s
 // CHECK-SVE-BF16: __ARM_FEATURE_BF16_SCALAR_ARITHMETIC 1
@@ -512,9 +513,7 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve 
-msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck 
-check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve 
-msve-vector-bits=512+ -x c -E -dM %s -o - 2>&1 | FileCheck 
-check-prefix=CHECK-NO-SVE-VECTOR-BITS %s
 // CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
-// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
 // CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_BITS
-// CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_VECTOR_OPERATORS
 
 // == Check Large System Extensions (LSE)
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+lse -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-LSE %s
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -489,9 +489,12 @@
   Builder.defineMacro("__FP_FAST_FMA", "1");
   Builder.defineMacro("__FP_FAST_FMAF", "1");
 
+  // C/C++ operators work on both VLS and VLA SVE types
+  if (FPU & SveMode)
+Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS", "2");
+
   if (Opts.VScaleMin && Opts.VScaleMin == Opts.VScaleMax) {
 Builder.defineMacro("__ARM_FEATURE_SVE_BITS", Twine(Opts.VScaleMin * 128));
-Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS");
   }
 }
 


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -150,6 +150,7 @@
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE: __ARM_FEATURE_SVE_VECTOR_OPERATORS 2
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve+bf16 -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-BF16 %s
 // CHECK-SVE-BF16: __ARM_FEATURE_BF16_SCALAR_ARITHMETIC 1
@@ -512,9 +513,7 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512+ -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-NO-SVE-VECTOR-BITS %s
 // CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
-// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
 // CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_BITS
-// CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_VECTOR_OPERATORS
 
 // == Check Large System Extensions (LSE)
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+lse -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-LSE %s
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -489,9 +489,12 @@
   Builder.defineMacro("__FP_FAST_FMA", "1");
   Builder.defineMacro("__FP_FAST_FMAF", "1");
 
+  // C/C++ operators work on both VLS and VLA SVE types
+  if (FPU & SveMode)
+Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS", "2");
+
   if (Opts.VScaleMin && Opts.VScaleMin == Opts.VScaleMax) {
 Builder.defineMacro("__ARM_FEATURE_SVE_BITS", Twine(Opts.VScaleMin * 128));
-Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS");
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 13a784f - [clang][AArch64][SVE] Change SVE_VECTOR_OPERATORS macro for VLA vectors

2022-08-11 Thread David Truby via cfe-commits

Author: David Truby
Date: 2022-08-11T13:23:52Z
New Revision: 13a784f368ef062a7bd652829bcf8bbdd94dc659

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

LOG: [clang][AArch64][SVE] Change SVE_VECTOR_OPERATORS macro for VLA vectors

The __ARM_FEATURE_SVE_VECTOR_OPERATORS macro should be changed to
indicate that this feature is now supported on VLA vectors as well as
VLS vectors. There is a complementary PR to the ACLE spec here
https://github.com/ARM-software/acle/pull/213

Reviewed By: peterwaller-arm

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/test/Preprocessor/aarch64-target-features.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 8612138c3194f..85346ebf92ab6 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -489,9 +489,12 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
&Opts,
   Builder.defineMacro("__FP_FAST_FMA", "1");
   Builder.defineMacro("__FP_FAST_FMAF", "1");
 
+  // C/C++ operators work on both VLS and VLA SVE types
+  if (FPU & SveMode)
+Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS", "2");
+
   if (Opts.VScaleMin && Opts.VScaleMin == Opts.VScaleMax) {
 Builder.defineMacro("__ARM_FEATURE_SVE_BITS", Twine(Opts.VScaleMin * 128));
-Builder.defineMacro("__ARM_FEATURE_SVE_VECTOR_OPERATORS");
   }
 }
 

diff  --git a/clang/test/Preprocessor/aarch64-target-features.c 
b/clang/test/Preprocessor/aarch64-target-features.c
index 6e495a0ae96e7..2d4ec2bbcae92 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -150,6 +150,7 @@
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE: __ARM_FEATURE_SVE_VECTOR_OPERATORS 2
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve+bf16 -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-SVE-BF16 %s
 // CHECK-SVE-BF16: __ARM_FEATURE_BF16_SCALAR_ARITHMETIC 1
@@ -512,9 +513,7 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve 
-msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck 
-check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve 
-msve-vector-bits=512+ -x c -E -dM %s -o - 2>&1 | FileCheck 
-check-prefix=CHECK-NO-SVE-VECTOR-BITS %s
 // CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
-// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
 // CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_BITS
-// CHECK-NO-SVE-VECTOR-BITS-NOT: __ARM_FEATURE_SVE_VECTOR_OPERATORS
 
 // == Check Large System Extensions (LSE)
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+lse -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-LSE %s



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


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment.

Noting related review: https://reviews.llvm.org/D115815 which added this 
variable to support this for other "external projects".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131665

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


[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D131528#3715217 , @mstorsjo wrote:

> There are still some cases that were broken by D131307 
> , that aren't fixed by this patch. Building 
> https://martin.st/temp/qt-enum.cpp with `clang -target i686-w64-mingw32 -c 
> -std=c++17 qt-enum.cpp -Wno-ignored-attributes -Wno-user-defined-literals` 
> succeeded before the change to make those errors downgradable, and those are 
> still an error now.

Diagnostic I'm getting is:

  :135567:18: error: integer value -1 is outside the valid range of 
values [0, 1] for this enumeration type [-Wenum-constexpr-conversion]
  if (order == Qt::SortOrder(-1))

SortOrder is:

   enum SortOrder {
  AscendingOrder,
  DescendingOrder
  };

So looks like at least the range diagnosed is correct.  Latest godbolt shows 
the issue:
https://godbolt.org/z/vKn57PbGf

BUT I believe this is exactly the case that this patch should have made no 
longer a problem.  Hopefully @shafik  can look into this to confirm, and see 
why this wasn't suppressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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


[clang] efc75a2 - Remove redundant condition check, NFC

2022-08-11 Thread Jun Zhang via cfe-commits

Author: Jun Zhang
Date: 2022-08-11T21:47:19+08:00
New Revision: efc75a2baedc7405193e3e0f5ea9aaa881783cec

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

LOG: Remove redundant condition check, NFC

Signed-off-by: Jun Zhang 

Added: 


Modified: 
clang/lib/Parse/ParseDecl.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 72d4804a1758e..39ba93ee33859 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2078,10 +2078,8 @@ Parser::DeclGroupPtrTy 
Parser::ParseDeclGroup(ParsingDeclSpec &DS,
   << (Fixit ? FixItHint::CreateInsertion(D.getBeginLoc(), "_Noreturn ")
 : FixItHint());
 }
-  }
 
-  // Check to see if we have a function *definition* which must have a body.
-  if (D.isFunctionDeclarator()) {
+// Check to see if we have a function *definition* which must have a body.
 if (Tok.is(tok::equal) && NextToken().is(tok::code_completion)) {
   cutOffParsing();
   Actions.CodeCompleteAfterFunctionEquals(D);



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


[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

2022-08-11 Thread Isuru Fernando via Phabricator via cfe-commits
isuruf added a comment.

As a downstream packager of libclang, I really like the SONAME not changing in 
libclang. It makes it possible for us to package things like qt that depend on 
libclang, but not have to rebuild qt for every major revision of LLVM.
It's not clear to me what the reason for removing this is. Can someone help me 
understand why this useful feature was removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129160

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 451838.
dongjunduo added a comment.

fix stringRef bug


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,9 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4736,6 +4736,79 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile = C.getArgs()
+   .getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  if (TimeTrace || TimeTraceFile) {
+SmallString<128> TracePath("");
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(SmallString<128>(
+  J.getOutputFilenames()[0].c_str( {
+  TracePath = llvm::sys::path::parent_path(
+  SmallString<128>(J.getOutputFilenames()[0].c_str()));
+} else {
+  TracePath = SmallString<128>(".");
+}
+  }
+}
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (TracePath.empty()){
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+llvm::sys::path::replace_extension(TracePath, "json");
+arg += std::string(TracePath.c_str());
+  }
+} else {
+  // /full_file_path_specified or /path_specified/yyy.json
+  TracePath = SmallString<128>(
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+  llvm::sys::path::replace_extension(TracePath, "json");
+  arg += std::string(TracePath.c_str());
+}
+
+const std::string::size_type size = arg.size();
+char *buffer = new char[size + 1];
+memcpy(buffer, arg.c_s

[PATCH] D131479: Handle explicitly defaulted consteval special members.

2022-08-11 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 451839.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.

Removed regex matching for diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -766,3 +766,102 @@
   static_assert(c == 8);
 }
 }
+
+namespace defaulted_special_member_template {
+template 
+struct default_ctor {
+  T data;
+  consteval default_ctor() = default; // expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+};
+
+template 
+struct copy {
+  T data;
+
+  consteval copy(const copy &) = default;// expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval copy &operator=(const copy &) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  copy() = default;
+};
+
+template 
+struct move {
+  T data;
+
+  consteval move(move &&) = default;// expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval move &operator=(move &&) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  move() = default;
+};
+
+struct foo {
+  foo() {}// expected-note {{declared here}}
+  foo(const foo &) {} // expected-note {{declared here}}
+  foo(foo &&) {}  // expected-note {{declared here}}
+
+  foo& operator=(const foo &) { return *this; } // expected-note {{declared here}}
+  foo& operator=(foo &&) { return *this; }  // expected-note {{declared here}}
+};
+
+void func() {
+  default_ctor fail0; // expected-error {{call to consteval function 'defaulted_special_member_template::default_ctor::default_ctor' is not a constant expression}} \
+  expected-note {{in call to 'default_ctor()'}}
+
+  copy good0;
+  copy fail1{good0}; // expected-error {{call to consteval function 'defaulted_special_member_template::copy::copy' is not a constant expression}} \
+ expected-note {{in call to 'copy(good0)'}}
+  fail1 = good0;  // expected-error {{call to consteval function 'defaulted_special_member_template::copy::operator=' is not a constant expression}} \
+ expected-note {{in call to '&fail1->operator=(good0)'}}
+
+  move good1;
+  move fail2{static_cast&&>(good1)}; // expected-error {{call to consteval function 'defaulted_special_member_template::move::move' is not a constant expression}} \
+   expected-note {{in call to 'move(good1)'}}
+  fail2 = static_cast&&>(good1);  // expected-error {{call to consteval function 'defaulted_special_member_template::move::operator=' is not a constant expression}} \
+   expected-note {{in call to '&fail2->operator=(good1)'}}
+}
+} // namespace defaulted_special_member_template
+
+namespace multiple_default_constructors {
+struct Foo {
+  Foo() {} // expected-note {{declared here}}
+};
+struct Bar {
+  Bar() = default;
+};
+struct Baz {
+  consteval Baz() {}
+};
+
+template 
+struct S {
+  T data;
+  S() requires (N==1) = default;
+  // This cannot be used in constexpr context.
+  S() requires (N==2) {}  // expected-note {{declared here}}
+  consteval S() requires (N==3) = default;  // expected-note {{non-constexpr constructor 'Foo' cannot be used in a constant expression}}
+};
+
+void func() {
+  // Explictly defaulted constructor.
+  S s1;
+  S s2;
+  // User provided constructor.
+  S s3;
+  S s4;
+  // Consteval explictly defaulted constructor.
+  S s5; // expected-error {{call to consteval function 'multiple_default_constructors::S::S' is not a constant expression}} \
+   expected-note {{in call to 'S()'}}
+  S s6;
+  S s7;
+}
+
+consteval int aConstevalFunction() { // expected-error {{consteval function never produces a constant expression}}
+  // Defaulted default constructors are implicitly consteval.
+  S s1;
+
+  S s4; // expected-note {{non-constexpr constructor 'S' cannot be used in a constant expression}}
+
+  S s2;
+  S s3;
+  return 0;
+}
+
+} // namespace multiple_default_constructors
Index: clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
===
--- clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
+++ clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
@@ -44,18 +44,20 @@
 }
 
 template struct S : T {
-  constexpr S() = default;
-  constexpr S(const S&) = default;
-  constexpr S(S&&) = d

[PATCH] D131657: Remove redundant condition check, NFC

2022-08-11 Thread Jun Zhang via Phabricator via cfe-commits
junaire added a comment.

In D131657#3715976 , @erichkeane 
wrote:

> LGTM!  Let us know if you need this committed for you.

Thanks! But I already have commit access :^)

Closed in efc75a2baedc7405193e3e0f5ea9aaa881783cec 
 (Sorry I 
accidentally missed the revesion info in the commit message)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131657

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-11 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo added a comment.

In D131469#3715929 , @jamieschmeiser 
wrote:

> You should not have debugging information in code that is up for review.  If 
> this is debugging information that you plan to leave in for future purposes 
> (which I doubt is the case here), you need to protect it so that it isn't 
> active unless some option is set.  For example, see LLVM_DEBUG code that 
> lives in various opt routines.

Got it. I want to use this unnecessary `print` to find why some tests passed in 
local but failed in CI. They will be removed soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D124434: [Clang][Test] Run tests in C++14 mode explicitly.

2022-08-11 Thread Jun Zhang via Phabricator via cfe-commits
junaire abandoned this revision.
junaire added a comment.
Herald added a subscriber: bzcheeseman.

Prefer https://reviews.llvm.org/D131464


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124434

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


[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

2022-08-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits
vladimir.plyashkun created this revision.
vladimir.plyashkun added reviewers: JonasToth, steveire, njames93.
vladimir.plyashkun added a project: clang-tools-extra.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
vladimir.plyashkun requested review of this revision.
Herald added a subscriber: cfe-commits.

Hello! 
Currently, the "hicpp/signed-bitwise" check returns the beginning of the 
binary/unary operator as location, which sometimes confuses users in the IDE 
due to incorrect highlighting 
.
 
Yes, the offset from Ranges can be used for this particular check, but i 
suppose better solution is to return begin location of the problematic operand 
instead of operator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131678

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise-integer-literals.cpp
  clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
@@ -42,7 +42,7 @@
   UResult = SValue & -1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UResult&= 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
 
   UResult = UValue & 1u; // Ok
   UResult = UValue & UValue; // Ok
@@ -63,43 +63,43 @@
 
   // More complex expressions.
   UResult = UValue & (SByte1 + (SByte1 | SByte2));
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
 
   // The rest is to demonstrate functionality but all operators are matched equally.
   // Therefore functionality is the same for all binary operations.
   UByte1 = UByte1 | UByte2; // Ok
   UByte1 = UByte1 | SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use of a signed integer operand with a binary bitwise operator
   UByte1|= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
   UByte1|= UByte2; // Ok
 
   UByte1 = UByte1 ^ UByte2; // Ok
   UByte1 = UByte1 ^ SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use of a signed integer operand with a binary bitwise operator
   UByte1^= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
   UByte1^= UByte2; // Ok
 
   UByte1 = UByte1 >> UByte2; // Ok
   UByte1 = UByte1 >> SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   UByte1>>= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UByte1>>= UByte2; // Ok
 
   UByte1 = UByte1 << UByte2; // Ok
   UByte1 = UByte1 << SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= UByte2; // Ok
 
   int SignedInt1 = 1 << 12;
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
   int SignedInt2 = 1u << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: 

[clang] 5e876c5 - [analyzer] exploded-graph-rewriter: Fix python3 string encoding issues

2022-08-11 Thread Balazs Benics via cfe-commits

Author: Balazs Benics
Date: 2022-08-11T16:07:52+02:00
New Revision: 5e876c54f2d70195dd1bde827e908825aab2f4fb

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

LOG: [analyzer] exploded-graph-rewriter: Fix python3 string encoding issues

This encapsulates 3 changes:
 - `DotDumpVisitor` now aggregates strings instead of *bytes* for both
   `python2` and `python3`. This difference caused crashes when it tried
to write out the content as *strings*, similarly described at D71746.
 - `graphviz.pipe()` expects the input in *bytes* instead of unicode
   strings. And it results in *bytes*. Due to string concatenations and
   similar operations, I'm using unicode string as the default, and
   converting to *bytes* on demand.
 - `write_temp_file()` now appends the `egraph-` prefix and more
   importantly, it will create the temp file in the **current working
   directory** instead of in the *temp*. This change makes `Firefox` be
   able to open the file even if the `security.sandbox.content.level` is
   set to the (default) most restricting `4`.
   See https://support.mozilla.org/si/questions/1259285

An artifact of the bad byte handling was previously in the `HTML`
produced by the script that it displayed the `b'` string at the top left
corner. Now it won't anymore :)

I've tested that the following command works on `Ubuntu 22.04`:
```
exploded-graph-rewriter my-egraph.dot
```
Both `python2` and `python3` works as expected.

PS: I'm not adding tests, as the current test infra does not support
testing HTML outputs for this script.
Check the `clang/test/Analysis/exploded-graph-rewriter/lit.local.cfg`.
We always pass the `--dump-dot-only` flag to the script.
Along with that, the default invocation will not only create this HTML
report but also try to open it. In addition to this, I'm not sure if the
buildbots have `graphviz` installed and also if this package is installed
on `pip`.
Unless we change some of these, we cannot test this change.
Given that D71746 had no tests, I'm not too worried about this either.

Reviewed By: NoQ

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

Added: 


Modified: 
clang/utils/analyzer/exploded-graph-rewriter.py

Removed: 




diff  --git a/clang/utils/analyzer/exploded-graph-rewriter.py 
b/clang/utils/analyzer/exploded-graph-rewriter.py
index b8382d151b86d..603bae9d69ac7 100755
--- a/clang/utils/analyzer/exploded-graph-rewriter.py
+++ b/clang/utils/analyzer/exploded-graph-rewriter.py
@@ -18,7 +18,6 @@
 import logging
 import os
 import re
-import sys
 
 
 #===---===#
@@ -423,10 +422,7 @@ def _dump_raw(self, s):
 
 def output(self):
 assert not self._dump_dot_only
-if sys.version_info[0] > 2 and sys.version_info[1] >= 5:
-return ''.join(self._output).encode()
-else:
-return ''.join(self._output)
+return ''.join(self._output)
 
 def _dump(self, s):
 s = s.replace('&', '&') \
@@ -854,8 +850,8 @@ def visit_end_of_graph(self):
 import sys
 import tempfile
 
-def write_temp_file(suffix, data):
-fd, filename = tempfile.mkstemp(suffix=suffix)
+def write_temp_file(suffix, prefix, data):
+fd, filename = tempfile.mkstemp(suffix, prefix, '.', True)
 print('Writing "%s"...' % filename)
 with os.fdopen(fd, 'w') as fp:
 fp.write(data)
@@ -873,13 +869,13 @@ def write_temp_file(suffix, data):
 print('You may also convert DOT to SVG manually via')
 print('  $ dot -Tsvg input.dot -o output.svg')
 print()
-write_temp_file('.dot', self.output())
+write_temp_file('.dot', 'egraph-', self.output())
 return
 
-svg = graphviz.pipe('dot', 'svg', self.output())
+svg = graphviz.pipe('dot', 'svg', self.output().encode()).decode()
 
 filename = write_temp_file(
-'.html', '%s' % (
+'.html', 'egraph-', '%s' % (
  '#1a1a1a' if self._dark_mode else 'white', svg))
 if sys.platform == 'win32':
 os.startfile(filename)



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


[PATCH] D131553: [analyzer] exploded-graph-rewriter: Fix python3 string encoding issues

2022-08-11 Thread Balázs Benics via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
steakhal marked an inline comment as done.
Closed by commit rG5e876c54f2d7: [analyzer] exploded-graph-rewriter: Fix 
python3 string encoding issues (authored by steakhal).

Changed prior to commit:
  https://reviews.llvm.org/D131553?vs=451401&id=451846#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131553

Files:
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -18,7 +18,6 @@
 import logging
 import os
 import re
-import sys
 
 
 #===---===#
@@ -423,10 +422,7 @@
 
 def output(self):
 assert not self._dump_dot_only
-if sys.version_info[0] > 2 and sys.version_info[1] >= 5:
-return ''.join(self._output).encode()
-else:
-return ''.join(self._output)
+return ''.join(self._output)
 
 def _dump(self, s):
 s = s.replace('&', '&') \
@@ -854,8 +850,8 @@
 import sys
 import tempfile
 
-def write_temp_file(suffix, data):
-fd, filename = tempfile.mkstemp(suffix=suffix)
+def write_temp_file(suffix, prefix, data):
+fd, filename = tempfile.mkstemp(suffix, prefix, '.', True)
 print('Writing "%s"...' % filename)
 with os.fdopen(fd, 'w') as fp:
 fp.write(data)
@@ -873,13 +869,13 @@
 print('You may also convert DOT to SVG manually via')
 print('  $ dot -Tsvg input.dot -o output.svg')
 print()
-write_temp_file('.dot', self.output())
+write_temp_file('.dot', 'egraph-', self.output())
 return
 
-svg = graphviz.pipe('dot', 'svg', self.output())
+svg = graphviz.pipe('dot', 'svg', self.output().encode()).decode()
 
 filename = write_temp_file(
-'.html', '%s' % (
+'.html', 'egraph-', '%s' % (
  '#1a1a1a' if self._dark_mode else 'white', svg))
 if sys.platform == 'win32':
 os.startfile(filename)


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -18,7 +18,6 @@
 import logging
 import os
 import re
-import sys
 
 
 #===---===#
@@ -423,10 +422,7 @@
 
 def output(self):
 assert not self._dump_dot_only
-if sys.version_info[0] > 2 and sys.version_info[1] >= 5:
-return ''.join(self._output).encode()
-else:
-return ''.join(self._output)
+return ''.join(self._output)
 
 def _dump(self, s):
 s = s.replace('&', '&') \
@@ -854,8 +850,8 @@
 import sys
 import tempfile
 
-def write_temp_file(suffix, data):
-fd, filename = tempfile.mkstemp(suffix=suffix)
+def write_temp_file(suffix, prefix, data):
+fd, filename = tempfile.mkstemp(suffix, prefix, '.', True)
 print('Writing "%s"...' % filename)
 with os.fdopen(fd, 'w') as fp:
 fp.write(data)
@@ -873,13 +869,13 @@
 print('You may also convert DOT to SVG manually via')
 print('  $ dot -Tsvg input.dot -o output.svg')
 print()
-write_temp_file('.dot', self.output())
+write_temp_file('.dot', 'egraph-', self.output())
 return
 
-svg = graphviz.pipe('dot', 'svg', self.output())
+svg = graphviz.pipe('dot', 'svg', self.output().encode()).decode()
 
 filename = write_temp_file(
-'.html', '%s' % (
+'.html', 'egraph-', '%s' % (
  '#1a1a1a' if self._dark_mode else 'white', svg))
 if sys.platform == 'win32':
 os.startfile(filename)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

2022-08-11 Thread Joey Watts via Phabricator via cfe-commits
joeywatts created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
joeywatts updated this revision to Diff 451689.
joeywatts added a comment.
Eugene.Zelenko retitled this revision from "(wip) improve modernize-use-emplace 
check" to "[clang-tidy][wip] improve modernize-use-emplace check".
Eugene.Zelenko added reviewers: alexfh, aaron.ballman, njames93, 
LegalizeAdulthood, JonasToth.
Eugene.Zelenko added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
joeywatts updated this revision to Diff 451842.
joeywatts retitled this revision from "[clang-tidy][wip] improve 
modernize-use-emplace check" to "[clang-tidy] Improve modernize-use-emplace 
check".
joeywatts edited the summary of this revision.
joeywatts updated this revision to Diff 451847.
joeywatts published this revision for review.
Herald added a subscriber: cfe-commits.

Update documentation


Eugene.Zelenko added a comment.

Please mention changes in Release Notes.


joeywatts added a comment.

Updated commit title/description.


joeywatts added a comment.

Add changes to release notes.


This patch improves the modernize-use-emplace check by adding support for
detecting inefficient invocations of the `push` and `push_front` methods on
STL-style containers and replacing them with their `emplace`-style equivalent.

Fixes #56996.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131623

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -62,6 +62,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
@@ -86,6 +89,9 @@
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   iterator emplace(const_iterator pos, Args &&...args){};
   template 
@@ -104,6 +110,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   void emplace_front(Args &&...args){};
   template 
@@ -235,6 +244,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -244,6 +256,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -253,6 +268,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -667,15 +685,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: l.emplace_back(42, 41);
 
+  l.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: l.emplace_front(42, 41);
+
   std::deque d;
   d.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: d.emplace_back(42);
 
+  d.push_front(Something(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: d.emplace_front(42);
+
   llvm::LikeASmallVector ls;
   ls.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
   // CHECK-FIXES: ls.emplace_back(42);
+
+  std::stack s;
+  s.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: s.emplace(42, 41);
+
+  std::queue q;
+  q.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: q.emplace(42, 41);
+
+  std::priority_queue pq;
+  pq.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace
+  // CHECK-FIXES: pq.emplace(42, 41);
+
+  std::forward_list fl;
+  fl.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front
+  // CHECK-FIXES: fl.emplace_front(42, 41);
 }
 
 class IntWrapper {
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
@@ -4,16 +4,22 @@
 =
 
 The check flags insertions to an STL-style container done by calling the
-``push_back

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-08-11 Thread David Rector via Phabricator via cfe-commits
davrec added a comment.

This part of the description is confusing:

> We take two types, X and Y, which we wish to unify as input.
> These types must have the same (qualified or unqualified) canonical
> type.
>
> We dive down fast through top-level type sugar nodes, to the
> underlying canonical node. If these canonical nodes differ, we
> build a common one out of the two, unifying any sugar they had.
> Note that this might involve a recursive call to unify any children
> of those. We then return that canonical node, handling any qualifiers.
>
> If they don't differ, we walk up the list of sugar type nodes we dived
> through, finding the last identical pair, and returning that as the
> result, again handling qualifiers.

The first paragraph says X and Y must have the same canonical type, the second 
suggests they need not.  In all the test cases, they have the same canonical 
type.

IIUC the second paragraph only applies to the stuff done in D130308 
; is that correct?

If so, the description should clearly state this patch only handles the case 
when the canonical types of X and Y are identical, and note that if the 
canonical types of X and Y differ, this patch does nothing, but D130308 
 adds some additional logic to search for 
common sugar in child type nodes of X and Y to use in the merged type.

And of course please add a full description to D130308 
 as well when you have a chance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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


[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

2022-08-11 Thread Joey Watts via Phabricator via cfe-commits
joeywatts updated this revision to Diff 451850.
joeywatts added a comment.

Update commit title/message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131623

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -62,6 +62,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
@@ -86,6 +89,9 @@
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   iterator emplace(const_iterator pos, Args &&...args){};
   template 
@@ -104,6 +110,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   void emplace_front(Args &&...args){};
   template 
@@ -235,6 +244,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -244,6 +256,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -253,6 +268,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -667,15 +685,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: l.emplace_back(42, 41);
 
+  l.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: l.emplace_front(42, 41);
+
   std::deque d;
   d.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: d.emplace_back(42);
 
+  d.push_front(Something(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: d.emplace_front(42);
+
   llvm::LikeASmallVector ls;
   ls.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
   // CHECK-FIXES: ls.emplace_back(42);
+
+  std::stack s;
+  s.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: s.emplace(42, 41);
+
+  std::queue q;
+  q.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: q.emplace(42, 41);
+
+  std::priority_queue pq;
+  pq.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace
+  // CHECK-FIXES: pq.emplace(42, 41);
+
+  std::forward_list fl;
+  fl.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front
+  // CHECK-FIXES: fl.emplace_front(42, 41);
 }
 
 class IntWrapper {
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
@@ -4,16 +4,22 @@
 =
 
 The check flags insertions to an STL-style container done by calling the
-``push_back`` method with an explicitly-constructed temporary of the container
-element type. In this case, the corresponding ``emplace_back`` method
-results in less verbose and potentially more efficient code.
-Right now the check doesn't support ``push_front`` and ``insert``.
-It also doesn't support ``insert`` functions for associative containers
-because replacing ``insert`` with ``emplace`` may result in
+``push_back``, ``push``, or ``push_front`` methods with an
+explicitly-constructed temporary of the container element type. In this case,
+the corresponding ``emplace`` equivalent methods result in less verbose and
+potentially more efficient code.  Right now the check doesn't support
+``insert``. It also doesn't support ``insert`` functions for associative
+containers because replacing ``insert`` with ``emplace`` may result in
 `speed regression `_, but it might get support with some addition flag in the future.
 
-By default only ``std::vector``, ``std::deque``, ``std::list`` are considered.
-This list can be modified using the :option:`ContainersWithPushBack` option.

  1   2   3   >