[PATCH] D131708: [RISCV] Change how mtune aliases are implemented.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: clang/test/Driver/riscv-cpus.c:27
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=rocket | FileCheck 
-check-prefix=MTUNE-ROCKET-64 %s
-// MTUNE-ROCKET-64: "-tune-cpu" "rocket-rv64"
-
-// RUN: %clang --target=riscv32 -### -c %s 2>&1 -mtune=sifive-7-series | 
FileCheck -check-prefix=MTUNE-SIFIVE7-SERIES-32 %s
-// MTUNE-SIFIVE7-SERIES-32: "-tune-cpu" "sifive-7-rv32"
-
-// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=sifive-7-series | 
FileCheck -check-prefix=MTUNE-SIFIVE7-SERIES-64 %s
-// MTUNE-SIFIVE7-SERIES-64: "-tune-cpu" "sifive-7-rv64"
+// MTUNE-ROCKET-64: "-tune-cpu" "rocket"
 

I got `error: unknown target CPU 'rocket'` (e.g. `clang --target=riscv64 
-mtune=rocket ~/hello.c`) after this patch, but no error if come with `-###`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131708

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

[PATCH] D131687: [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll:22
+; CHECK-NEXT:mov z4.d, z8.d
+; CHECK-NEXT:bl llvm.aarch64.vector.insert.nxv8f64.nx2f64
+; CHECK-NEXT:mov w0, #2

There is something going wrong here, because it should not have been lowered to 
a call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131687

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

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Thanks! With this, I think all my cases (that built before D131307 
) now build successfully again! (Can't say 
much about the code change itself, but I think it looks reasonable.)


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

https://reviews.llvm.org/D131874

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

[PATCH] D131879: [clang][analyzer] Errno modeling code refactor (NFC).

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

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

Some of the code used in StdLibraryFunctionsChecker is applicable to
other checkers, this is put into common functions. Errno related
parts of the checker are simplified and renamed. Documentations in
errno_modeling functions are updated.

This change makes it available to have more checkers that perform
modeling of some standard functions. These can set the errno state
with common functions and the bug report messages (note tags) can
look similar.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131879

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -57,19 +57,6 @@
 using namespace clang;
 using namespace clang::ento;
 
-/// Produce a textual description of the state of \c errno (this describes the
-/// way how it is allowed to be used).
-/// The returned string is insertable into a longer warning message (in the form
-/// "the value 'errno' <...>").
-/// Currently only the \c errno_modeling::MustNotBeChecked state is supported.
-/// But later other kind of errno state may be needed if functions with special
-/// handling of \c errno are added.
-static const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS) {
-  assert(CS == errno_modeling::MustNotBeChecked &&
- "Errno description not applicable.");
-  return "may be undefined after the call and should not be used";
-}
-
 namespace {
 class StdLibraryFunctionsChecker
 : public Checker {
@@ -392,45 +379,42 @@
   using ConstraintSet = std::vector;
 
   /// Define how a function affects the system variable 'errno'.
-  /// This works together with the ErrnoModeling and ErrnoChecker classes.
+  /// This works together with the \c ErrnoModeling and \c ErrnoChecker classes.
+  /// Currently 3 use cases exist: success, failure, irrelevant.
+  /// In the future the failure case can be customized to set \c errno to a
+  /// more specific constraint (for example > 0), or new case can be added
+  /// for functions which require check of \c errno in both success and failure
+  /// case.
   class ErrnoConstraintBase {
   public:
 /// Apply specific state changes related to the errno variable.
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const = 0;
-/// Get a description about what is applied to 'errno' and how is it allowed
-/// to be used. If ErrnoChecker generates a bug then this message is
-/// displayed as a note at the function call.
-/// It may return empty string if no note tag is to be added.
-virtual std::string describe(StringRef FunctionName) const { return ""; }
+/// Get a NoteTag about the changes made to 'errno' and the possible bug.
+/// It may return \c nullptr (if no bug report from \c ErrnoChecker is
+/// expected).
+virtual const NoteTag *describe(CheckerContext &C,
+StringRef FunctionName) const {
+  return nullptr;
+}
 
 virtual ~ErrnoConstraintBase() {}
 
   protected:
-/// Many of the descendant classes use this value.
-const errno_modeling::ErrnoCheckState CheckState;
-
-ErrnoConstraintBase(errno_modeling::ErrnoCheckState CS) : CheckState(CS) {}
+ErrnoConstraintBase() {}
 
 /// This is used for conjure symbol for errno to differentiate from the
 /// original call expression (same expression is used for the errno symbol).
 static int Tag;
   };
 
-  /// Set value of 'errno' to be related to 0 in a specified way, with a
-  /// specified "errno check state". For example with \c BO_GT 'errno' is
-  /// constrained to be greater than 0. Use this for failure cases of functions.
-  class ZeroRelatedErrnoConstraint : public ErrnoConstraintBase {
-BinaryOperatorKind Op;
-
+  /// Set errno constraint at failure cases of standard functions.
+  /// Failure case: 'errno' becomes not equal to 0 and may or may not be checked
+  /// by the program. \c ErrnoChecker does not emit a bug report after such a
+  /// function call.
+  class FailureErrnoConstraint : public ErrnoConstraintBase {
   public:
-ZeroRelatedErrnoConstraint(cla

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

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun updated this revision to Diff 452605.
vladimir.plyashkun added a comment.

- Executed clang-format after changes
- Return operator location (in additional range)


Repository:
  rG LLVM Github Monorepo

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

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: :[[@LINE-1]]:26: warning: use of a signed integer operand with a binary bitwise operator
 }
 
 void f1(unsigned char c) {}
@@ -113,28 +113,28 @@
   UByte1 = ~UByte1; // Ok
   SByte1 = ~UByte1;
   SByte1 = ~SByte1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
   UByte1 = ~SByte1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  //

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 452610.
ChuanqiXu marked an inline comment as done.
ChuanqiXu added a comment.

Address comments.


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,779 @@
+=
+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``.
+
+Built Module Interface file
+~~~
+
+A ``Built Module Interface file`` stands for the precompiled result of an importable module unit.
+It is also called the acronym ``BMI`` genrally.
+
+Global module fragment
+~~

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 452612.
ChuanqiXu added a comment.

Address comments.


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,779 @@
+=
+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``.
+
+Built Module Interface file
+~~~
+
+A ``Built Module Interface file`` stands for the precompiled result of an importable module unit.
+It is also called the acronym ``BMI`` genrally.
+
+Global module fragment
+~~
+
+In a module unit, the section from ``mod

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D131388#3719794 , @dblaikie wrote:

> Actually, when it comes to diagrams - maybe what'd be good is a diagram of 
> classic compilation and a diagram of modules and header modules
>
>   src1.cpp -+> clang++ src1.cpp --> src1.o ---, 
>   hdr1.h  --' +-> clang++ src1.o src2.o ->  
> executable
>   hdr2.h  --, |
>   src2.cpp -+> clang++ src2.cpp --> src2.o ---'
>
> src1.cpp +> clang++ 
> src1.cpp ---> src1.o -, 
>   (header unit) hdr1.h-> clang++ hdr1.h ...-> hdr1.pcm --'
> +-> clang++ src1.o mod1.o src2.o ->  executable
> mod1.cppm -> clang++ mod1.cppm ... -> mod1.pcm --,--> clang++ 
> mod1.pcm ... -> mod1.o -+
> src2.cpp +> clang++ 
> src2.cpp ---> src2.o -'   
>
> (no doubt could look a lot better - but would pretty quickly summarize what's 
> going where - maybe the command lines are too long to include nicely in such 
> a diagram and so the diagram could be a more simplified block diagram and the 
> full command lines shown separately)

Thanks for the diagrams! It is helpful. I added a sentence:  `(But we can't do 
this for the BMI from header units. See the later section for the definition of 
header units)` since I'm not sure if all readers are familiar with header units.




Comment at: clang/docs/CPlusPlus20Modules.rst:309
+
+Remember to compile and link BMIs
+~

ruoso wrote:
> I think this is a bit confusing. The BMI is not linked...
> 
> Maybe something like: "Remember that modules still have an object counterpart 
> to the BMI". 
> 
> Specifically, it may be important to make a distinction between when the 
> compiler is invoked to produce the bmi and when it's invoked to produce the 
> object, and that you use the bmi when translating code that imports it, and 
> the object when linking.
> 
> Specifically, it may be important to make a distinction between when the 
> compiler is invoked to produce the bmi and when it's invoked to produce the 
> object, and that you use the bmi when translating code that imports it, and 
> the object when linking.

I don't get the meaning here. The doc said the user need to compile `*.cppm` to 
`*.pcm` before imported it and the doc also said the user need to compile 
`*.pcm` to `*.o`  to link. So I guess  the doc has stated it clear?



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:
> > > ChuanqiXu wrote:
> > > > 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 i

[PATCH] D131872: [Intrinsics] Add initial support for NonNull attribute

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1415
 def int_threadlocal_address : DefaultAttrsIntrinsic<[llvm_anyptr_ty], 
[LLVMMatchType<0>],
-   [IntrNoMem, IntrSpeculatable, IntrWillReturn]>;
+   [NonNull, NonNull>, 
IntrNoMem, IntrSpeculatable, IntrWillReturn]>;
 





Comment at: llvm/utils/TableGen/IntrinsicEmitter.cpp:710-711
 unsigned AttrIdx = Intrinsic.ArgumentAttributes[Ai].Index;
-
+if (AttrIdx == 0)
+  OS << "  // AttrParam0 corresponds to return value.\n";
 OS << "  const Attribute::AttrKind AttrParam" << AttrIdx << "[]= 
{";

Is this necessary? BTW, the comment specifier may be `;` instead of `//`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872

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

[PATCH] D131872: [Intrinsics] Add initial support for NonNull attribute

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 452615.
alexander-shaposhnikov added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872

Files:
  clang/test/CodeGenCXX/threadlocal_address.cpp
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86.mir
  llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86_64.mir
  llvm/test/CodeGen/X86/threadlocal_address.ll
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -700,11 +700,13 @@
 unsigned numAttrs = 0;
 
 // The argument attributes are alreadys sorted by argument index.
+assert(is_sorted(Intrinsic.ArgumentAttributes) &&
+   "Argument attributes are not sorted");
+
 unsigned Ai = 0, Ae = Intrinsic.ArgumentAttributes.size();
 if (Ae) {
   while (Ai != Ae) {
 unsigned AttrIdx = Intrinsic.ArgumentAttributes[Ai].Index;
-
 OS << "  const Attribute::AttrKind AttrParam" << AttrIdx << "[]= {";
 ListSeparator LS(",");
 
@@ -721,6 +723,9 @@
   case CodeGenIntrinsic::NoUndef:
 OS << LS << "Attribute::NoUndef";
 break;
+  case CodeGenIntrinsic::NonNull:
+OS << LS << "Attribute::NonNull";
+break;
   case CodeGenIntrinsic::Returned:
 OS << LS << "Attribute::Returned";
 break;
@@ -756,7 +761,8 @@
 OS << LSV << V;
   OS << "};\n";
 }
-
+// AttributeList::ReturnIndex = 0, AttrParam0 corresponds to return
+// value.
 OS << "  AS[" << numAttrs++ << "] = AttributeList::get(C, "
<< AttrIdx << ", AttrParam" << AttrIdx;
 if (!AllValuesAreZero)
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -893,6 +893,9 @@
   } else if (R->isSubClassOf("NoUndef")) {
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 ArgumentAttributes.emplace_back(ArgNo, NoUndef, 0);
+  } else if (R->isSubClassOf("NonNull")) {
+unsigned ArgNo = R->getValueAsInt("ArgNo");
+ArgumentAttributes.emplace_back(ArgNo, NonNull, 0);
   } else if (R->isSubClassOf("Returned")) {
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 ArgumentAttributes.emplace_back(ArgNo, Returned, 0);
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -154,6 +154,7 @@
 NoCapture,
 NoAlias,
 NoUndef,
+NonNull,
 Returned,
 ReadOnly,
 WriteOnly,
Index: llvm/test/CodeGen/X86/threadlocal_address.ll
===
--- llvm/test/CodeGen/X86/threadlocal_address.ll
+++ llvm/test/CodeGen/X86/threadlocal_address.ll
@@ -37,5 +37,5 @@
   ret i32 %3
 }
 
-declare ptr @llvm.threadlocal.address(ptr) nounwind readnone willreturn
-declare ptr addrspace(1) @llvm.threadlocal.address.p1(ptr addrspace(1)) nounwind readnone willreturn
+declare nonnull ptr @llvm.threadlocal.address(ptr nonnull) nounwind readnone willreturn
+declare nonnull ptr addrspace(1) @llvm.threadlocal.address.p1(ptr addrspace(1) nonnull) nounwind readnone willreturn
Index: llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86_64.mir
===
--- llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86_64.mir
+++ llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86_64.mir
@@ -23,7 +23,7 @@
   }
 
   ; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
-  declare ptr @llvm.threadlocal.address.p0(ptr) #0
+  declare nonnull ptr @llvm.threadlocal.address.p0(ptr nonnull) #0
 
   attributes #0 = { nocallback nofree nosync nounwind readnone speculatable willreturn }
 
Index: llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86.mir
===
--- llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86.mir
+++ llvm/test/CodeGen/X86/peephole-nofold-tpoff-x86.mir
@@ -33,7 +33,7 @@
   }
 
   ; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
-  declare ptr @llvm.threadlocal.address.p0(ptr) #0
+  declare nonnull ptr @llvm.threadlocal.address.p0(ptr nonnull) #0
 
   attributes #0 = { nocallback nofree nosync nounwind readnone speculatable willreturn }
 
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/ll

[PATCH] D131872: [Intrinsics] Add initial support for NonNull attribute

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/CodeGenCXX/threadlocal_address.cpp:27
 // CHECK-O1-NEXT: entry:
-// CHECK-O1-NEXT:   %[[I_ADDR:.+]] = {{.*}}call ptr 
@llvm.threadlocal.address.p0(ptr nonnull @i)
+// CHECK-O1-NEXT:   %[[I_ADDR:.+]] = {{.*}}call ptr 
@llvm.threadlocal.address.p0(ptr @i)
 // CHECK-O1-NEXT:   %[[VAL:.+]] = load i32, ptr %[[I_ADDR]]

Why do we move the `nonnull` specifier here? And it looks like we need a 
nonnull attribute for the return value. (Same for other tests.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872

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

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

[Alexander Malkov via Phabricator via cfe-commits]

alexiprof added a comment.

I can't merge the changes((


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131808

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

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

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 452617.
iains added a comment.

rebased, addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/Reachability-Private.cpp
  clang/test/Modules/cxx20-10-5-ex1.cpp

Index: clang/test/Modules/cxx20-10-5-ex1.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-5-ex1.cpp
@@ -0,0 +1,53 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-5-ex1-interface.cpp \
+// RUN: -DBAD_FWD_DECL  -fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-5-ex1-interface.cpp \
+// RUN: -o A.pcm
+
+// RUN: %clang_cc1 -std=c++20 std-10-5-ex1-use.cpp  -fmodule-file=A.pcm \
+// RUN:-fsyntax-only -verify
+
+//--- std-10-5-ex1-interface.cpp
+
+export module A;
+#ifdef BAD_FWD_DECL
+export inline void fn_e(); // expected-error {{inline function not defined before the private module fragment}}
+   // expected-n...@std-10-5-ex1-interface.cpp:21 {{private module fragment begins here}}
+#endif
+export inline void ok_fn() {}
+export inline void ok_fn2();
+#ifdef BAD_FWD_DECL
+inline void fn_m(); // expected-error {{inline function not defined before the private module fragment}}
+// expected-n...@std-10-5-ex1-interface.cpp:21 {{private module fragment begins here}}
+#endif
+static void fn_s();
+export struct X;
+export void g(X *x) {
+  fn_s();
+}
+export X *factory();
+void ok_fn2() {}
+
+module :private;
+struct X {};
+X *factory() {
+  return new X();
+}
+
+void fn_e() {}
+void fn_m() {}
+void fn_s() {}
+
+//--- std-10-5-ex1-use.cpp
+
+import A;
+
+void foo() {
+  X x; // expected-error 1+{{missing '#include'; 'X' must be defined before it is used}}
+   // expected-n...@std-10-5-ex1-interface.cpp:22 1+{{definition here is not reachable}}
+  X *p = factory();
+}
Index: clang/test/Modules/Reachability-Private.cpp
===
--- clang/test/Modules/Reachability-Private.cpp
+++ clang/test/Modules/Reachability-Private.cpp
@@ -4,18 +4,25 @@
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
 //
-// RUN: %clang_cc1 -std=c++20 %t/Private.cppm -emit-module-interface -o %t/Private.pcm
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/Private.cppm -emit-module-interface \
+// RUN: -o %t/Private.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp \
+// RUN: -DTEST_BADINLINE -verify -fsyntax-only
 
 //--- Private.cppm
 export module Private;
-inline void fn_m(); // OK, module-linkage inline function
+#ifdef TEST_BADINLINE
+inline void fn_m(); // expected-error {{un-exported inline function not defined before the private module fragment}}
+// expected-n...@private.cppm:13 {{private module fragment begins here}}
+#endif
 static void fn_s();
 export struct X;
 
 export void g(X *x) {
   fn_s(); // OK, call to static function in same translation unit
-  fn_m(); // OK, call to module-linkage inline function
+#ifdef TEST_BADINLINE
+  fn_m(); // fn_m is not OK.
+#endif
 }
 export X *factory(); // OK
 
@@ -30,10 +37,8 @@
 //--- Use.cpp
 import Private;
 void foo() {
-  X x; // expected-error {{definition of 'X' must be imported from module 'Private.' before it is required}}
-   // expected-error@-1 {{definition of 'X' must be imported from module 'Private.' before it is required}}
-   // expected-note@* {{definition here is not reachable}}
-   // expected-note@* {{definition here is not reachable}}
+  X x; // expected-error 1+{{missing '#include'; 'X' must be defined before it is used}}
+   // expected-n...@private.cppm:18 1+{{definition here is not reachable}}
   auto _ = factory();
   auto *__ = factory();
   X *___ = factory();
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -910,6 +910,17 @@
 diagExportedUnnamedDecl(*this, UnnamedDeclKind::Context, Child,
 BlockStart);
   }
+  if (auto *FD = dyn_cast(Child)) {
+// [dcl.inline]/7
+// If an inline function or variable that is attached to a named module
+// is declared in a definition domain, it shall be defined in that
+// domain.
+// So, if the current declaration does not have a definition, we must
+// check at the end of the TU (or when the PMF starts) to see that we
+// have a definition at that point.
+

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

[Iain Sandoe via Phabricator via cfe-commits]

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



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

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > iains wrote:
> > > > > ChuanqiXu wrote:
> > > > > > iains wrote:
> > > > > > > iains wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > From my reading, 'exported' is not emphasized.
> > > > > > > > it is here:
> > > > > > > > https://eel.is/c++draft/module#private.frag-2.1
> > > > > > > > ( I agree it is somewhat confusing, but the export makes the 
> > > > > > > > linkage external, which the example treats differently from the 
> > > > > > > > fn_m() case which has module linkage).
> > > > > > > > 
> > > > > > > > It is possible that we might need to pull together several 
> > > > > > > > pieces of the std and maybe ask core for clarification?
> > > > > > > > it is here:
> > > > > > > > https://eel.is/c++draft/module#private.frag-2.1
> > > > > > > > ( I agree it is somewhat confusing, but the export makes the 
> > > > > > > > linkage external, which the example treats differently from the 
> > > > > > > > fn_m() case which has module linkage).
> > > > > > > 
> > > > > > > hmm... my linkage comment is wrong - however the distinction 
> > > > > > > between exported and odr-used seems to be made here (fn_m() and 
> > > > > > > fn_e()).
> > > > > > > > 
> > > > > > > > It is possible that we might need to pull together several 
> > > > > > > > pieces of the std and maybe ask core for clarification?
> > > > > > > 
> > > > > > > 
> > > > > > What I read is:
> > > > > > > [dcl.inline]p7: https://eel.is/c++draft/dcl.inline#7
> > > > > > > If an inline function or variable that is attached to a named 
> > > > > > > module is declared in a definition domain, it shall be defined in 
> > > > > > > that domain.
> > > > > > 
> > > > > > and the definition of `definition domain` is:
> > > > > > > [basic.def.odr]p12: https://eel.is/c++draft/basic#def.odr-12
> > > > > > >   A definition domain is a private-module-fragment or the 
> > > > > > > portion of a translation unit excluding its 
> > > > > > > private-module-fragment (if any).
> > > > > > 
> > > > > > The definition of "attached to a named module" is:
> > > > > > > [module.unit]p7: https://eel.is/c++draft/module.unit#7
> > > > > > >  A module is either a named module or the global module. A 
> > > > > > > declaration is attached to a module as follows: ...
> > > > > > 
> > > > > > So it is clearly not consistency with [module.private.frag]p2.1. I 
> > > > > > would send this to WG21.
> > > > > Yes, that was what I found - maybe we are  missing something about 
> > > > > the export that changes those rules.
> > > > > .
> > > > I think that we can consider this closed by the question to the ext 
> > > > reflector and the amendment proposed by core.
> > > I prefer `inline function attached to a named module not defined 
> > > %select{| before the private module fragment}1`. Since the `export` part 
> > > is not important here and the important part is whether or not they are 
> > > attached to a named module.
> > I took the error message from here:
> > https://github.com/cplusplus/draft/pull/5537
> > which was prepared after the dicussion that you started on the ext 
> > reflector.  Actually, I do not have a strong feeling either way.
> Oh, I didn't track that. I feel my suggested version is slightly better. 
> Otherwise users might want to make it work by adding/removing `export` clause.
I removed the reference to un/exported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

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

[PATCH] D131687: [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret

[Caroline via Phabricator via cfe-commits]

CarolineConcatto added inline comments.



Comment at: llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll:22
+; CHECK-NEXT:mov z4.d, z8.d
+; CHECK-NEXT:bl llvm.aarch64.vector.insert.nxv8f64.nx2f64
+; CHECK-NEXT:mov w0, #2

sdesmalen wrote:
> There is something going wrong here, because it should not have been lowered 
> to a call.
Thank you Sander.
Good spot. I left aarch64 in the intrinsic name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131687

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D131687: [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll:127
 ; CHECK-NEXT:ptrue p0.d
-; CHECK-NEXT:st1d { z16.d }, p0, [sp]
-; CHECK-NEXT:st1d { z17.d }, p0, [sp, #1, mul vl]
-; CHECK-NEXT:st1d { z18.d }, p0, [sp, #2, mul vl]
+; CHECK-NEXT:st1d { z2.d }, p0, [sp]
+; CHECK-NEXT:st1d { z2.d }, p0, [sp, #1, mul vl]

I would have expected no asm changes. The test is not equivalent to the 
previous code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131687

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

[PATCH] D131879: [clang][analyzer] Errno modeling code refactor (NFC).

[Balázs Benics via Phabricator via cfe-commits]

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

Approved with nits. Thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:267
 
+const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS) {
+  assert(CS == errno_modeling::MustNotBeChecked &&

The same applies for these.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:298
+  if (!State)
+return State;
+  return errno_modeling::setErrnoValue(State, C.getLocationContext(), ErrnoSym,

I would prefer returning `nullptr` explicitly here.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:304-307
+  std::string Message = "Assuming that function '";
+  Message += Fn;
+  Message += "' is successful, in this case the value 'errno' ";
+  Message += describeErrnoCheckState(MustNotBeChecked);

I believe you should use `Twine()` for chaining string slices.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:83
+/// others are not used by the clients.
+const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS);
+

It seems like you are already within this namespace. You don't need to 
explicitly qualify this.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:405
   protected:
-/// Many of the descendant classes use this value.
-const errno_modeling::ErrnoCheckState CheckState;
-
-ErrnoConstraintBase(errno_modeling::ErrnoCheckState CS) : CheckState(CS) {}
+ErrnoConstraintBase() {}
 

Why don't you `= default` it if the body is actually empty?



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:416
+  /// function call.
+  class FailureErrnoConstraint : public ErrnoConstraintBase {
   public:

I find this name much more appealing than it was before! Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131879

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

[PATCH] D131526: [OMPIRBuilder] Add support for safelen clause

[Shraiysh via Phabricator via cfe-commits]

shraiysh added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:625
+  void applySimd(CanonicalLoopInfo *Loop, Value *IfCond, ConstantInt *Simdlen,
+ ConstantInt *Safelen);
 

psoni2628 wrote:
> shraiysh wrote:
> > [nit] Please set the default value of Safelen to nullptr here.
> Setting a default value for only `Safelen` but not `Simdlen` or `IfCond` is a 
> bit inconsistent and could potentially cause confusion in the future. I think 
> it would make more sense to set default values for all the clause values. 
> Maybe we could this separately in a different patch?
Sounds good to me.


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

https://reviews.llvm.org/D131526

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

We leave formatting decisions in clang-tidy to clang-format and I don't think 
we should deviate from that policy here without a very clear understanding of 
when we should relax that restriction. That said, I'm personally not certain we 
should have such an option (the long-term goal has generally been to integrate 
clang-format functionality into clang-tidy so there can be an option to just 
run format after applying fixes in a TU). Is there a compelling reason we 
should have it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D3976: -Wcomma, a new warning for questionable uses of the comma operator

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D3976#3721421 , @gpakosz wrote:

> Hello,
>
> As of today, `-Wcomma` is not really documented: 
> https://clang.llvm.org/docs/DiagnosticsReference.html#wcomma
>
>> The current whitelisted expressions are increments, decrements, assignments, 
>> compound assignments, overloaded versions of these operators, **and void 
>> returning functions**

FWIW, none of our diagnostics get documentation beyond what you see there (it's 
auto-generated from the diagnostic definition file).

> About `void` returning functions, the following example 
>  emits `-Wcomma`:
>
>   void foo()
>   {
>   
>   }
>   
>   int bar(int i)
>   {
> return i;
>   }
>   
>   int main(int argc, char* argv[])
>   {
> if (foo(), bar(argc))
>   return 0;
>   
> return -1;
>   }
>
>
>
>   :13:12: warning: possible misuse of comma operator here [-Wcomma]
> if (foo(), bar(argc))
>  ^
>   :13:7: note: cast expression to void to silence warning
> if (foo(), bar(argc))
> ^
> (void)( )
>   1 warning generated.
>
> Should I open a bug?

I think opening a bug is appropriate, good catch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D3976

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

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, only nits left that can be fixed when landing (no need for additional 
review).




Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6560
+.. code-block:: c++
+  RWBuffer Uav : register(u3, space1);
+  Buffer Buf : register(t1);
+The full documentation is available here: 
https://docs.microsoft.com/en-us/windows/win32/direct3d12/resource-binding-in-hlsl

RST is very picky about that whitespace.



Comment at: clang/test/SemaHLSL/resource_binding_attr_error.hlsl:15
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' 
followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+

python3kgae wrote:
> aaron.ballman wrote:
> > Isn't this a re-definition of `a` which would cause an error?
> The name for cbuffer is not used.
> And no error should be reported for re-definition.
> 
> Discussed with the team, have to keep it like this for back-compat reason.
Wow.

Please either add a comment explaining why this bizarre situation gets no 
diagnostic, or change the identifiers used so nobody else gets confused by this 
when reading the test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

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

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Please measure the performance implications/gains of this patch to justify this 
change.
Preferably test it on multiple kinds of projects ranging from old-school C and 
even some modern C++ projects.
Let me know your findings and if you need some help setting it up.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:549-550
+  CachedLocationType = T->isObjCObjectType() ? 
C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+}
+return CachedLocationType;

I would appreciate it if you assert that the resulting type cannot be `null`, 
which would imply that if `CachedLocationType.isNull()` means that we haven't 
cached it yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131707

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

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13069
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;

ShawnZhong wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > ShawnZhong wrote:
> > > > ShawnZhong wrote:
> > > > > ShawnZhong wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > thakis wrote:
> > > > > > > > This was to suppress false positives. All instances we've seen 
> > > > > > > > of this are in fact false positives.
> > > > > > > > 
> > > > > > > > Was there any analysis on true / false positives for this 
> > > > > > > > change?
> > > > > > > > 
> > > > > > > > At least for our code, this seems to make the warning strictly 
> > > > > > > > worse.
> > > > > > > I've not performed any such analysis, but it would be good to 
> > > > > > > know. FWIW, this is the kind of situation I was thinking this 
> > > > > > > diagnostic would help make far more clear: 
> > > > > > > https://godbolt.org/z/336n9xex3 (not that I expect people to 
> > > > > > > write that static assert, but I very much expect people who 
> > > > > > > assign the value 1 into a bit-field and then check for the value 
> > > > > > > 1 to come back out are going to be surprised).
> > > > > > > 
> > > > > > > That said, another approach to that specific scenario, which 
> > > > > > > might have a better true positive rate would be:
> > > > > > > ```
> > > > > > > struct S {
> > > > > > >   int bit : 1;
> > > > > > > };
> > > > > > > 
> > > > > > > int main() {
> > > > > > >   constexpr S s{1}; // No warning
> > > > > > >   if (s.bit == 1) { // Warn about explicit comparison of a 1-bit 
> > > > > > > bit-field
> > > > > > > ...
> > > > > > >   } else if (s.bit == 0) { // Warn about explicit comparison of a 
> > > > > > > 1-bit bit-field?
> > > > > > > ...
> > > > > > >   } else if (s.bit) { // No warning
> > > > > > > ...
> > > > > > >   } else if (!s.bit) { // No warning
> > > > > > > ...
> > > > > > >   }
> > > > > > > }
> > > > > > > ```
> > > > > > Do you have something in mind that counts as false positives? 
> > > > > BTW, I realized that no warning is reported when a bool is assigned 
> > > > > to a single-bit signed bit-field:
> > > > > 
> > > > > https://godbolt.org/z/M5ex48T8s
> > > > > 
> > > > > ```
> > > > > int main() {
> > > > >   struct S { int b : 1; } s;
> > > > >   s.b = true;
> > > > >   if (s.b == true) { puts("T"); } else { puts("F"); }
> > > > > }
> > > > > ```
> > > > > 
> > > > > 
> > > > For reference, I found this issue on chromium: 
> > > > 
> > > > https://bugs.chromium.org/p/chromium/issues/detail?id=1352183
> > > > Do you have something in mind that counts as false positives?
> > > 
> > > In my example above, I consider the use of `s.bit` and `!s.bit` to count 
> > > as false positives. The value in the bit-field being 1 or -1 has no 
> > > bearing on the semantics of the program, the only thing that matters is 
> > > zero vs nonzero because of the boolean conversion.
> > > 
> > > > BTW, I realized that no warning is reported when a bool is assigned to 
> > > > a single-bit signed bit-field:
> > > 
> > > Good catch, the conversion from bool to integer does give the value `1` 
> > > explicitly. At the same time, I would consider the assignment to the 
> > > bit-field from a boolean to be a non-issue, the problem only stems from 
> > > attempting to use inspect the value.
> > >>BTW, I realized that no warning is reported when a bool is assigned to a 
> > >>single-bit signed bit-field:
> > > Good catch, the conversion from bool to integer does give the value 1 
> > > explicitly. At the same time, I would consider the assignment to the 
> > > bit-field from a boolean to be a non-issue, the problem only stems from 
> > > attempting to use inspect the value.
> > 
> > Actually, that's a more interesting case than I had originally realized. 
> > See C2x 6.7.2.1p12. "If the value 0 or 1 is stored into a nonzero-width 
> > bit-field of type bool, the value of the bit-field shall compare equal to 
> > the value stored; a bool bit-field has the semantics of a bool."
> > 
> > So if you store a 1 into a bool bit-field, you have to get a 1 back out of 
> > it. The bit-field is implicitly unsigned, effectively.
> > In my example above, I consider the use of `s.bit` and `!s.bit` to count as 
> > false positives. The value in the bit-field being 1 or -1 has no bearing on 
> > the semantics of the program, the only thing that matters is zero vs 
> > nonzero because of the boolean conversion.
> 
> I agree with you that if `s.bit` is only used to be converted to `bool`, then 
> the value stored being 1 or -1 does not matter that much. But hypothetically, 
> one could use `s.bit` for arithmetic (e.g., index into an array), assignment 
> (say into another int or enum), or, in your example, comparison against 
> another value.  If a warning is not generated d

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

[Alexander Malkov via Phabricator via cfe-commits]

alexiprof added a comment.

Could someone land this patch please?
Alexander Malkov 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131808

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Nathan James via Phabricator via cfe-commits]

njames93 added a reviewer: Mordante.
njames93 added a comment.

In D131386#3722749 , @aaron.ballman 
wrote:

> We leave formatting decisions in clang-tidy to clang-format and I don't think 
> we should deviate from that policy here without a very clear understanding of 
> when we should relax that restriction. That said, I'm personally not certain 
> we should have such an option (the long-term goal has generally been to 
> integrate clang-format functionality into clang-tidy so there can be an 
> option to just run format after applying fixes in a TU). Is there a 
> compelling reason we should have it?

The reason for this is due to the issue that `QualifierAlignment`  is a non 
whitespace only change and clang-format lists that using it could break some 
code.
In light of this some users may wish to set the option to `QAS_Leave` to be 
sure no code is broken even though they would prefer a specific style. 
Therefore having a dedicated option in the check will let those users specify 
the style, without having to set a clang-format configuration which they aren't 
content in using.




Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp:66
+  ConstAlignment(
+  Options.get("ConstAlignment", utils::fixit::QualifierPolicy::Right)) 
{
   if (AnalyzeValues == false && AnalyzeReferences == false)

Mordante wrote:
> I would suggest to use `QualifierAlignment` to match the name in clang-format.
I thought about that, but the clang-format option doesn't just align the const 
qualifier. It works for all qualifiers.
I am easy on what name we use for the option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Nathan James via Phabricator via cfe-commits]

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

Remove unneeded include and add Release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

Files:
  clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-alignment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-alignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-alignment.cpp
@@ -0,0 +1,46 @@
+// RUN: %check_clang_tidy %s misc-const-correctness %t -check-suffixes=",WEST" \
+// RUN:   -config="{CheckOptions: { \
+// RUN:   misc-const-correctness.ConstAlignment: west, \
+// RUN:   misc-const-correctness.WarnPointersAsValues: true, \
+// RUN:   misc-const-correctness.TransformPointersAsValues: true \
+// RUN:   }}"
+// RUN: %check_clang_tidy %s misc-const-correctness %t -check-suffixes=",WEST" \
+// RUN:   -config="{CheckOptions: { \
+// RUN:   misc-const-correctness.ConstAlignment: left, \
+// RUN:   misc-const-correctness.WarnPointersAsValues: true, \
+// RUN:   misc-const-correctness.TransformPointersAsValues: true \
+// RUN:   }}"
+// RUN: %check_clang_tidy %s misc-const-correctness %t -check-suffixes=",EAST" \
+// RUN:   -config="{CheckOptions: { \
+// RUN:   misc-const-correctness.ConstAlignment: east, \
+// RUN:   misc-const-correctness.WarnPointersAsValues: true, \
+// RUN:   misc-const-correctness.TransformPointersAsValues: true \
+// RUN:   }}"
+// RUN: %check_clang_tidy %s misc-const-correctness %t -check-suffixes=",EAST" \
+// RUN:   -config="{CheckOptions: { \
+// RUN:   misc-const-correctness.ConstAlignment: right, \
+// RUN:   misc-const-correctness.WarnPointersAsValues: true, \
+// RUN:   misc-const-correctness.TransformPointersAsValues: true \
+// RUN:   }}"
+
+void foo() {
+  int I = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'I' of type 'int' can be declared 'const'
+  // CHECK-FIXES-WEST: const int I = 0;
+  // CHECK-FIXES-EAST: int const I = 0;
+}
+
+void foo(int I) {
+  int & IRef = I;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'IRef' of type 'int &' can be declared 'const'
+  // CHECK-FIXES-WEST: const int & IRef = I;
+  // Having the double space and the const attached the the `&` is unfortunate,
+  // however it shouldn't matter for users as fixes get clang-formatted.
+  // CHECK-FIXES-EAST: int  const& IRef = I;
+
+  // When decorating a pointer with const, the const always goes to the right 
+  // of the `*`.
+  int *IPtr = &I;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'IPtr' of type 'int *' can be declared 'const'
+  // CHECK-FIXES: int *const IPtr = &I;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
@@ -148,3 +148,25 @@
 int *changing_pointee = &value;
 changing_pointee = &result;
 
+.. option:: ConstAlignment 
+
+  Controls where to insert ``const`` when creating fix-it hints, defaults to ``right``.
+
+  - ``left``:
+
+   .. code-block:: c++ 
+
+ const int I = 0;
+ const int &IRef = I;
+ int *const IPtr = &I;
+
+
+  - ``right``:
+
+   .. code-block:: c++
+  
+ int const I = 0;
+ int const &IRef = I;
+ int *const IPtr = &I;
+
+  ``east`` and ``west`` aliases are also supported.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,6 +121,10 @@
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
 
+- :doc:`misc-const-correctness `
+  check now supports a ``ConstAlignment`` option to control the location of the
+  ``const`` keyword when emitting fixes.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
===
--- clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
+++ clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CONSTCORRECTNESSCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/FixItHintUtils.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "llvm/ADT/DenseSet.h"
 
@@ -48,6 +49,8 @@
   const bool TransformValues;
   const bool TransformReferences;
   const 

[PATCH] D131802: [clang] fix missing initialization of original number of expansions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This looks reasonable to me, but I leave it to others to do the final sign-off. 
Please be sure to add a release note for the fix though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131802

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131386#3722822 , @njames93 wrote:

> In D131386#3722749 , @aaron.ballman 
> wrote:
>
>> We leave formatting decisions in clang-tidy to clang-format and I don't 
>> think we should deviate from that policy here without a very clear 
>> understanding of when we should relax that restriction. That said, I'm 
>> personally not certain we should have such an option (the long-term goal has 
>> generally been to integrate clang-format functionality into clang-tidy so 
>> there can be an option to just run format after applying fixes in a TU). Is 
>> there a compelling reason we should have it?
>
> The reason for this is due to the issue that `QualifierAlignment`  is a non 
> whitespace only change and clang-format lists that using it could break some 
> code.

Yes, and the community decided that risk was reasonable for clang-format.

> In light of this some users may wish to set the option to `QAS_Leave` to be 
> sure no code is broken even though they would prefer a specific style. 
> Therefore having a dedicated option in the check will let those users specify 
> the style, without having to set a clang-format configuration which they 
> aren't content in using.

When we decided that clang-format was allowed to break code, we also decided 
that any time it does so on real world code is considered a bug and is 
something we should work to fix. So I'm not in favor of this change; if 
clang-tidy uncovers a bug in clang-format, that should be fixed in clang-format 
rather than worked around in clang-tidy. And if clang-tidy can't trust 
clang-format to be production quality, that's something we should address with 
the clang-format folks as a serious concern that needs a plan of action. But I 
don't think introducing formatting options into clang-tidy is a road we want to 
go down.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131386#3722849 , @aaron.ballman 
wrote:

> In D131386#3722822 , @njames93 
> wrote:
>
>> In D131386#3722749 , 
>> @aaron.ballman wrote:
>>
>>> We leave formatting decisions in clang-tidy to clang-format and I don't 
>>> think we should deviate from that policy here without a very clear 
>>> understanding of when we should relax that restriction. That said, I'm 
>>> personally not certain we should have such an option (the long-term goal 
>>> has generally been to integrate clang-format functionality into clang-tidy 
>>> so there can be an option to just run format after applying fixes in a TU). 
>>> Is there a compelling reason we should have it?
>>
>> The reason for this is due to the issue that `QualifierAlignment`  is a non 
>> whitespace only change and clang-format lists that using it could break some 
>> code.
>
> Yes, and the community decided that risk was reasonable for clang-format.
>
>> In light of this some users may wish to set the option to `QAS_Leave` to be 
>> sure no code is broken even though they would prefer a specific style. 
>> Therefore having a dedicated option in the check will let those users 
>> specify the style, without having to set a clang-format configuration which 
>> they aren't content in using.
>
> When we decided that clang-format was allowed to break code, we also decided 
> that any time it does so on real world code is considered a bug and is 
> something we should work to fix. So I'm not in favor of this change; if 
> clang-tidy uncovers a bug in clang-format, that should be fixed in 
> clang-format rather than worked around in clang-tidy. And if clang-tidy can't 
> trust clang-format to be production quality, that's something we should 
> address with the clang-format folks as a serious concern that needs a plan of 
> action. But I don't think introducing formatting options into clang-tidy is a 
> road we want to go down.

I suppose another way to look at it is: clang-format options that are 
considered dangerous (comes with a warning as QualifierAlignment does in 
https://clang.llvm.org/docs/ClangFormatStyleOptions.html) are things clang-tidy 
checks with fixes could have options for. However, that gets awkward when 
clang-format finds a way to remove that warning; then clang-tidy is left 
supporting formatting options that it shouldn't have (but removing those 
options can break people's .clang-tidy config files), so I'm not certain I like 
that approach any better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D3976: -Wcomma, a new warning for questionable uses of the comma operator

[Gregory PAKOSZ via Phabricator via cfe-commits]

gpakosz added a comment.

>> Should I open a bug?
>
> I think opening a bug is appropriate, good catch!

Thanks for the reply Aaron.
Here's the report: https://github.com/llvm/llvm-project/issues/57151


Repository:
  rL LLVM

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

https://reviews.llvm.org/D3976

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

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551
+  if (auto ValD = Info.EvaluatingDecl.dyn_cast()) {
+const VarDecl *VD = dyn_cast_or_null(ValD);
+if (VD && !VD->isConstexpr())
+  NotConstexprVar = true;
+  }

This seems to be equivalent unless I'm misunderstanding something about the 
member dyn_cast.



Comment at: clang/lib/AST/ExprConstant.cpp:13572-13575
 if (ED->getNumNegativeBits() &&
 (Max.slt(Result.getInt().getSExtValue()) ||
- Min.sgt(Result.getInt().getSExtValue(
-  Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-   
diag::warn_constexpr_unscoped_enum_out_of_range)
-  << llvm::toString(Result.getInt(),10) << Min.getSExtValue() << 
Max.getSExtValue();
+ Min.sgt(Result.getInt().getSExtValue())) &&
+!NotConstexprVar)

Might as well test the easy condition before doing more work to get values and 
compare them.

I think we should rename `NotConstexprVar` to `ConstexprVar` so that we don't 
need the double negation here. WDYT?



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2424-2427
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant 
expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}

Do you think it's worth it to add this as a test as well?
```
consteval void testDefaultArgForParam2(E2 e2Param = (E2)-1) {
}

void test() {
  testDefaultArgForParam2(); // Make sure we get the error
  testDefaultArgForParam2((E2)0); // Make sure we don't get the error
}
```


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

https://reviews.llvm.org/D131874

___
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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:892
+ LangOptions::ClangABI::Ver15)) {
+  // C++03 [class]p4:
+  //   A POD-struct is an aggregate class that has [...] no 
user-defined

dblaikie wrote:
> erichkeane wrote:
> > Does this language change much if there are constraints on this method?  We 
> > might need to consider that if we are breaking ABI here.
> Oh, sorry, I should've used more words - but this is intended as an ABI 
> break/fix. It's a place where Clang's ABI implementation is inconsistent with 
> GCC and MSVC - so this is intended to fix/make Clang compatible with those 
> ABIs where we are trying to be compatible with them. (& why we're checking 
> the ClangABI version here - so if you're asking for the old ABI you still get 
> it)
> 
> Perhaps I'm misunderstanding your questions/concerns?
Yeah, my concern is whether this behavior is different with a constraint on it. 
 That is, does the eligibility of the constructor matter for this case?  

This ctor/copy assign/etc might not pass a constraint check/never be callable.

So the question is whether we need to consider that here.


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] D119051: Extend the C++03 definition of POD to include defaulted functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do we need to gate this on use of `-fms-compatibility` as well? I'm not certain 
how cygwin factors in where it's sort of gcc and sort of msvc (perhaps the 
triple is sufficient for that?).




Comment at: clang/lib/AST/DeclCXX.cpp:892
+if ((!Method->isDeleted() && !Method->isDefaulted() &&
+ SMKind != SMF_MoveAssignment) ||
+(getLangOpts().getClangABICompat() <=

Ah, it took me a moment to realize that move assignment is called out here but 
not move construction because move construction is handled above.



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

dblaikie wrote:
> aaron.ballman wrote:
> > 
> This doesn't compile under the first `RUN` line which uses C++98, I think?
Ah, fair point. I don't have a strong opinion, but you could do 
`-Dstatic_assert=_Static_assert` for that RUN line and not use the extension 
version. But it doesn't matter for the test functionality, so whichever 
approach sparks the most joy for you is fine by me.


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] D131388: [docs] Add "C++20 Modules"

[Daniel Ruoso via Phabricator via cfe-commits]

ruoso added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:338-339
+
+If we want to create libraries for the BMIs, we can't wrap these BMIs directly.
+We must compile these BMIs(``*.pcm``) into object files(``*.o``) and wrap 
these object files.
+




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

https://reviews.llvm.org/D131388

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

[PATCH] D131888: [Clang][Interp] Add some basic functionality to the experimental new constant interpreter

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, MaskRay, rsmith, 
nand, nandor.
Herald added a subscriber: StephenFan.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

I wanted to maybe work on the new experimental constant interpreter in clang. I 
took last week to get to know the code a little and implement some basic 
functionality.

New in this patch are:

- Simple function calls
- array initializers and subscript expressions
- pointer reference and dereference
- local and global variables

Some tests for the functionality is in `test/AST/Interp/interp.cpp`.

As far as I know the interpreter has been abandoned quite some time ago, so I 
was just looking for some basic feedback about whether this is worth it to 
pursue. If the feedback is positive, I will continue working on it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131888

Files:
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.h
  clang/lib/AST/Interp/Disasm.cpp
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBlock.h
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Source.h
  clang/test/AST/Interp/interp.cpp

Index: clang/test/AST/Interp/interp.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/interp.cpp
@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fexperimental-new-constant-interpreter %s -verify
+
+// expected-no-diagnostics
+
+constexpr bool foo() { return true; }
+constexpr bool bar() { return false; }
+static_assert(foo());
+
+static_assert(5);
+static_assert(!0);
+
+///--
+/// Unary inv
+static_assert(!false);
+static_assert(!!true);
+static_assert(!bar());
+static_assert(!!foo());
+
+///--
+/// Return values
+constexpr int get4() { return 4; }
+static_assert(get4() != 5);
+
+
+///--
+/// Functions calling other functions
+constexpr bool getTrue() { return foo(); }
+constexpr bool getTrue2() { return !bar(); }
+static_assert(getTrue());
+static_assert(getTrue2());
+
+//--
+// Parameters
+// TODO:
+//template
+//constexpr T identity(T t) { return t; }
+//static_assert(identity(true));
+constexpr bool bool_identity(bool b) { return b; }
+static_assert(bool_identity(true));
+static_assert(!bool_identity(false));
+
+constexpr bool inv(bool b) { return !b;}
+static_assert(inv(false));
+
+constexpr int add(int a, int b) { return a + b; }
+static_assert(add(1,2) == 3);
+
+constexpr int sub(int a, int b) { return a - b; }
+static_assert(sub(0, 2) == -2);
+
+
+
+///--
+/// Global Variables
+constexpr bool t = true;
+constexpr bool f = false;
+constexpr int  n = 13;
+constexpr int zero = 0;
+static_assert(t);
+static_assert(!f);
+static_assert(n);
+static_assert(!zero);
+
+
+///--
+/// Local variables
+constexpr int assign() {
+  int f = 20;
+  f = 100;
+  return f;
+}
+static_assert(assign() == 100);
+
+
+
+///--
+/// Pointers
+
+constexpr const int *ptr = nullptr;
+static_assert(ptr == nullptr);
+constexpr const int *const* ptr2 = &ptr;
+static_assert(ptr2 != nullptr);
+
+constexpr const int *zomg = *ptr2;
+static_assert(zomg == nullptr);
+
+constexpr int five = 5;
+constexpr const int *pointer_to_5 = &five;
+constexpr const int five_from_pointer = *pointer_to_5;
+static_assert(five_from_pointer == 5);
+
+
+
+
+
+///--
+/// If Statements
+constexpr int cond(bool b) {
+  if (b) {
+return 5;
+  } else {
+return 3;
+  }
+}
+static_assert(cond(true) == 5);
+static_assert(cond(false) == 3);
+
+
+///--
+/// Arrays
+constexpr int intarr[] = { 1, 2, 3, 4 };
+static_assert(intarr[0] == 1);
+static_assert(intarr[1] == 2);
+static_assert(intarr[2] == 3);
+static_assert(intarr[3] == 4);
+constexpr int someArr() {
+  int v[] = { 1, 2, 3, 4};
+  return v[0] + v

[PATCH] D131888: [Clang][Interp] Add some basic functionality to the experimental new constant interpreter

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So you're right, we haven't seen much work on this interpreter in a while, and 
I have no real experience with reviewing it at all, which I'm sure is the 
experience of most of my co-reviewers.

At the same time, I believe there is good potential in this interpreter, so I 
would love to see progress on it.

SO, if you could start by splitting this patch up into the smallest parts 
possible, it would be really appreciated.  This patch seems to be doing a lot, 
and I'm a bit overwhelmed trying to review it all at once.

Additionally, I suspect there is going to be a LOT of tests, so I'd suggest 
writing 1 whole 'test file' per topic (instead of throwing everything into 
interp.cpp).  I also suspect that there is need for more in depth testing than 
that.  For exmaple, 'if' statements likely need to check things like 
side-effects and init statements/etc.

One valuable strategy might be to temporarily switch this over to the 'default' 
interpreter in your local workspace, make changes, and see if you can add a new 
'run' line to any existing tests that each individual change you make causes to 
pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131888

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

[PATCH] D131888: [Clang][Interp] Add some basic functionality to the experimental new constant interpreter

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

tbaeder added a comment.

Thanks for the fast reply, Erich!

In D131888#3723017 , @erichkeane 
wrote:

> So you're right, we haven't seen much work on this interpreter in a while, 
> and I have no real experience with reviewing it at all, which I'm sure is the 
> experience of most of my co-reviewers.

Sure, that's expected.

> At the same time, I believe there is good potential in this interpreter, so I 
> would love to see progress on it.
>
> SO, if you could start by splitting this patch up into the smallest parts 
> possible, it would be really appreciated.  This patch seems to be doing a 
> lot, and I'm a bit overwhelmed trying to review it all at once.

I actually have several smaller patches locally but merged them into one for 
review :) I'll try to find a middle way.

> Additionally, I suspect there is going to be a LOT of tests, so I'd suggest 
> writing 1 whole 'test file' per topic (instead of throwing everything into 
> interp.cpp).  I also suspect that there is need for more in depth testing 
> than that.  For exmaple, 'if' statements likely need to check things like 
> side-effects and init statements/etc.
>
> One valuable strategy might be to temporarily switch this over to the 
> 'default' interpreter in your local workspace, make changes, and see if you 
> can add a new 'run' line to any existing tests that each individual change 
> you make causes to pass.

I tried running the clang testsuite locally with the new interpreter, but 
basically all tests fail because something is missing, so I think that's gonna 
have to wait. But adding more files and tests on a per-patch basis is a good 
approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131888

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

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

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Hi @Sockke - this seems to be ready to land. Is there something holding it 
back? We ran into the same issue recently.


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

https://reviews.llvm.org/D127293

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 452655.
steplong added a comment.

- Update unit test to see if this regex works


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

Files:
  clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-zc.cpp
  clang/unittests/StaticAnalyzer/CallEventTest.cpp

Index: clang/unittests/StaticAnalyzer/CallEventTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "CheckerRegistration.h"
+#include "gmock/gmock.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
@@ -22,6 +23,8 @@
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
+using testing::MatchesRegex;
+
 namespace clang {
 namespace ento {
 namespace {
@@ -81,7 +84,12 @@
 }
   )",
  Diags));
-  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+// Sized deallocation is enabled on Windows, so we expect two arguments.
+#if defined(GTEST_USES_POSIX_RE)
+  EXPECT_THAT(Diags, MatchesRegex("test.CXXDeallocator: NumArgs: (1|2)\n"));
+#elif defined(GTEST_USES_SIMPLE_RE)
+  EXPECT_THAT(Diags, MatchesRegex("test.CXXDeallocator: NumArgs: 1?\n?2?\n?"));
+#endif
 }
 
 } // namespace
Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,19 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC %s
+// RUN: %clang_cl /c -### -fms-compatibility-version=18 -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC-OFF %s
+// SIZED-DEALLOC: "-fsized-deallocation"
+// SIZED-DEALLOC-OFF-NOT: "-fsized-deallocation"
+
+// RUN: %clang_cl /c /std:c++11 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++14 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++20 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++latest -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// ALIGNED-NEW-BEFORE-CPP17-NOT: "-faligned-allocation"
+// ALIGNED-NEW-CPP17ONWARDS: "-faligned-allocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6470,6 +6470,7 @@
 }
 CmdArgs.push_back(LanguageStandard.data());
   }
+  bool IsCPP17Onwards = false;
   if (ImplyVCPPCXXVer) {
 StringRef LanguageStandard;
 if (const Arg *StdArg = Args.getLastArg(options::OPT__SLASH_std)) {
@@ -6493,6 +6494,9 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+IsCPP17Onwards =
+LanguageStandard != "-std=c++11" && LanguageStandard != "-std=c++14";
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6622,9 +6626,15 @@
 options::OPT_fno_relaxed_template_template_args);
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
-  // most platforms.
-  Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
-options::OPT_fno_sized_deallocation);
+  // most platforms. MSVC turns on /Zc:sizedDealloc by default, starting in
+  // MSVC 2015.
+  if (IsWindowsMSVC && IsMSVC2015Compatible &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+  else
+Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
+  options::OPT_fno_sized_deallocation);
 
   // -faligned-allocation is on by default in C++17 onwards and otherwise off
   // by default.
@@ -6635,6 +6645,8 @@
   CmdArgs.push_back("-fno-aligned-allocation");
 else
   CmdArgs.push_back("-faligned-allocation");
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }
 
   // The default new alignment can be specified using a dedicated option or via
Index: clang-tools-extra/test/clang-tidy/checkers/mis

[PATCH] D131687: [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret

[Caroline via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll:127
 ; CHECK-NEXT:ptrue p0.d
-; CHECK-NEXT:st1d { z16.d }, p0, [sp]
-; CHECK-NEXT:st1d { z17.d }, p0, [sp, #1, mul vl]
-; CHECK-NEXT:st1d { z18.d }, p0, [sp, #2, mul vl]
+; CHECK-NEXT:st1d { z2.d }, p0, [sp]
+; CHECK-NEXT:st1d { z2.d }, p0, [sp, #1, mul vl]

sdesmalen wrote:
> I would have expected no asm changes. The test is not equivalent to the 
> previous code.
Yes, you were correct. I was taken the same load twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131687

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

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

I think we usually try to do the same, if the intrinsics have been in released 
compilers. There is an example in 
https://reviews.llvm.org/D98487#change-tOTTgECYYAO5, hopefully these would be 
equally simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131547

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

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

[Thomas Etter via Phabricator via cfe-commits]

thomasetter added a comment.

Is there anything else you would like me to change?


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

https://reviews.llvm.org/D130130

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

[PATCH] D131891: [clang][dataflow] Debug string for value kinds.

[weiyi via Phabricator via cfe-commits]

wyt created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131891

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


Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -31,6 +31,32 @@
 using llvm::fmt_pad;
 using llvm::formatv;
 
+std::string debugString(Value::Kind Kind) {
+  switch (Kind) {
+  case Value::Kind::Integer:
+return "Integer";
+  case Value::Kind::Reference:
+return "Reference";
+  case Value::Kind::Pointer:
+return "Pointer";
+  case Value::Kind::Struct:
+return "Struct";
+  case Value::Kind::AtomicBool:
+return "AtomicBool";
+  case Value::Kind::Conjunction:
+return "Conjunction";
+  case Value::Kind::Disjunction:
+return "Disjunction";
+  case Value::Kind::Negation:
+return "Negation";
+  case Value::Kind::Implication:
+return "Implication";
+  case Value::Kind::Biconditional:
+return "Biconditional";
+  }
+  llvm_unreachable("Unhandled value kind");
+}
+
 std::string debugString(Solver::Result::Assignment Assignment) {
   switch (Assignment) {
   case Solver::Result::Assignment::AssignedFalse:
Index: clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
===
--- clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -24,6 +24,9 @@
 namespace clang {
 namespace dataflow {
 
+/// Returns a string representation of a value kind.
+std::string debugString(Value::Kind Kind);
+
 /// Returns a string representation of a boolean assignment to true or false.
 std::string debugString(Solver::Result::Assignment Assignment);
 


Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -31,6 +31,32 @@
 using llvm::fmt_pad;
 using llvm::formatv;
 
+std::string debugString(Value::Kind Kind) {
+  switch (Kind) {
+  case Value::Kind::Integer:
+return "Integer";
+  case Value::Kind::Reference:
+return "Reference";
+  case Value::Kind::Pointer:
+return "Pointer";
+  case Value::Kind::Struct:
+return "Struct";
+  case Value::Kind::AtomicBool:
+return "AtomicBool";
+  case Value::Kind::Conjunction:
+return "Conjunction";
+  case Value::Kind::Disjunction:
+return "Disjunction";
+  case Value::Kind::Negation:
+return "Negation";
+  case Value::Kind::Implication:
+return "Implication";
+  case Value::Kind::Biconditional:
+return "Biconditional";
+  }
+  llvm_unreachable("Unhandled value kind");
+}
+
 std::string debugString(Solver::Result::Assignment Assignment) {
   switch (Assignment) {
   case Solver::Result::Assignment::AssignedFalse:
Index: clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
===
--- clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -24,6 +24,9 @@
 namespace clang {
 namespace dataflow {
 
+/// Returns a string representation of a value kind.
+std::string debugString(Value::Kind Kind);
+
 /// Returns a string representation of a boolean assignment to true or false.
 std::string debugString(Solver::Result::Assignment Assignment);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

> but removing those options can break people's .clang-tidy config files

Aren't there deprecation mechanisms? I think trying to be backwards compatible 
across all possible versions can lead to suboptimal solutions and make the tool 
harder to use and hard/slow to adapt to the needs of the community.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131386#3723144 , @carlosgalvezp 
wrote:

>> but removing those options can break people's .clang-tidy config files
>
> Aren't there deprecation mechanisms? I think trying to be backwards 
> compatible across all possible versions can lead to suboptimal solutions and 
> make the tool harder to use and hard/slow to adapt to the needs of the 
> community.

Nothing stops us from deprecating config options with some nice diagnostic 
behavior, but again, this pushes the work off onto the user to maintain their 
scripts and gives them an inconsistent interface to clang-tidy where some 
checks get formatting options and other checks do not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D131809: [clang][dataflow] Add an option for context-sensitive depth

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:395
   // `DeclContext` of the block being analysed if provided.
-  const DeclContext *DeclCtx = nullptr;
+  std::vector CallString;
 





Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:210
+ const DeclContext *Callee) const {
+  return CallString.size() <= MaxDepth &&
+ std::find(CallString.begin(), CallString.end(), Callee) ==

If `canDescend` is supposed to return false for `MaxDepth = 0`, shouldn't this 
be `<`?



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:4003
 
-TEST(TransferTest, ContextSensitiveSetTwoLayers) {
+TEST(TransferTest, ContextSensitiveSetTwoLayersDepthOne) {
   std::string Code = R"(

Let's add a similar test with `Depth` set to 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131809

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

[PATCH] D131818: [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks!

Can you add a test for non-constexpr unreachable code within the constexpr if 
and test that that still warns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131818

___
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

[Alexander Yermolovich via Phabricator via cfe-commits]

ayermolo added a comment.

In D131528#3722395 , @shafik wrote:

> In D131528#3719430 , @ayermolo 
> wrote:
>
>> Was this and previous change intended to capture this case?
>>  const enum NumberType neg_one = (enum NumberType) ((enum NumberType) 0 - 
>> (enum NumberType) 1);
>
> That was not intended and I plan on fixing that.

Ah I see. Thanks for elaborating. Can you tag me in the fix diff please.


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] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D131386#3723164 , @aaron.ballman 
wrote:

> In D131386#3723144 , @carlosgalvezp 
> wrote:
>
>>> but removing those options can break people's .clang-tidy config files
>>
>> Aren't there deprecation mechanisms? I think trying to be backwards 
>> compatible across all possible versions can lead to suboptimal solutions and 
>> make the tool harder to use and hard/slow to adapt to the needs of the 
>> community.
>
> Nothing stops us from deprecating config options with some nice diagnostic 
> behavior, but again, this pushes the work off onto the user to maintain their 
> scripts and gives them an inconsistent interface to clang-tidy where some 
> checks get formatting options and other checks do not.

Yes, I'd agree that the interface becomes inconsistent. Best would be a unified 
tool that does both fix and formatting as you mentioned earlier.

FWIW, there are already other checks that have formatting settings, for example:
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-bounds-constant-array-index.html#cmdoption-arg-includestyle


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6649
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }

Do we want to do this in the driver? From what I understand, we already do this 
(MSVC or not) at the Driver->Frontend boundary. See Options.td:

```
defm aligned_allocation : BoolFOption<"aligned-allocation",
  LangOpts<"AlignedAllocation">, Default,
  PosFlag,
  NegFlag, BothFlags<[CC1Option]>>;
```

It defaults to on when c++17 is set, and makes its way to 
`LangOpts.AlignedAllocation`.

Does this change here have any effect? From looking at the code, this should be 
the current behavior already (?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

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

[PATCH] D131809: [clang][dataflow] Add an option for context-sensitive depth

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

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

Once the Stanislav's comments are resolved, it looks good to me.




Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:665
+if (!(Options.ContextSensitiveOpts &&
+  Env.canDescend(Options.ContextSensitiveOpts->Depth, F)))
   return;

Alternatively, `canDescend` could get the optional `ContextSensitiveOpts` and 
we can do all the checking there. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131809

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

[PATCH] D131891: [clang][dataflow] Debug string for value kinds.

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

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp:34
 
+std::string debugString(Value::Kind Kind) {
+  switch (Kind) {

I think all of these `debugString` functions could return a `llvm::StringRef` 
or an `std::string_view`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131891

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

[PATCH] D131755: [CMake] Explicit bootstrap options override any passthrough ones.

[Chris Bieneman via Phabricator via cfe-commits]

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

Looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131755

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

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

[YingChi Long via Phabricator via cfe-commits]

inclyc created this revision.
Herald added a project: All.
inclyc updated this revision to Diff 452665.
inclyc added a comment.
inclyc added reviewers: aaron.ballman, rtrieu.
inclyc published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

comments


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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131892

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning 
type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13958,7 +13958,7 @@
 }
 
 // Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13973,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14017,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13958,7 +13958,7 @@
 }
 
 // Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13973,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14017,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal
Thank you for the review.
Could you please do testing for me, since I'm on Windows and have no prepared 
testing environment as I know you are.
I'll add an assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131707

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

[clang] b8a1b69 - [clang] fix missing initialization of original number of expansions

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2022-08-15T17:39:38+02:00
New Revision: b8a1b698afb2fc84819c7596090aabf4d826b436

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

LOG: [clang] fix missing initialization of original number of expansions

When expanding undeclared function parameters, we should initialize
the original number of expansions, if known, before trying to expand
them, otherwise a length mismatch with an outer pack might not be
diagnosed.

Fixes PR56094.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/TreeTransform.h
clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a635b734c47aa..ded0b39c27ea5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -71,6 +71,9 @@ Bug Fixes
 - Fix `#57008 `_ - Builtin
   C++ language extension type traits instantiated by a template with unexpected
   number of arguments cause an assertion fault.
+- Fix multi-level pack expansion of undeclared function parameters.
+  This fixes `Issue 56094 
`_.
+
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 07e71a2be4f2c..dd684d42665b2 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -5792,6 +5792,7 @@ bool TreeTransform::TransformFunctionTypeParams(
= dyn_cast(OldType)) 
{
   // We have a function parameter pack that may need to be expanded.
   QualType Pattern = Expansion->getPattern();
+  NumExpansions = Expansion->getNumExpansions();
   SmallVector Unexpanded;
   getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
 

diff  --git a/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp 
b/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
index 206e9f73e9f05..6c4c260cdf8f0 100644
--- a/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -469,3 +469,25 @@ int fn() {
   bar(b);
 }
 }
+
+namespace pr56094 {
+template  struct D {
+  template  using B = int(int (*...p)(T, U));
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a 
diff erent length (1 vs. 2) from outer parameter packs}}
+  template  D(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested 
here}}
+};
+using t1 = D::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::D' 
requested here}}
+
+template  struct F {};
+template  struct G {};
+template  struct E {
+  template  using B = G...>;
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a 
diff erent length (1 vs. 2) from outer parameter packs}}
+  template  E(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested 
here}}
+};
+using t2 = E::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::E' 
requested here}}
+} // namespace pr56094



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

[PATCH] D131802: [clang] fix missing initialization of original number of expansions

[Matheus Izvekov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb8a1b698afb2: [clang] fix missing initialization of original 
number of expansions (authored by mizvekov).

Changed prior to commit:
  https://reviews.llvm.org/D131802?vs=452281&id=452688#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131802

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp


Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -469,3 +469,25 @@
   bar(b);
 }
 }
+
+namespace pr56094 {
+template  struct D {
+  template  using B = int(int (*...p)(T, U));
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a 
different length (1 vs. 2) from outer parameter packs}}
+  template  D(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested 
here}}
+};
+using t1 = D::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::D' 
requested here}}
+
+template  struct F {};
+template  struct G {};
+template  struct E {
+  template  using B = G...>;
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a 
different length (1 vs. 2) from outer parameter packs}}
+  template  E(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested 
here}}
+};
+using t2 = E::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::E' 
requested here}}
+} // namespace pr56094
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -5792,6 +5792,7 @@
= dyn_cast(OldType)) 
{
   // We have a function parameter pack that may need to be expanded.
   QualType Pattern = Expansion->getPattern();
+  NumExpansions = Expansion->getNumExpansions();
   SmallVector Unexpanded;
   getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -71,6 +71,9 @@
 - Fix `#57008 `_ - Builtin
   C++ language extension type traits instantiated by a template with unexpected
   number of arguments cause an assertion fault.
+- Fix multi-level pack expansion of undeclared function parameters.
+  This fixes `Issue 56094 
`_.
+
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -469,3 +469,25 @@
   bar(b);
 }
 }
+
+namespace pr56094 {
+template  struct D {
+  template  using B = int(int (*...p)(T, U));
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  template  D(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
+};
+using t1 = D::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::D' requested here}}
+
+template  struct F {};
+template  struct G {};
+template  struct E {
+  template  using B = G...>;
+  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  template  E(B *);
+  // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
+};
+using t2 = E::B;
+// expected-note@-1 {{in instantiation of template class 'pr56094::E' requested here}}
+} // namespace pr56094
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -5792,6 +5792,7 @@
= dyn_cast(OldType)) {
   // We have a function parameter pack that may need to be expanded.
   QualType Pattern = Expansion->getPattern();
+  NumExpansions = Expansion->getNumExpansions();
   SmallVector Unexpanded;
   getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -71,6 +71,9 @@
 - Fix `#57008 `_ - Builtin
   C++ language extension type traits instantiated by a temp

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 452689.
ASDenysPetrov added a comment.

Add assertion accroding to the remark.


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

https://reviews.llvm.org/D131707

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed 
value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,15 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext &ctx = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext &C = getContext();
+  CachedLocationType = T->isObjCObjectType() ? 
C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+  assert(!CachedLocationType.isNull() &&
+ "Cached value supposed to be non-null.");
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext &Context) const {


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,15 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext &ctx = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext &C = getContext();
+  CachedLocationType = T->isObjCObjectType() ? C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+  assert(!CachedLocationType.isNull() &&
+ "Cached value supposed to be non-null.");
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext &Context) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D131707#3723403 , @ASDenysPetrov 
wrote:

> @steakhal
> Thank you for the review.
> Could you please do testing for me, since I'm on Windows and have no prepared 
> testing environment as I know you do.
> I'll add an assertion.




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

https://reviews.llvm.org/D131707

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

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 452691.
inclyc added a comment.

Sync comments of function `IgnoreCommaOperand`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning 
type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : 
LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returins void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14019,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returins void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14019,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 452692.
inclyc added a comment.

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning 
type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : 
LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returns void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14019,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);


Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,16 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void buggy(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returns void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,9 @@
 }
   }
 
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();
   return false;
 }
 
@@ -14014,7 +14019,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov edited the summary of this revision.
mizvekov updated this revision to Diff 452693.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
  clang/test/SemaTemplate/pack-deduction.cpp

Index: clang/test/SemaTemplate/pack-deduction.cpp
===
--- clang/test/SemaTemplate/pack-deduction.cpp
+++ clang/test/SemaTemplate/pack-deduction.cpp
@@ -134,14 +134,14 @@
   template struct tuple {};
   template struct A {
 template static pair, tuple> f(pair ...p);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 // expected-note@-2 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}}
 
 template static pair, tuple> g(pair ...p, ...);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 
 template static tuple h(tuple..., pair>);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}}
   };
 
   pair, tuple> k1 = A().f(pair(), pair());
Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -97,7 +97,7 @@
   template  struct Constant {};
 
   template  struct Sum {
-template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter pack 'Js' that has a different length (1 vs. 2) from outer parameter packs}}
+template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter packs 'Is' and 'Js' that have different lengths (1 vs. 2)}}
   };
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -473,7 +473,7 @@
 namespace pr56094 {
 template  struct D {
   template  using B = int(int (*...p)(T, U));
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
   template  D(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
@@ -484,7 +484,7 @@
 template  struct G {};
 template  struct E {
   template  using B = G...>;
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'I' and 'U' that have different lengths (1 vs. 2)}}
   template  E(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+  Unexpanded.push_back({T, SourceLocation()});
+  return true;
+}
+
+bool
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}
+
 /// Record occurrences of function and non-type template
 /// parameter packs in an expression.
 bool VisitDeclRefExpr(DeclRefExpr *E) {
@@ -306,7 +323,8 @@
   auto *TTPD = dyn_cast(Loc

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a subscriber: balazske.
steakhal added a comment.

In D131707#3723403 , @ASDenysPetrov 
wrote:

> @steakhal
> Thank you for the review.
> Could you please do testing for me, since I'm on Windows and have no prepared 
> testing environment as I know you do.

ATM it seems like I don't have the bandwidth. What I meant is choosing the 
right projects to test etc.
Maybe @balazske could give some helping hands. AFAIK it shouldn't be hard for 
him to enqueue a benchmark.
What do you say @balazske, could you please have a differential measurement in 
release configuration on the test set demonstrating the performance gain of 
this patch compared to the baseline?


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

https://reviews.llvm.org/D131707

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D131386#3722749 , @aaron.ballman 
wrote:

> We leave formatting decisions in clang-tidy to clang-format and I don't think 
> we should deviate from that policy here without a very clear understanding of 
> when we should relax that restriction. That said, I'm personally not certain 
> we should have such an option (the long-term goal has generally been to 
> integrate clang-format functionality into clang-tidy so there can be an 
> option to just run format after applying fixes in a TU). Is there a 
> compelling reason we should have it?

The reason for me to work on this feature is the fact that clang-format doesn't 
work reliable for changing the location of the `const`. After using clang-tidy 
and clang-format some `const`s were still one the right hand side. I didn't do 
a deep investigation, but it seems to affect classes in namespaces. So some 
occurrences of `std::string` were affected, but not all. So in the current 
state I can't use this clang-tidy fix.

Since clang-format doesn't fully parse the code I wonder whether all cases can 
be properly fixed. On the other hand clang-tidy has all information available.

Would integrating clang-format in clang-tidy mean clang-format will use the 
full information of the AST?




Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp:66
+  ConstAlignment(
+  Options.get("ConstAlignment", utils::fixit::QualifierPolicy::Right)) 
{
   if (AnalyzeValues == false && AnalyzeReferences == false)

njames93 wrote:
> Mordante wrote:
> > I would suggest to use `QualifierAlignment` to match the name in 
> > clang-format.
> I thought about that, but the clang-format option doesn't just align the 
> const qualifier. It works for all qualifiers.
> I am easy on what name we use for the option.
Fair point.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst:149
 int *changing_pointee = &value;
 changing_pointee = &result;
 

Do you want to incorporate the D131859 here too or do you prefer that I make a 
separate review for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

___
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)

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:100
+ "%select{binary|unary}0 bitwise operator")
+  << IsUnary << OperatorLoc;
 }

We still need the source range of the operand
```
[build]   Value <<= diagTest(1, 2, 3);
[build] ~~~ ^
```
Vs 
```
[build]   Value <<= diagTest(1, 2, 3);
[build] ~~~ ^
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131678

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

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

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

FYI, had an offline chat with @dblaikie and we decided that the best way 
forward would be to try stop emitting auto return types in DWARF, instead of 
relying on lambda/member-function specific hacks. Not sure what the timeline on 
this will be but tracking it internally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123319

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

[PATCH] D131818: [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Alan Zhao via Phabricator via cfe-commits]

ayzhao updated this revision to Diff 452721.
ayzhao added a comment.

Add tests for nested non-constexpr if


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131818

Files:
  clang/lib/Analysis/ReachableCode.cpp
  clang/test/SemaCXX/unreachable-code.cpp


Index: clang/test/SemaCXX/unreachable-code.cpp
===
--- clang/test/SemaCXX/unreachable-code.cpp
+++ clang/test/SemaCXX/unreachable-code.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
+// RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
 
 int j;
 int bar();
@@ -99,3 +99,34 @@
 }
 
 }
+
+namespace gh57123 {
+  bool foo() {
+if constexpr (true) {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+else
+  return false; // no-warning
+  }
+
+  bool bar() {
+if (true)
+  return true;
+else
+  return false; // expected-warning {{will never be executed}}
+  }
+
+  bool baz() {
+if constexpr (true)
+  return true;
+else {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -299,6 +299,12 @@
 if (isa(Term)) {
   return isConfigurationValue(Term, PP);
 }
+// Do not treat constexpr if statement successors as unreachable in 
warnings
+// since the point of these statements is to determine branches at compile
+// time.
+if (const auto *IS = dyn_cast(Term);
+IS != nullptr && IS->isConstexpr())
+  return true;
   }
 
   const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);


Index: clang/test/SemaCXX/unreachable-code.cpp
===
--- clang/test/SemaCXX/unreachable-code.cpp
+++ clang/test/SemaCXX/unreachable-code.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -Wunreachable-code-aggressive -fblocks -verify %s
+// RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fexceptions -fsyntax-only -Wunreachable-code-aggressive -fblocks -verify %s
 
 int j;
 int bar();
@@ -99,3 +99,34 @@
 }
 
 }
+
+namespace gh57123 {
+  bool foo() {
+if constexpr (true) {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+else
+  return false; // no-warning
+  }
+
+  bool bar() {
+if (true)
+  return true;
+else
+  return false; // expected-warning {{will never be executed}}
+  }
+
+  bool baz() {
+if constexpr (true)
+  return true;
+else {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -299,6 +299,12 @@
 if (isa(Term)) {
   return isConfigurationValue(Term, PP);
 }
+// Do not treat constexpr if statement successors as unreachable in warnings
+// since the point of these statements is to determine branches at compile
+// time.
+if (const auto *IS = dyn_cast(Term);
+IS != nullptr && IS->isConstexpr())
+  return true;
   }
 
   const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131818: [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added a comment.

In D131818#3723227 , @thakis wrote:

> Thanks!
>
> Can you add a test for non-constexpr unreachable code within the constexpr if 
> and test that that still warns?

Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131818

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

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/SemaHLSL/resource_binding_attr_error.hlsl:15
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' 
followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+

aaron.ballman wrote:
> python3kgae wrote:
> > aaron.ballman wrote:
> > > Isn't this a re-definition of `a` which would cause an error?
> > The name for cbuffer is not used.
> > And no error should be reported for re-definition.
> > 
> > Discussed with the team, have to keep it like this for back-compat reason.
> Wow.
> 
> Please either add a comment explaining why this bizarre situation gets no 
> diagnostic, or change the identifiers used so nobody else gets confused by 
> this when reading the test file.
`cbuffer` is really weird (and hopefully someday will disappear entirely). From 
a language perspective it behaves almost more like a named anonymous namespace 
than a datatype (yea, I know that sentence makes no sense).

The functionality that it provides is the ability to group a set of global 
constant variables so that the CPU-code can pack and provide those constants in 
known clumps.

One might ask why have them named at all? Which is a fair question... The names 
do get exposed via the CPU-side reflection API which supports querying bindings 
and the like. We _probably_ shouldn't have ever allowed duplicate names, but as 
long as they have unique "slot" bindings the names don't really matter.

`cbuffers` are important because we try to strip unused resources from shaders 
(because resource binding costs time). A `cbuffer` can either be striped 
entirely or not at all, which prevents individual binding elements from being 
stripped and simplifies shader calling code so the caller doesn't need to 
condition binding every individual resource element.

We have newer structures in the language which can handle all of the use cases 
for `cbuffer`, however there are still some cases (particularly targeting older 
versions of windows), where `cbuffer` is still the dominant technique.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6649
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }

thakis wrote:
> Do we want to do this in the driver? From what I understand, we already do 
> this (MSVC or not) at the Driver->Frontend boundary. See Options.td:
> 
> ```
> defm aligned_allocation : BoolFOption<"aligned-allocation",
>   LangOpts<"AlignedAllocation">, Default,
>   PosFlag,
>   NegFlag, BothFlags<[CC1Option]>>;
> ```
> 
> It defaults to on when c++17 is set, and makes its way to 
> `LangOpts.AlignedAllocation`.
> 
> Does this change here have any effect? From looking at the code, this should 
> be the current behavior already (?)
Hmmm, my test regarding checking for `-faligned-allocation` fails if I remove 
the code here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

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

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 452739.
python3kgae added a comment.

Fix format in AttrDocs.td and avoid name conflict in test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/HLSL/resource_binding_attr.hlsl
  clang/test/SemaHLSL/resource_binding_attr_error.hlsl

Index: clang/test/SemaHLSL/resource_binding_attr_error.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-error@+5 {{expected ';' after top level declarator}}
+// expected-error@+4 {{expected ')'}}
+// expected-note@+3 {{to match this '('}}
+// expected-error@+2 {{a type specifier is required for all declarations}}
+// expected-error@+1 {{illegal storage class on file-scoped variable}}
+float a : register(c0, space1);
+
+// expected-error@+1 {{invalid resource class specifier 'i' used; expected 'b', 's', 't', or 'u'}}
+cbuffer b : register(i0) {
+
+}
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer c : register(b0, s2) {
+
+}
+// expected-error@+1 {{register number should be an integer}}
+cbuffer d : register(bf, s2) {
+
+}
+// expected-error@+1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
+cbuffer e : register(b2, spaces) {
+
+}
+
+// expected-error@+1 {{expected identifier}}
+cbuffer A : register() {}
+
+// expected-error@+1 {{register number should be an integer}}
+cbuffer B : register(space1) {}
+
+// expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}}
+cbuffer C : register(b 2) {}
+
+// expected-error@+2 {{wrong argument format for hlsl attribute, use b2 instead}}
+// expected-error@+1 {{wrong argument format for hlsl attribute, use space3 instead}}
+cbuffer D : register(b 2, space 3) {}
Index: clang/test/AST/HLSL/resource_binding_attr.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:6:9 cbuffer CB
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}}  "b3" "space2"
+// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'float'
+cbuffer CB : register(b3, space2) {
+  float a;
+}
+
+// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:13:9 tbuffer TB
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}}  "t2" "space1"
+// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
+tbuffer TB : register(t2, space1) {
+  float b;
+}
+
+float foo() {
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}}  'float' '+'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[A]] 'a' 'float'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[B]] 'b' 'float'
+  return a + b;
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6934,6 +6934,77 @@
   return HLSLShaderAttr::Create(Context, ShaderType, AL);
 }
 
+static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
+  const ParsedAttr &AL) {
+  StringRef Space = "space0";
+  StringRef Slot = "";
+
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }
+
+  IdentifierLoc *Loc = AL.getArgAsIdent(0);
+  StringRef Str = Loc->Ident->getName();
+  SourceLocation ArgLoc = Loc->Loc;
+
+  SourceLocation SpaceArgLoc;
+  if (AL.getNumArgs() == 2) {
+Slot = Str;
+if (!AL.isArgIdent(1)) {
+  S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+  << AL << AANT_ArgumentIdentifier;
+  return;
+}
+
+IdentifierLoc *Loc = AL.getArgAsIdent(1);
+Space = Loc->Ident->getName();
+SpaceArgLoc = Loc->Loc;
+  } else {
+Slot = Str;
+  }
+
+  // Validate.
+  if (!Slot.empty()) {
+switch (Slot[0]) {
+case 'u':
+case 'b':
+case 's':
+case 't':
+  break;
+default:
+  S.Diag(ArgLoc, diag::err_hlsl_unsupported_register_type)
+  << Slot.substr(0, 1);
+  return;
+}
+
+StringRef SlotNum = Slot.substr(1);
+unsigned Num = 0;
+if (

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 452740.
pscoro added a comment.

chain input fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,206 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX-FAST
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-FAST
+
+attributes #1 = { nounwind }
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() #1 {
+; CHECK-AIX-LABEL: test_kill_canary:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:blr
+;
+; CHECK-LABEL: test_kill_canary:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:blr
+;
+; CHECK-AIX64-LABEL: test_kill_canary:
+; CHECK-AIX64:   # %bb.0: # %entry
+; CHECK-AIX64-NEXT:blr
+;
+; CHECK-LINUX-LE-LABEL: test_kill_canary:
+; CHECK-LINUX-LE:   # %bb.0: # %entry
+; CHECK-LINUX-LE-NEXT:blr
+;
+; CHECK-AIX-FAST-LABEL: test_kill_canary:
+; CHECK-AIX-FAST:   # %bb.0: # %entry
+; CHECK-AIX-FAST-NEXT:blr
+;
+; CHECK-FAST-LABEL: test_kill_canary:
+; CHECK-FAST:   # %bb.0: # %entry
+; CHECK-FAST-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 #1 {
+; CHECK-AIX-LABEL: test_kill_canary_ssp:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:mflr r0
+; CHECK-AIX-NEXT:stw r0, 8(r1)
+; CHECK-AIX-NEXT:stwu r1, -64(r1)
+; CHECK-AIX-NEXT:lwz r3, L..C0(r2) # @__ssp_canary_word
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:not r4, r4
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r3, 0(r3)
+; CHECK-AIX-NEXT:lwz r4, 60(r1)
+; CHECK-AIX-NEXT:cmplw r3, r4
+; CHECK-AIX-NEXT:bne cr0, L..BB1_2
+; CHECK-AIX-NEXT:  # %bb.1: # %entry
+; CHECK-AIX-NEXT:addi r1, r1, 64
+; CHECK-AIX-NEXT:lwz r0, 8(r1)
+; CHECK-AIX-NEXT:mtlr r0
+; CHECK-AIX-NEXT:blr
+; CHECK-AIX-NEXT:  L..BB1_2: # %entry
+; CHECK-AIX-NEXT:bl .__stack_chk_fail[PR]
+; CHECK-AIX-NEXT:nop
+;
+; CHECK-LABEL: test_kill_canary_ssp:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mflr r0
+; CHECK-NEXT:std r0, 16(r1)
+; CHECK-NEXT:stdu r1, -128(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:not r3, r3
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, 120(r1)
+; CHECK-NEXT:ld r4, -28688(r13)
+; CHECK-NEXT:cmpld r4, r3
+; CHECK-NEXT:bne cr0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %entry
+; CHECK-NEXT:addi r1, r1, 128
+; CHECK-NEXT:ld r0, 16(r1)
+; CHECK-NEXT:mtlr r0
+; CHECK-NE

[clang] f37b285 - Removing an unused function; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-08-15T13:35:16-04:00
New Revision: f37b2852999c695af03c6d809816793d8d345538

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

LOG: Removing an unused function; NFC

It turns out there are zero in-tree callers of CallExpr::getNumCommas()
so it's reasonable to remove.

Added: 


Modified: 
clang/include/clang/AST/Expr.h

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 1fc6dc90b6c7c..d8efe663ce8ee 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3061,10 +3061,6 @@ class CallExpr : public Expr {
   PREARGS_START + getNumPreArgs() + getNumArgs());
   }
 
-  /// getNumCommas - Return the number of commas that must have been present in
-  /// this function call.
-  unsigned getNumCommas() const { return getNumArgs() ? getNumArgs() - 1 : 0; }
-
   /// Get FPOptionsOverride from trailing storage.
   FPOptionsOverride getStoredFPFeatures() const {
 assert(hasStoredFPFeatures());



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

[clang] 465d908 - [test][clang] Opaquify pragma-init_seg.cpp

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-08-15T10:45:24-07:00
New Revision: 465d9084ece1626335bc41eac96c56428faed7e9

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

LOG: [test][clang] Opaquify pragma-init_seg.cpp

Added: 


Modified: 
clang/test/CodeGenCXX/pragma-init_seg.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/pragma-init_seg.cpp 
b/clang/test/CodeGenCXX/pragma-init_seg.cpp
index a35b16baf4228..187a06a0c0f0c 100644
--- a/clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ b/clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers %s -triple=i686-pc-win32 
-fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
 
 int f();
 
@@ -10,12 +10,12 @@ namespace simple_init {
 #pragma init_seg(compiler)
 int x = f();
 // CHECK: @"?x@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr = private constant void ()* 
@"??__Ex@simple_init@@YAXXZ", section ".CRT$XCC"
+// CHECK: @__cxx_init_fn_ptr = private constant ptr 
@"??__Ex@simple_init@@YAXXZ", section ".CRT$XCC"
 
 #pragma init_seg(lib)
 int y = f();
 // CHECK: @"?y@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.1 = private constant void ()* 
@"??__Ey@simple_init@@YAXXZ", section ".CRT$XCL"
+// CHECK: @__cxx_init_fn_ptr.1 = private constant ptr 
@"??__Ey@simple_init@@YAXXZ", section ".CRT$XCL"
 
 #pragma init_seg(user)
 int z = f();
@@ -29,14 +29,14 @@ namespace internal_init {
 namespace {
 int x = f();
 // CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, 
align 4
-// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* 
@"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
+// CHECK: @__cxx_init_fn_ptr.2 = private constant ptr 
@"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
 }
 }
 
 namespace selectany_init {
 int __declspec(selectany) x = f();
 // CHECK: @"?x@selectany_init@@3HA" = weak_odr dso_local global i32 0, comdat, 
align 4
-// CHECK: @__cxx_init_fn_ptr.3 = private constant void ()* 
@"??__Ex@selectany_init@@YAXXZ", section ".asdf", 
comdat($"?x@selectany_init@@3HA")
+// CHECK: @__cxx_init_fn_ptr.3 = private constant ptr 
@"??__Ex@selectany_init@@YAXXZ", section ".asdf", 
comdat($"?x@selectany_init@@3HA")
 }
 
 namespace explicit_template_instantiation {
@@ -44,7 +44,7 @@ template  struct A { static const int x; };
 template  const int A::x = f();
 template struct A;
 // CHECK: @"?x@?$A@H@explicit_template_instantiation@@2HB" = weak_odr 
dso_local global i32 0, comdat, align 4
-// CHECK: @__cxx_init_fn_ptr.4 = private constant void ()* 
@"??__E?x@?$A@H@explicit_template_instantiation@@2HB@@YAXXZ", section ".asdf", 
comdat($"?x@?$A@H@explicit_template_instantiation@@2HB")
+// CHECK: @__cxx_init_fn_ptr.4 = private constant ptr 
@"??__E?x@?$A@H@explicit_template_instantiation@@2HB@@YAXXZ", section ".asdf", 
comdat($"?x@?$A@H@explicit_template_instantiation@@2HB")
 }
 
 namespace implicit_template_instantiation {
@@ -52,21 +52,21 @@ template  struct A { static const int x; };
 template  const int A::x = f();
 int g() { return A::x; }
 // CHECK: @"?x@?$A@H@implicit_template_instantiation@@2HB" = linkonce_odr 
dso_local global i32 0, comdat, align 4
-// CHECK: @__cxx_init_fn_ptr.5 = private constant void ()* 
@"??__E?x@?$A@H@implicit_template_instantiation@@2HB@@YAXXZ", section ".asdf", 
comdat($"?x@?$A@H@implicit_template_instantiation@@2HB")
+// CHECK: @__cxx_init_fn_ptr.5 = private constant ptr 
@"??__E?x@?$A@H@implicit_template_instantiation@@2HB@@YAXXZ", section ".asdf", 
comdat($"?x@?$A@H@implicit_template_instantiation@@2HB")
 }
 
 // ... and here's where we emitted user level ctors.
-// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }]
-// CHECK: [{ i32, void ()*, i8* } { i32 65535, void ()* 
@_GLOBAL__sub_I_pragma_init_seg.cpp, i8* null }]
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }]
+// CHECK: [{ i32, ptr, ptr } { i32 65535, ptr 
@_GLOBAL__sub_I_pragma_init_seg.cpp, ptr null }]
 
 // We have to mark everything used so we can survive globalopt, even through
 // LTO.  There's no way LLVM could really understand if data in the .asdf
 // section is really used or dead.
 //
-// CHECK: @llvm.used = appending global [6 x i8*]
-// CHECK: [i8* bitcast (void ()** @__cxx_init_fn_ptr to i8*),
-// CHECK: i8* bitcast (void ()** @__cxx_init_fn_ptr.1 to i8*),
-// CHECK: i8* bitcast (void ()** @__cxx_init_fn_ptr.2 to i8*),
-// CHECK: i8* bitcast (void ()** @__cxx_init_fn_ptr.3 to i8*),
-// CHECK: i8* bitcast (void ()** @__cxx_init_fn_ptr.4 to i8*),
-// CHECK: i8* bitcast (void ()** @__cxx_init_fn_ptr.5 to i8*)], section 
"llvm.metadata"
+// CHECK: @ll

[PATCH] D128133: [Driver] Support linking to compiler-rt for target AVR

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

We started seeing test failures after this change on our bots. Would it be 
possible to revert the change and address these issues before relanding?

In `Clang :: Driver/avr-ld.c`:

  Script:
  --
  : 'RUN: at line 1';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=at90s2313 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKA 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 4';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=at90s8515 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKB 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 7';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=attiny13 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKC 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 10';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=attiny44 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKD 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 13';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega103 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKE 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 16';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega8u2 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKF 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 19';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega48pa --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKG 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 22';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega328 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKH 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 25';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega1281 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKI 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 28';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atmega2560 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKJ 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 31';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=attiny10 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKK 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 34';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atxmega16a4 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck -check-prefix LINKL 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c
  : 'RUN: at line 37';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -### 
--target=avr -mmcu=atxmega64b3 --sysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/basic_avr_tree 
/b/s/w/ir/x/w/llv

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this fix! I think it's quite close to finished, but it 
needs some additional test coverage. Also, please add a release note about the 
fix so users know what's going on.




Comment at: clang/lib/Sema/SemaExpr.cpp:13978-13980
+  if (const CallExpr *CE = dyn_cast(E))
+if (const Type *T = CE->getCallReturnType(Context).getTypePtrOrNull())
+  return T->isVoidType();





Comment at: clang/test/SemaCXX/warn-comma-operator.cpp:151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+

Can you also add test cases like:
```
struct S {
  void mem();
};

typedef void Void;
Void typedef_func();

void whatever() {
  S s;
  
  // Member function calls also work as expected.
  s.mem(), int_func();
  // As do lambda calls.
  []() { return; }(), int_func();
  // And we don't get confused about type aliases.
  typedef_func(), int_func();
  // Even function pointers don't confuse us.
  void (*fp)() = void_func;
  fp(), int_func();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Abid via Phabricator via cfe-commits]

abidmalikwaterloo updated this revision to Diff 452743.
abidmalikwaterloo added a comment.

[MLIR][OpenMP] Added target data, exit data, and enter data operation 
definition for MLIR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

Files:
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td


Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -625,7 +625,7 @@
  Variadic:$map_type_modifier_operands);

   let regions = (region AnyRegion:$region); 
-   
+  
   let assemblyFormat = [{
 (`if` `(` $if_expr^ `)` )?
 (`device` `(` $device^ `:` type($device) `)` )?


Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -625,7 +625,7 @@
  Variadic:$map_type_modifier_operands);

   let regions = (region AnyRegion:$region); 
-   
+  
   let assemblyFormat = [{
 (`if` `(` $if_expr^ `)` )?
 (`device` `(` $device^ `:` type($device) `)` )?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: owenpan, MyDeveloperDay.
aaron.ballman added a comment.

In D131386#3723490 , @Mordante wrote:

> In D131386#3722749 , @aaron.ballman 
> wrote:
>
>> We leave formatting decisions in clang-tidy to clang-format and I don't 
>> think we should deviate from that policy here without a very clear 
>> understanding of when we should relax that restriction. That said, I'm 
>> personally not certain we should have such an option (the long-term goal has 
>> generally been to integrate clang-format functionality into clang-tidy so 
>> there can be an option to just run format after applying fixes in a TU). Is 
>> there a compelling reason we should have it?
>
> The reason for me to work on this feature is the fact that clang-format 
> doesn't work reliable for changing the location of the `const`. After using 
> clang-tidy and clang-format some `const`s were still one the right hand side. 
> I didn't do a deep investigation, but it seems to affect classes in 
> namespaces. So some occurrences of `std::string` were affected, but not all. 
> So in the current state I can't use this clang-tidy fix.

CC @MyDeveloperDay and @owenpan for awareness.

> Since clang-format doesn't fully parse the code I wonder whether all cases 
> can be properly fixed. On the other hand clang-tidy has all information 
> available.

Mostly. clang-tidy will miss everything in a preprocessor conditional block, as 
an example of what it can't handle but clang-format can.

> Would integrating clang-format in clang-tidy mean clang-format will use the 
> full information of the AST?

I wouldn't imagine that to be the case (that'd be a pretty big change to the 
architecture of clang-format; basically a rewrite). I would expect it to be 
integrated more as a library to call into to perform the post-fix formatting to 
just specific ranges of code. However, it is interesting to think about the 
fact that such a library could potentially accept an optional AST node 
representing the root of the AST represented by the text as a way to help it 
disambiguate situations it couldn't otherwise figure out.

In D131386#3723254 , @carlosgalvezp 
wrote:

> FWIW, there are already other checks that have formatting settings, for 
> example:
> https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-bounds-constant-array-index.html#cmdoption-arg-includestyle

Those docs are pretty terrible because I have no idea what an "llvm-style" 
include even *is*.

IIRC, we added this because we added the include sorter to clang-tidy and at 
the time, it was deemed unsafe to add to clang-format because of the likelihood 
of breaking code. IMO, it's on the borderline as to whether we should have 
added it or not -- it's the same situation I was worried about above where we 
add a feature to clang-tidy and then clang-format later gets the ability to 
perform the formatting and so the two gradually drift out of sync. Then again, 
it seems our checks have drifted out of sync with what IncludeSorter supports 
because there's a third option for google Objective C formatting style 
(whatever that is).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105255#3723865 , 
@abidmalikwaterloo wrote:

> [MLIR][OpenMP] Added target data, exit data, and enter data operation 
> definition for MLIR

Looks like it's not the complete revision. There are almost no diffs anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Abid via Phabricator via cfe-commits]

abidmalikwaterloo added a comment.

In D105255#3723940 , @clementval 
wrote:

> In D105255#3723865 , 
> @abidmalikwaterloo wrote:
>
>> [MLIR][OpenMP] Added target data, exit data, and enter data operation 
>> definition for MLIR
>
> Looks like it's not the complete revision. There are almost no diffs anymore.

My OS system crashed last week. I am working on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D131910: [Windows] Put init_seg(compiler/lib) in llvm.global_ctors

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added reviewers: rnk, hans.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Currently we treat initializers with init_seg(compiler/lib) as similar
to any other init_seg, they simply have a global variable in the proper
section (".CRT$XCC" for compiler/".CRT$XCL" for lib) and are added to
llvm.used. However, this doesn't match with how LLVM sees normal (or
init_seg(user)) initializers via llvm.global_ctors. This
causes issues like incorrect init_seg(compiler) vs init_seg(user)
ordering due to GlobalOpt evaluating constructors, and the
ability to remove init_seg(compiler/lib) initializers at all.

To preserve the existing uses of 'A' for priorities less than 200 and
otherwise 'T' as much as possible, use priority 200 for
init_seg(compiler) and 201 for init_seg(lib), which use 'C' and 'L'.

Fixes #56922


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131910

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/test/CodeGenCXX/pragma-init_seg.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/ctor-priority-coff.ll

Index: llvm/test/CodeGen/X86/ctor-priority-coff.ll
===
--- llvm/test/CodeGen/X86/ctor-priority-coff.ll
+++ llvm/test/CodeGen/X86/ctor-priority-coff.ll
@@ -6,6 +6,12 @@
 ; CHECK: .section.CRT$XCA00042,"dr"
 ; CHECK: .p2align3
 ; CHECK: .quad   f
+; CHECK: .section.CRT$XCC00200,"dr"
+; CHECK: .p2align3
+; CHECK: .quad   i
+; CHECK: .section.CRT$XCL00201,"dr"
+; CHECK: .p2align3
+; CHECK: .quad   j
 ; CHECK: .section.CRT$XCT12345,"dr"
 ; CHECK: .p2align3
 ; CHECK: .quad   g
@@ -24,10 +30,12 @@
 @str1 = private dso_local unnamed_addr constant [6 x i8] c"first\00", align 1
 @str2 = private dso_local unnamed_addr constant [5 x i8] c"main\00", align 1
 
-@llvm.global_ctors = appending global [3 x { i32, ptr, ptr }] [
+@llvm.global_ctors = appending global [5 x { i32, ptr, ptr }] [
   { i32, ptr, ptr } { i32 12345, ptr @g, ptr null },
   { i32, ptr, ptr } { i32 42, ptr @f, ptr null },
-  { i32, ptr, ptr } { i32 23456, ptr @init_h, ptr @h }
+  { i32, ptr, ptr } { i32 23456, ptr @init_h, ptr @h },
+  { i32, ptr, ptr } { i32 200, ptr @i, ptr null },
+  { i32, ptr, ptr } { i32 201, ptr @j, ptr null }
 ]
 
 declare dso_local i32 @puts(ptr nocapture readonly) local_unnamed_addr
@@ -50,6 +58,18 @@
   ret void
 }
 
+define dso_local void @i() {
+entry:
+  store i8 43, ptr @h
+  ret void
+}
+
+define dso_local void @j() {
+entry:
+  store i8 44, ptr @h
+  ret void
+}
+
 
 ; Function Attrs: nounwind uwtable
 define dso_local i32 @main() local_unnamed_addr {
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1889,11 +1889,21 @@
 // string that sorts between .CRT$XCA and .CRT$XCU. In the general case, we
 // make a name like ".CRT$XCT12345", since that runs before .CRT$XCU. Really
 // low priorities need to sort before 'L', since the CRT uses that
-// internally, so we use ".CRT$XCA1" for them.
+// internally, so we use ".CRT$XCA1" for them. We have a contract with
+// the frontend that "init_seg(compiler)" corresponds to priority 200 and
+// "init_seg(lib)" corresponds to priority 201, and those respectively use
+// 'C' and 'L'.
 SmallString<24> Name;
+char LastLetter = 'T';
+if (Priority == 201)
+  LastLetter = 'L';
+else if (Priority == 200)
+  LastLetter = 'C';
+else if (Priority < 200)
+  LastLetter = 'A';
 raw_svector_ostream OS(Name);
-OS << ".CRT$X" << (IsCtor ? "C" : "T") <<
-(Priority < 200 ? 'A' : 'T') << format("%05u", Priority);
+OS << ".CRT$X" << (IsCtor ? "C" : "T") << LastLetter
+   << format("%05u", Priority);
 MCSectionCOFF *Sec = Ctx.getCOFFSection(
 Name, COFF::IMAGE_SCN_CNT_INITIALIZED_DATA | COFF::IMAGE_SCN_MEM_READ,
 SectionKind::getReadOnly());
Index: clang/test/CodeGenCXX/pragma-init_seg.cpp
===
--- clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -10,12 +10,12 @@
 #pragma init_seg(compiler)
 int x = f();
 // CHECK: @"?x@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr = private constant ptr @"??__Ex@simple_init@@YAXXZ", section ".CRT$XCC"
+// No function pointer!  This one goes on @llvm.global_ctors.
 
 #pragma init_seg(lib)
 int y = f();
 // CHECK: @"?y@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.1 = private constant ptr @"??__Ey@simple_init@@YAXXZ", secti

[PATCH] D129464: [Clang][CodeGen] Set FP options of builder at entry to compound statement

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

As specific interaction of FP pragmas and template instantiation was not 
supported, this patch is actual without additional changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129464

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

[PATCH] D131789: [clang-tools-extra][NFC] Rewrite prints in python3 compatible way

[Danil Sidoruk via Phabricator via cfe-commits]

eoanermine added a comment.

Please, commit this patch for me

I don't have an access to commit the patch by myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131789

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

[PATCH] D131818: [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Nico Weber via Phabricator via cfe-commits]

thakis accepted this revision.
thakis 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/D131818/new/

https://reviews.llvm.org/D131818

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 452757.
steplong added a comment.

- Clang-format test

The regex isn't optimal since it will still accept strings that aren't "Num 
Args: 1\n" or "Num Args: 2\n" such as "Num Args: \n2\n"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

Files:
  clang-tools-extra/test/clang-tidy/checkers/misc/new-delete-overloads.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-zc.cpp
  clang/unittests/StaticAnalyzer/CallEventTest.cpp

Index: clang/unittests/StaticAnalyzer/CallEventTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -20,8 +20,11 @@
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+using testing::MatchesRegex;
+
 namespace clang {
 namespace ento {
 namespace {
@@ -81,7 +84,12 @@
 }
   )",
  Diags));
-  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+// Sized deallocation is enabled on Windows, so we expect two arguments.
+#if defined(GTEST_USES_POSIX_RE)
+  EXPECT_THAT(Diags, MatchesRegex("test.CXXDeallocator: NumArgs: (1|2)\n"));
+#elif defined(GTEST_USES_SIMPLE_RE)
+  EXPECT_THAT(Diags, MatchesRegex("test.CXXDeallocator: NumArgs: 1?\n?2?\n?"));
+#endif
 }
 
 } // namespace
Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,19 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC %s
+// RUN: %clang_cl /c -### -fms-compatibility-version=18 -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC-OFF %s
+// SIZED-DEALLOC: "-fsized-deallocation"
+// SIZED-DEALLOC-OFF-NOT: "-fsized-deallocation"
+
+// RUN: %clang_cl /c /std:c++11 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++14 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++20 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++latest -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// ALIGNED-NEW-BEFORE-CPP17-NOT: "-faligned-allocation"
+// ALIGNED-NEW-CPP17ONWARDS: "-faligned-allocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6523,6 +6523,7 @@
 }
 CmdArgs.push_back(LanguageStandard.data());
   }
+  bool IsCPP17Onwards = false;
   if (ImplyVCPPCXXVer) {
 StringRef LanguageStandard;
 if (const Arg *StdArg = Args.getLastArg(options::OPT__SLASH_std)) {
@@ -6546,6 +6547,9 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+IsCPP17Onwards =
+LanguageStandard != "-std=c++11" && LanguageStandard != "-std=c++14";
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6675,9 +6679,15 @@
 options::OPT_fno_relaxed_template_template_args);
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
-  // most platforms.
-  Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
-options::OPT_fno_sized_deallocation);
+  // most platforms. MSVC turns on /Zc:sizedDealloc by default, starting in
+  // MSVC 2015.
+  if (IsWindowsMSVC && IsMSVC2015Compatible &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+  else
+Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
+  options::OPT_fno_sized_deallocation);
 
   // -faligned-allocation is on by default in C++17 onwards and otherwise off
   // by default.
@@ -6688,6 +6698,8 @@
   CmdArgs.push_back("-fno-aligned-allocation");
 else
   CmdArgs.push_back("-faligned-allocation");
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }
 
   // The default new alignment can be specified using a dedicated option or via
Index: clang-tools-extra/test/clang-tidy/checkers/misc/new-delete-overloads.cpp

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 452761.
python3kgae marked 3 inline comments as done.
python3kgae added a comment.

Add more comment about no name conflict for cbuffer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaHLSL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/test/AST/HLSL/cbuffer_tbuffer.hlsl
  clang/test/SemaHLSL/cb_error.hlsl

Index: clang/test/SemaHLSL/cb_error.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/cb_error.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-error@+2 {{expected identifier}}
+// expected-error@+1 {{expected unqualified-id}}
+cbuffer { ... };
+// expected-error@+1 {{expected '{'}}
+cbuffer missing_definition;
+// expected-error@+1 {{expected unqualified-id}}
+int cbuffer;
+// expected-error@+1 {{expected identifier}}
+cbuffer;
+
+// expected-error@+2 {{expected identifier}}
+// expected-error@+1 {{expected unqualified-id}}
+tbuffer { ... };
+// expected-error@+1 {{expected '{'}}
+tbuffer missing_definition;
+// expected-error@+1 {{expected unqualified-id}}
+int tbuffer;
+// expected-error@+1 {{expected identifier}}
+tbuffer;
Index: clang/test/AST/HLSL/cbuffer_tbuffer.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/cbuffer_tbuffer.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:5:9 cbuffer CB
+// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'float'
+cbuffer CB {
+  float a;
+}
+
+// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:11:9 tbuffer TB
+// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
+tbuffer TB {
+  float b;
+}
+
+float foo() {
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}}  'float' '+'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[A]] 'a' 'float'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[B]] 'b' 'float'
+  return a + b;
+}
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -433,6 +433,7 @@
   case Decl::LifetimeExtendedTemporary:
   case Decl::RequiresExprBody:
   case Decl::UnresolvedUsingIfExists:
+  case Decl::HLSLBuffer:
 return false;
 
   // These indirectly derive from Redeclarable but are not actually
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -873,6 +873,10 @@
   llvm_unreachable("Translation units cannot be instantiated");
 }
 
+Decl *TemplateDeclInstantiator::VisitHLSLBufferDecl(HLSLBufferDecl *Decl) {
+  llvm_unreachable("HLSL buffer declarations cannot be instantiated");
+}
+
 Decl *
 TemplateDeclInstantiator::VisitPragmaCommentDecl(PragmaCommentDecl *D) {
   llvm_unreachable("pragma comment cannot be instantiated");
Index: clang/lib/Sema/SemaHLSL.cpp
===
--- /dev/null
+++ clang/lib/Sema/SemaHLSL.cpp
@@ -0,0 +1,37 @@
+//===- SemaHLSL.cpp - Semantic Analysis for HLSL constructs ---===//
+//
+// 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
+//
+//===--===//
+// This implements Semantic Analysis for HLSL constructs.
+//===--===//
+
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+Decl *Sema::ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
+ SourceLocation KwLoc, IdentifierInfo *Ident,
+ S

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/test/SemaHLSL/resource_binding_attr_error.hlsl:15
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' 
followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+

beanz wrote:
> aaron.ballman wrote:
> > python3kgae wrote:
> > > aaron.ballman wrote:
> > > > Isn't this a re-definition of `a` which would cause an error?
> > > The name for cbuffer is not used.
> > > And no error should be reported for re-definition.
> > > 
> > > Discussed with the team, have to keep it like this for back-compat reason.
> > Wow.
> > 
> > Please either add a comment explaining why this bizarre situation gets no 
> > diagnostic, or change the identifiers used so nobody else gets confused by 
> > this when reading the test file.
> `cbuffer` is really weird (and hopefully someday will disappear entirely). 
> From a language perspective it behaves almost more like a named anonymous 
> namespace than a datatype (yea, I know that sentence makes no sense).
> 
> The functionality that it provides is the ability to group a set of global 
> constant variables so that the CPU-code can pack and provide those constants 
> in known clumps.
> 
> One might ask why have them named at all? Which is a fair question... The 
> names do get exposed via the CPU-side reflection API which supports querying 
> bindings and the like. We _probably_ shouldn't have ever allowed duplicate 
> names, but as long as they have unique "slot" bindings the names don't really 
> matter.
> 
> `cbuffers` are important because we try to strip unused resources from 
> shaders (because resource binding costs time). A `cbuffer` can either be 
> striped entirely or not at all, which prevents individual binding elements 
> from being stripped and simplifies shader calling code so the caller doesn't 
> need to condition binding every individual resource element.
> 
> We have newer structures in the language which can handle all of the use 
> cases for `cbuffer`, however there are still some cases (particularly 
> targeting older versions of windows), where `cbuffer` is still the dominant 
> technique.
Changed the names to avoid confusion.
More comment is added to https://reviews.llvm.org/D129883


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

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

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 452767.
inclyc added a comment.

Address comments.

Thanks a lot for your suggestion, I noticed that the regression test tested both
C and C++, so I split the test mentioned in the comment into two parts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp

Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,27 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void void_function_comma(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
+typedef void Void;
+Void typedef_func();
+
+void whatever() {
+  // We don't get confused about type aliases.
+  typedef_func(), int_func();
+  // Even function pointers don't confuse us.
+  void (*fp)() = void_func;
+  fp(), int_func();
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
@@ -296,4 +317,23 @@
   (void)T{}, 0;
   static_cast(T{}), 0;
 }
+
+namespace {
+
+// issue #57151
+
+struct S {
+  void mem() {}
+};
+
+void whatever() {
+  struct S s;
+  // Member function calls also work as expected.
+  s.mem(), int_func();
+  // As do lambda calls.
+  []() { return; }(), int_func();
+}
+
+} // namespace
+
 #endif  // ifdef __cplusplus
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returns void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,8 @@
 }
   }
 
+  if (const auto *CE = dyn_cast(E))
+return CE->getCallReturnType(Context)->isVoidType();
   return false;
 }
 
@@ -14014,7 +14018,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -71,6 +71,8 @@
 - Fix `#57008 `_ - Builtin
   C++ language extension type traits instantiated by a template with unexpected
   number of arguments cause an assertion fault.
+- Fix `#57151 `_.
+  ``-Wcomma`` is emitted for void returning functions.
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:74
   number of arguments cause an assertion fault.
+- Fix `#57151 `_.
+  ``-Wcomma`` is emitted for void returning functions.

Do we need an NFC commit to unify the format of issues mentioned here? (Maybe 
it can be scheduled for a later release). The different ways of citing issues 
here can be confusing.  `Issue ` `# xxx` in this context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

___
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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: hans.
rnk added a comment.

Regarding the usage of isPOD in the MSVC ABI code, we should have a routine 
similar to `isTrivialForAArch64MSVC` that works for the other MSVC 
architectures that we support (x86, x64, and arm32). I don't think we should 
try to rely on definition data bits. Clang is almost always tracking something 
slightly different than what MSVC tracks, so we should write down MSVC's logic 
once and keep it separate from Clang's notion of POD.

I think you can work out MSVC's behavior with godbolt, but if you need a hand, 
I volunteer @hans to take a stab at refactoring 
`MicrosoftCXXABI::classifyReturnType`. I remember making lots of comments to 
try to simplify the code during review, but everyone is always wary of scope 
creep.


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] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 452775.
shafik marked an inline comment as done.
shafik added a comment.

- Changed `NotConstexprVar` to `ConstExprVar`
- Moved checking of `ConstexprVar` up so that we do the less expensive check 
first


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

https://reviews.llvm.org/D131874

Files:
  clang/lib/AST/ExprConstant.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
@@ -2419,6 +2419,13 @@
 
 enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
 
+enum NumberType {};
+
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant 
expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
   constexpr E1 x2 = static_cast(8);
@@ -2454,6 +2461,8 @@
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range 
of values [-2147483648, 2147483647] for this enumeration type}}
+
+  const NumberType neg_one = (NumberType) ((NumberType) 0 - (NumberType) 1); 
// ok, not a constant expression context
 }
 
 enum SortOrder {
@@ -2470,3 +2479,8 @@
 return;
 }
 }
+
+GH50055::E2 GlobalInitNotCE1 = (GH50055::E2)-1; // ok, not a constant 
expression context
+GH50055::E2 GlobalInitNotCE2 = GH50055::testDefaultArgForParam(); // ok, not a 
constant expression context
+constexpr GH50055::E2 GlobalInitCE = (GH50055::E2)-1;
+// expected-error@-1 {{integer value -1 is outside the valid range of values 
[0, 7] for this enumeration type}}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13536,6 +13536,20 @@
 if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
 Info.EvalMode == EvalInfo::EM_ConstantExpression &&
 DestType->isEnumeralType()) {
+
+  bool ConstexprVar = true;
+
+  // We know if we are here that we are in a context that we might require
+  // a constant expression or a context that requires a constant
+  // value. But if we are initializing a value we don't know if it is a
+  // constexpr variable or not. We can check the EvaluatingDecl to 
determine
+  // if it constexpr or not. If not then we don't want to emit a 
diagnostic.
+  if (auto ValD = Info.EvaluatingDecl.dyn_cast()) {
+const VarDecl *VD = dyn_cast_or_null(ValD);
+if (VD && !VD->isConstexpr())
+  ConstexprVar = false;
+  }
+
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();
   // Check that the value is within the range of the enumeration values.
@@ -13555,13 +13569,14 @@
 ED->getValueRange(Max, Min);
 --Max;
 
-if (ED->getNumNegativeBits() &&
+if (ED->getNumNegativeBits() && ConstexprVar &&
 (Max.slt(Result.getInt().getSExtValue()) ||
  Min.sgt(Result.getInt().getSExtValue(
-  Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-   
diag::warn_constexpr_unscoped_enum_out_of_range)
-  << llvm::toString(Result.getInt(),10) << Min.getSExtValue() << 
Max.getSExtValue();
-else if (!ED->getNumNegativeBits() &&
+  Info.Ctx.getDiagnostics().Report(
+  E->getExprLoc(), diag::warn_constexpr_unscoped_enum_out_of_range)
+  << llvm::toString(Result.getInt(), 10) << Min.getSExtValue()
+  << Max.getSExtValue();
+else if (!ED->getNumNegativeBits() && ConstexprVar &&
  Max.ult(Result.getInt().getZExtValue()))
   Info.Ctx.getDiagnostics().Report(E->getExprLoc(),

diag::warn_constexpr_unscoped_enum_out_of_range)


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2419,6 +2419,13 @@
 
 enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
 
+enum NumberType {};
+
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
   constexpr E1 x2 = static_cast(8);
@@ -2454,6 +2461,8 @@
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551
+  if (auto ValD = Info.EvaluatingDecl.dyn_cast()) {
+const VarDecl *VD = dyn_cast_or_null(ValD);
+if (VD && !VD->isConstexpr())
+  NotConstexprVar = true;
+  }

aaron.ballman wrote:
> This seems to be equivalent unless I'm misunderstanding something about the 
> member dyn_cast.
I think the problem is that `PointerUnion` requires that it be one of the 
static types it was defined with and in this case that is `const ValueDecl *, 
const Expr *` but maybe I am missing something.



Comment at: clang/lib/AST/ExprConstant.cpp:13572-13575
 if (ED->getNumNegativeBits() &&
 (Max.slt(Result.getInt().getSExtValue()) ||
- Min.sgt(Result.getInt().getSExtValue(
-  Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-   
diag::warn_constexpr_unscoped_enum_out_of_range)
-  << llvm::toString(Result.getInt(),10) << Min.getSExtValue() << 
Max.getSExtValue();
+ Min.sgt(Result.getInt().getSExtValue())) &&
+!NotConstexprVar)

aaron.ballman wrote:
> Might as well test the easy condition before doing more work to get values 
> and compare them.
> 
> I think we should rename `NotConstexprVar` to `ConstexprVar` so that we don't 
> need the double negation here. WDYT?
Makes sense.



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2424-2427
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant 
expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}

aaron.ballman wrote:
> Do you think it's worth it to add this as a test as well?
> ```
> consteval void testDefaultArgForParam2(E2 e2Param = (E2)-1) {
> }
> 
> void test() {
>   testDefaultArgForParam2(); // Make sure we get the error
>   testDefaultArgForParam2((E2)0); // Make sure we don't get the error
> }
> ```
It is a good idea but the diagnostic does not point to the line in `test()` 
which is unfortunate. 


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

https://reviews.llvm.org/D131874

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

[clang] ff8aadf - [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Alan Zhao via cfe-commits]

Author: Alan Zhao
Date: 2022-08-15T15:24:39-04:00
New Revision: ff8aadf58d1a0ea7d8fa2b9a7a17ff0c6059baa5

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

LOG: [clang][diagnostics] Don't warn about unreachable code in constexpr if

The point of a constexpr if statement is to determine which branch to
take at compile time, so warning on unreachable code is meaningless in
these situations.

Fixes #57123.

Reviewed By: thakis

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

Added: 


Modified: 
clang/lib/Analysis/ReachableCode.cpp
clang/test/SemaCXX/unreachable-code.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ReachableCode.cpp 
b/clang/lib/Analysis/ReachableCode.cpp
index 652bef34cebb7..ce0f4f83ba04f 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -299,6 +299,12 @@ static bool shouldTreatSuccessorsAsReachable(const 
CFGBlock *B,
 if (isa(Term)) {
   return isConfigurationValue(Term, PP);
 }
+// Do not treat constexpr if statement successors as unreachable in 
warnings
+// since the point of these statements is to determine branches at compile
+// time.
+if (const auto *IS = dyn_cast(Term);
+IS != nullptr && IS->isConstexpr())
+  return true;
   }
 
   const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);

diff  --git a/clang/test/SemaCXX/unreachable-code.cpp 
b/clang/test/SemaCXX/unreachable-code.cpp
index 6a95f767bef02..410ed9ae8861b 100644
--- a/clang/test/SemaCXX/unreachable-code.cpp
+++ b/clang/test/SemaCXX/unreachable-code.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
+// RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
 
 int j;
 int bar();
@@ -99,3 +99,34 @@ void f(int a) {
 }
 
 }
+
+namespace gh57123 {
+  bool foo() {
+if constexpr (true) {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+else
+  return false; // no-warning
+  }
+
+  bool bar() {
+if (true)
+  return true;
+else
+  return false; // expected-warning {{will never be executed}}
+  }
+
+  bool baz() {
+if constexpr (true)
+  return true;
+else {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+  }
+}



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

[PATCH] D131818: [clang][diagnostics] Don't warn about unreachable code in constexpr if

[Alan Zhao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff8aadf58d1a: [clang][diagnostics] Don't warn about 
unreachable code in constexpr if (authored by ayzhao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131818

Files:
  clang/lib/Analysis/ReachableCode.cpp
  clang/test/SemaCXX/unreachable-code.cpp


Index: clang/test/SemaCXX/unreachable-code.cpp
===
--- clang/test/SemaCXX/unreachable-code.cpp
+++ clang/test/SemaCXX/unreachable-code.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
+// RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fexceptions -fsyntax-only 
-Wunreachable-code-aggressive -fblocks -verify %s
 
 int j;
 int bar();
@@ -99,3 +99,34 @@
 }
 
 }
+
+namespace gh57123 {
+  bool foo() {
+if constexpr (true) {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+else
+  return false; // no-warning
+  }
+
+  bool bar() {
+if (true)
+  return true;
+else
+  return false; // expected-warning {{will never be executed}}
+  }
+
+  bool baz() {
+if constexpr (true)
+  return true;
+else {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -299,6 +299,12 @@
 if (isa(Term)) {
   return isConfigurationValue(Term, PP);
 }
+// Do not treat constexpr if statement successors as unreachable in 
warnings
+// since the point of these statements is to determine branches at compile
+// time.
+if (const auto *IS = dyn_cast(Term);
+IS != nullptr && IS->isConstexpr())
+  return true;
   }
 
   const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);


Index: clang/test/SemaCXX/unreachable-code.cpp
===
--- clang/test/SemaCXX/unreachable-code.cpp
+++ clang/test/SemaCXX/unreachable-code.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -Wunreachable-code-aggressive -fblocks -verify %s
+// RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fexceptions -fsyntax-only -Wunreachable-code-aggressive -fblocks -verify %s
 
 int j;
 int bar();
@@ -99,3 +99,34 @@
 }
 
 }
+
+namespace gh57123 {
+  bool foo() {
+if constexpr (true) {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+else
+  return false; // no-warning
+  }
+
+  bool bar() {
+if (true)
+  return true;
+else
+  return false; // expected-warning {{will never be executed}}
+  }
+
+  bool baz() {
+if constexpr (true)
+  return true;
+else {
+  if (true)
+return true;
+  else
+return false; // expected-warning {{will never be executed}}
+}
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -299,6 +299,12 @@
 if (isa(Term)) {
   return isConfigurationValue(Term, PP);
 }
+// Do not treat constexpr if statement successors as unreachable in warnings
+// since the point of these statements is to determine branches at compile
+// time.
+if (const auto *IS = dyn_cast(Term);
+IS != nullptr && IS->isConstexpr())
+  return true;
   }
 
   const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131809: [clang][dataflow] Add an option for context-sensitive depth

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:210
+ const DeclContext *Callee) const {
+  return CallString.size() <= MaxDepth &&
+ std::find(CallString.begin(), CallString.end(), Callee) ==

sgatev wrote:
> If `canDescend` is supposed to return false for `MaxDepth = 0`, shouldn't 
> this be `<`?
I don't follow; could you clarify? The `CallStack` should always be nonempty.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:665
+if (!(Options.ContextSensitiveOpts &&
+  Env.canDescend(Options.ContextSensitiveOpts->Depth, F)))
   return;

xazax.hun wrote:
> Alternatively, `canDescend` could get the optional `ContextSensitiveOpts` and 
> we can do all the checking there. 
Ah, good idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131809

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

[PATCH] D131809: [clang][dataflow] Add an option for context-sensitive depth

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:665
+if (!(Options.ContextSensitiveOpts &&
+  Env.canDescend(Options.ContextSensitiveOpts->Depth, F)))
   return;

samestep wrote:
> xazax.hun wrote:
> > Alternatively, `canDescend` could get the optional `ContextSensitiveOpts` 
> > and we can do all the checking there. 
> Ah, good idea!
Oh... actually that doesn't work quite so nicely, because it introduces a 
circular dependency between `Transfer.h` and `DataflowEnvironment.h`. I'll 
leave it as-is for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131809

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

[PATCH] D131809: [clang][dataflow] Add an option for context-sensitive depth

[Sam Estep via Phabricator via cfe-commits]

samestep updated this revision to Diff 452783.
samestep added a comment.

Address Stanislav's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131809

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3902,6 +3902,36 @@
   {TransferOptions{/*.ContextSensitiveOpts=*/llvm::None}});
 }
 
+TEST(TransferTest, ContextSensitiveDepthZero) {
+  std::string Code = R"(
+bool GiveBool();
+void SetBool(bool &Var) { Var = true; }
+
+void target() {
+  bool Foo = GiveBool();
+  SetBool(Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_FALSE(Env.flowConditionImplies(FooVal));
+EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+  },
+  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/0}}});
+}
+
 TEST(TransferTest, ContextSensitiveSetTrue) {
   std::string Code = R"(
 bool GiveBool();
@@ -4000,7 +4030,7 @@
   {TransferOptions{ContextSensitiveOptions{}}});
 }
 
-TEST(TransferTest, ContextSensitiveSetTwoLayers) {
+TEST(TransferTest, ContextSensitiveSetTwoLayersDepthOne) {
   std::string Code = R"(
 bool GiveBool();
 void SetBool1(bool &Var) { Var = true; }
@@ -4028,7 +4058,146 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/1}}});
+}
+
+TEST(TransferTest, ContextSensitiveSetTwoLayersDepthTwo) {
+  std::string Code = R"(
+bool GiveBool();
+void SetBool1(bool &Var) { Var = true; }
+void SetBool2(bool &Var) { SetBool1(Var); }
+
+void target() {
+  bool Foo = GiveBool();
+  SetBool2(Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+  },
+  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/2}}});
+}
+
+TEST(TransferTest, ContextSensitiveSetThreeLayersDepthTwo) {
+  std::string Code = R"(
+bool GiveBool();
+void SetBool1(bool &Var) { Var = true; }
+void SetBool2(bool &Var) { SetBool1(Var); }
+void SetBool3(bool &Var) { SetBool2(Var); }
+
+void target() {
+  bool Foo = GiveBool();
+  SetBool3(Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_FALSE(Env.flowConditionImplies(FooVal));
+EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+  },
+  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/2}}});
+}
+
+TEST(TransferTest, ContextSensitiveSetThreeLayersDepthThree) {
+  std::string Code = R"(
+bool GiveBool();
+void SetBool1(bool &Var) { Var = true; }
+void SetBool2(bool &Var) { SetBool1(Var); }
+void SetBool3(bool &Var) { SetBool2(Var); }
+
+void target() {

[clang] 2efc8f8 - [clang][dataflow] Add an option for context-sensitive depth

[Sam Estep via cfe-commits]

Author: Sam Estep
Date: 2022-08-15T19:58:40Z
New Revision: 2efc8f8d6561421c3f47de2f27a365ddfb734425

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

LOG: [clang][dataflow] Add an option for context-sensitive depth

This patch adds a `Depth` field (default value 2) to `ContextSensitiveOptions`, 
allowing context-sensitive analysis of functions that call other functions. 
This also requires replacing the `DeclCtx` field on `Environment` with a 
`CallString` field that contains a vector of decl contexts, to ensure that the 
analysis doesn't try to analyze recursive or mutually recursive calls (which 
would result in a crash, due to the way we handle `StorageLocation`s).

Reviewed By: xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.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 5b29915e368ed..6955bcf2fb4ca 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -348,10 +348,12 @@ class Environment {
 
   /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
   /// returns null.
-  const DeclContext *getDeclCtx() { return DeclCtx; }
+  const DeclContext *getDeclCtx() { return CallStack.back(); }
 
-  /// Sets the `DeclContext` of the block being analysed.
-  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+  /// Returns whether this `Environment` can be extended to analyze the given
+  /// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and 
a
+  /// given `MaxDepth`.
+  bool canDescend(unsigned MaxDepth, const DeclContext *Callee) const;
 
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
@@ -390,7 +392,7 @@ class Environment {
   DataflowAnalysisContext *DACtx;
 
   // `DeclContext` of the block being analysed if provided.
-  const DeclContext *DeclCtx = nullptr;
+  std::vector CallStack;
 
   // In a properly initialized `Environment`, `ReturnLoc` should only be null 
if
   // its `DeclContext` could not be cast to a `FunctionDecl`.

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h 
b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
index 93afcc284e1af..fa05013bb7208 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -21,7 +21,11 @@
 namespace clang {
 namespace dataflow {
 
-struct ContextSensitiveOptions {};
+struct ContextSensitiveOptions {
+  /// The maximum depth to analyze. A value of zero is equivalent to disabling
+  /// context-sensitive analysis entirely.
+  unsigned Depth = 2;
+};
 
 struct TransferOptions {
   /// Options for analyzing function bodies when present in the translation

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 119ef337c6319..f64ade34bcb82 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,10 +154,10 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
-  ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
-  ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
-  MemberLocToStruct(Other.MemberLocToStruct),
+: DACtx(Other.DACtx), CallStack(Other.CallStack),
+  ReturnLoc(Other.ReturnLoc), ThisPointeeLoc(Other.ThisPointeeLoc),
+  DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
+  LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
   FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) 
{
 }
 
@@ -168,11 +168,11 @@ Environment &Environment::operator=(const Environment 
&Other) {
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtxArg)
+ const DeclContext &DeclCtx)
 : Environment(DACtx) {
-  setDeclCtx(&DeclCtxArg);
+  CallStack.push_back(&DeclCtx);
 
-  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
+  if (const auto *FuncDecl = dyn_