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

[Chuanqi Xu via Phabricator via cfe-commits]

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

Remove `non-inline` terms.


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,753 @@
+=
+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 f

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



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:
> > > > > I'm not sure I'm able to follow the example and how it justifies the 
> > > > > rough theory as inadequate to explain the motivation for modules - 
> > > > > could you clarify more directly (in comments, and then we can discuss 
> > > > > how to word it) what the motivation for this section is/what you're 
> > > > > trying to convey?
> > > > Let me answer the motivation first. The motivation comes from my 
> > > > personal experience. I feel like when most people heard modules, they 
> > > > would ask "how much speedup could we get"? And there are some other 
> > > > questions like "why does modules speedup the compilation?". So I guess 
> > > > the readers of the document may have similar questions and I try to 
> > > > answer it here.
> > > > 
> > > > The complexity theory is correct but it may be too abstract to our 
> > > > users. Since the complexity theory is about the scaling. But for 
> > > > certain users, the scales of their codes are temporarily fixed. So when 
> > > > they try to use modules but find the speedup doesn't meet their 
> > > > expectation in O2. They may feel frustrated. And it doesn't work if I 
> > > > say, "hey, you'll get much better speedup if the your codes get 10x 
> > > > longer." I guess they won't buy in. So what I try to do here is to 
> > > > manage the user's expectation to avoid any misunderstanding.
> > > > 
> > > > Following off is about the explanation. For example, there are `1` 
> > > > module interface and `10` users. There is a function `F` in the module 
> > > > interface and the function is used by every users. And let's say we 
> > > > need a `T` time to compile the function `F` and each users without the 
> > > > function `F`.
> > > > In O0, the function `F` will get compiled completely once and get 
> > > > involved in the Sema part 10 times. Due to the Sema part is relatively 
> > > > fast and let's say the Sema part would take `0.1T`. Given we compile 
> > > > them serially, we need `12T` to compile the project.
> > > > 
> > > > But if we are with optimizations, each function `F` will get involved 
> > > > in optimizations and IPO in every users. And these optimizations are 
> > > > most time-consuming. Let's say these optimizations will consume `0.8T`. 
> > > > And the time required will be `19T`. It is easy to say the we need 
> > > > `20T` to compile the project if we're using headers. So we could find 
> > > > the speedup with optimization is much slower.
> > > > 
> > > > BTW, if we write the required time with variables, it will be `nT + mT 
> > > > + T*m*additional_compilation_part`. The `additional_compilation_part ` 
> > > > here corresponds to the time percentage of `Sema` or `Optimizations`. 
> > > > And since `T` and `additional_compilation_part ` are both constant. So 
> > > > if we write them in `O()` form, it would be `O(n+m)`.
> > > > So the theory is still correct.
> > > > 
> > > > 
> > > I think the message is getting a bit lost in the text (both in the 
> > > proposed text, and the comment here).
> > > 
> > > "At -O0 implementations of non-inline functions defined in a module will 
> > > not impact module users, but at higher optimization levels the 
> > > definitions of such functions are provided to user compilations for the 
> > > purposes of optimization (but definitions of these functions are still 
> > > not included in the use's object file) - this means build speed at higher 
> > > optimization levels may be lower than expected given -O0 experience, but 
> > > does provide by more optimization opportunities"
> > > 
> > Yes, it is hard to talk clearly and briefly. In your suggested wording, you 
> > mentioned `non-inline` function, it is accurate but bring new information 
> > to this document. I'm worrying if the reader could understand it if the 
> > reader don't know c++ so much.
> > 
> > I put the suggested wording as the conclusion paragraph for the section and 
> > hope it could make the reader focus on the intention of the section.
> Maybe "non-inline" could be replaced by "module implementation details" (but 
> "function bodies" sounds OK too)
> 
> I think the issue for me is that the current description seems to go into 
> more detail about compiler implementation details than might be helpful for a 
> document at this level. I was/am hoping maybe a one paragraph summary might 
> be simpler/more approachable/sufficiently accurate for the audience.
Yeah, it is hard to control the balance between `readability` vs `accuracy`. 
From my **personal** experience, the 3-stage compilation mode

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D131528#3716876 , @shafik wrote:

> In D131528#3715841 , @thakis wrote:
>
>> We're also still seeing the diag fire after this: 
>> https://ci.chromium.org/p/chromium/builders/ci/ToTLinux
>>
>> (And we cleaned up our codebase back when it was still an error.)
>>
>> Our bots have been red since the change to turn this into a warning landed.
>
> Apologies, my condition was not strict enough, I have put up D131704 
> 

Unfortunately, even after D131704  (with both 
commits), I'm still hitting a couple cases that weren't an error before this. 
With https://martin.st/temp/qt-jit-enum.cpp and 
https://martin.st/temp/protobuf-wire-format.cpp, built with `clang -target 
i686-w64-mingw32 -c qt-jit-enum.cpp -std=c++17 -Wno-user-defined-literals 
-Wno-ignored-attributes` (and same for the other file, although `-std=c++17` 
can be omitted for the protobuf source) I'm still hitting errors for things 
that weren't even warned about before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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

[clang-tools-extra] 6e75ec5 - [clang-tidy] Support C++14 in bugprone-signal-handler.

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

Author: Balázs Kéri
Date: 2022-08-12T09:45:53+02:00
New Revision: 6e75ec5e38dacb14c9ac9578c8e07548861b6d27

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

LOG: [clang-tidy] Support C++14 in bugprone-signal-handler.

Check `bugprone-signal-handler` is improved to check for
C++-specific constructs in signal handlers. This check is
valid until C++17.

Reviewed By: whisperity

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

Added: 
clang-tools-extra/docs/clang-tidy/checks/cert/msc54-cpp.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/signal-handler/stdcpp.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/signal-handler.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.c

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
index 132fbf85c1fe..fc91d79f26a6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
@@ -22,6 +22,7 @@ constexpr llvm::StringLiteral MinimalConformingFunctions[] = {
 // mentioned POSIX specification was not updated after 'quick_exit' appeared
 // in the C11 standard.
 // Also, we want to keep the "minimal set" a subset of the "POSIX set".
+// The list is repeated in bugprone-signal-handler.rst and should be kept up 
to date.
 constexpr llvm::StringLiteral POSIXConformingFunctions[] = {
 "_Exit",
 "_exit",
@@ -277,6 +278,25 @@ bool isStandardFunction(const FunctionDecl *FD) {
   FD->getCanonicalDecl()->getLocation());
 }
 
+/// Check if a statement is "C++-only".
+/// This includes all statements that have a class name with "CXX" prefix
+/// and every other statement that is declared in file ExprCXX.h.
+bool isCXXOnlyStmt(const Stmt *S) {
+  StringRef Name = S->getStmtClassName();
+  if (Name.startswith("CXX"))
+return true;
+  // Check for all other class names in ExprCXX.h that have no 'CXX' prefix.
+  return isa(S);
+}
+
 /// Given a call graph node of a \p Caller function and a \p Callee that is
 /// called from \p Caller, get a \c CallExpr of the corresponding function 
call.
 /// It is unspecified which call is found if multiple calls exist, but the 
order
@@ -291,6 +311,18 @@ Expr *findCallExpr(const CallGraphNode *Caller, const 
CallGraphNode *Callee) {
   return FoundCallee->CallExpr;
 }
 
+SourceRange getSourceRangeOfStmt(const Stmt *S, ASTContext &Ctx) {
+  ParentMapContext &PM = Ctx.getParentMapContext();
+  DynTypedNode P = DynTypedNode::create(*S);
+  while (P.getSourceRange().isInvalid()) {
+DynTypedNodeList PL = PM.getParents(P);
+if (PL.size() != 1)
+  return {};
+P = PL[0];
+  }
+  return P.getSourceRange();
+}
+
 } // namespace
 
 AST_MATCHER(FunctionDecl, isStandardFunction) {
@@ -317,11 +349,7 @@ void 
SignalHandlerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 
 bool SignalHandlerCheck::isLanguageVersionSupported(
 const LangOptions &LangOpts) const {
-  // FIXME: Make the checker useful on C++ code.
-  if (LangOpts.CPlusPlus)
-return false;
-
-  return true;
+  return !LangOpts.CPlusPlus17;
 }
 
 void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) {
@@ -332,13 +360,23 @@ void SignalHandlerCheck::registerMatchers(MatchFinder 
*Finder) {
   unless(isExpandedFromMacro("SIG_IGN")),
   unless(isExpandedFromMacro("SIG_DFL")))
   .bind("handler_expr");
-  Finder->addMatcher(
-  callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr))
-  .bind("register_call"),
-  this);
+  auto HandlerLambda = cxxMemberCallExpr(
+  on(expr(ignoringParenImpCasts(lambdaExpr().bind("handler_lambda");
+  Finder->addMatcher(callExpr(callee(SignalFunction),
+  hasArgument(1, anyOf(HandlerExpr, 
HandlerLambda)))
+ .bind("register_call"),
+ this);
 }
 
 void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *HandlerLambda =
+  Result.Nodes.getNodeAs("handler_lambda")) {
+diag(HandlerLambda->getBeginLoc(),
+ "lambda function is not allowed as signal handler (until C++17)")
+<< HandlerLambda->getSourceRange();
+return;
+  }
+
   cons

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

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

balazske added a comment.

The target option was added, but I can not verify if the change fixes all the 
buildbots, we will see if it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D112374#3716982 , @mizvekov wrote:

> We even match GCC now: https://gcc.godbolt.org/z/WT93WdE7e
>
> Ie we are printing the function type as-written correctly now.

We don't match GCC in other cases. GCC seems to always print the type name 
without qualifiers, clang used to always print with qualifiers, but will now 
print what was typed in the code, see https://gcc.godbolt.org/z/jznncGboM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D129833: Use @llvm.threadlocal.address intrinsic to access TLS

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D129833#3717291 , @jyknight wrote:

> Note that this change caused LLVM to no longer be aware that a TLS variable 
> cannot be NULL. Thus, code like:
>
>   __thread int x;
>   int main() {
> int* y = &x;
> return *y;
>   }
>
> built with `clang -O -fsanitize=null` emits a null-pointer check, when it 
> wouldn't previously. I think llvm.threadlocal.address's return value probably 
> ought to be given the nonnull attribute. We also might want to infer other 
> properties of the returned pointer based on knowledge of the global value 
> parameter, such as alignment.
>
> Furthermore, while the above problem should simply be a very minor 
> optimization degradation, in fact it caused miscompiles, due to a 
> pre-existing bug in the X86 backend. I've sent 
> https://reviews.llvm.org/D131716 to fix //that// part of the problem.

Oh, got it. I'll try to make it. And I find we can't set an intrinsic as 
`NonNull` directly in Intrinsics.td and I filed an issue for it: 
https://github.com/llvm/llvm-project/issues/57113.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129833

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

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I found another case of this warning, which is kinda borderline whether it 
really is an error not:

  #include 
  static _Noreturn void my_exit(void) {
exit(42);
  }
  __attribute__((noreturn)) void (*handler)(void) = my_exit;

The fix is simple though, just be consistent with `_Noreturn` vs 
`__attribute__((noreturn))` on both function and function pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D131307#3714629 , @shafik wrote:

> In D131307#3713011 , @sberg wrote:
>
>> With this commit,
>>
>>   $ cat test.cc
>>   #include "boost/numeric/conversion/cast.hpp"
>>   int main() { return boost::numeric_cast(0L); }
>>   
>>   $ clang++ test.cc
>>
>> succeeds without any diagnostic, while with its parent commit 
>> https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
>>  " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
>> enum to a value outside the range of the enumeration values" it had started 
>> to fail with
>
> Yes, that is intended. When modifying the change to allow it to be turned 
> into a warning it started applying outside of constant expression contexts 
> and that broke a lot more stuff.

I think we might be talking past each other here.  There are three commits:

1. D130058  "[Clang] Diagnose ill-formed 
constant expression when setting a non fixed enum to a value outside the range 
of the enumeration values"
2. D131307  "[Clang] Allow downgrading to a 
warning the diagnostic for setting a non fixed enum to a value outside the 
range of the enumeration values"
3. D131528  "[Clang] Restrict non fixed enum 
to a value outside the range of the enumeration values warning to context 
requiring a constant expression"

(1) had started to diagnose my reproducer as an error (and I assume it did 
rightly so).  But then (2) stopped diagnosing it at all, letting the reproducer 
compile successfully without any diagnostic (and I assume wrongly so).  (3) 
didn't make any further change regarding that behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Roy Jacobson via Phabricator via cfe-commits]

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

LGTM, thank you for this patch!
I'll wait until Monday to let others have a look and then we can backport, I 
think it's useful and it's small enough to not be a problem.

Could you add name+email for the commit message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131730

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

[clang] e6db064 - [doc] Remove release notes from the main branch for changes that were backported to 15.x

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-08-12T11:21:51+03:00
New Revision: e6db064394bc19aac906531564ce59e9de255cc1

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

LOG: [doc] Remove release notes from the main branch for changes that were 
backported to 15.x

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
lld/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e895a6c8e095..f2339da1a04be 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -91,13 +91,6 @@ Improvements to Clang's diagnostics
   language modes. It may be downgraded to a warning with
   ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
   ``-Wno-implicit-function-pointer-types``.
-- When including a PCH from a GCC style directory with multiple alternative PCH
-  files, Clang now requires all defines set on the command line while 
generating
-  the PCH and when including it to match. This matches GCC's behaviour.
-  Previously Clang would tolerate defines to be set when creating the PCH but
-  missing when used, or vice versa. This makes sure that Clang picks the
-  correct one, where it previously would consider multiple ones as potentially
-  acceptable (and erroneously use whichever one is tried first).
 - Clang will now print more information about failed static assertions. In
   particular, simple static assertion expressions are evaluated to their
   compile-time value and printed out if the assertion fails.

diff  --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index d6b149d16a041..033f76c70d8ff 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -39,11 +39,7 @@ COFF Improvements
 MinGW Improvements
 --
 
-* The ``--exclude-symbols`` option is now supported.
-  (`D130118 `_)
-
-* Support for an entirely new object file directive, ``-exclude-symbols:``,
-  has been implemented. (`D130120 `_)
+* ...
 
 MachO Improvements
 --



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

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: sylvestre.ledru, andrewng, russell.gallop, 
phosek, beanz.
CarlosAlbertoEnciso added projects: All, LLVM, clang.
Herald added a subscriber: mgorny.
CarlosAlbertoEnciso requested review of this revision.
Herald added a subscriber: cfe-commits.

The https://reviews.llvm.org/D53014 added CMAKE_BUILD_TYPE to 
the list of BOOTSTRAP_DEFAULT_PASSTHROUGH variables.

The downside is that both stage-1 and stage-2 configurations
are the same. So it is not possible to build different stage
configurations.

This patch allow explicit bootstrap options to override any 
passthrough ones.

For instance, the following settings would build:
stage-1 (Release) and stage-2 (Debug)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131755

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

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

move implemention of CheckTypeTraitArity into SemaExprCXX, since type traits 
are C++ specific language extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5427,9 +5427,26 @@
 }
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);
 
+  bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
+
   ExprResult CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
 SourceLocation OpLoc,
 Unary

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:140
   for (const Expr *Init : Sem->inits()) {
+if (!Init)
+  continue;

we should have this bail out after introducing the scope_exit below to make 
sure we skip the field.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1427
+  return {
+.b = A(),
+  };

a better test would be

```
struct A{};
struct Foo {int a; int b;};
Foo f{A(), 1);
```
and make sure we still get the hints for `1` as `b`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

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

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

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 452123.
usaxena95 added a comment.

Added a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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

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

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

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG72ac7cac3f14: Handle explicitly defaulted consteval special 
members. (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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

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

[clang] 72ac7ca - Handle explicitly defaulted consteval special members.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-08-12T12:13:06+02:00
New Revision: 72ac7cac3f14c905426ba7ff0c6c36ee3b3fb375

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

LOG: Handle explicitly defaulted consteval special members.

Followup patch for D128083

Previously, using a non-consteval constructor from an consteval constructor 
would code generates the consteval constructor.
Example
```
template 
struct S {
  T i;
  consteval S() = default;
};
struct Foo {
Foo() {}
};
void func() {
  S three; // incorrectly accepted by clang.
}
```

This happened because clang erroneously disregards `consteval` specifier for a 
`consteval explicitly defaulted special member functions in a class template` 
if it has dependent data members without a `consteval default constructor`.

According to
```
C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358):
If the instantiated template specialization of a constexpr function
template or member function of a class template would fail to satisfy
the requirements for a constexpr function or constexpr constructor, that
specialization is still a constexpr function or constexpr constructor,
even though a call to such a function cannot appear in a constant
expression.
```

Therefore the `consteval defaulted constructor of a class template` should be 
considered `consteval` even if the data members' default constructors are not 
consteval.
Keeping this constructor `consteval` allows complaining while processing the 
call to data member constructors.
(Same applies for other special member functions).

This works fine even when we have more than one default constructors since we 
process the constructors after the templates are instantiated.

This does not address initialization issues raised in
[2602](https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2602) and 
compiler divergence seen in https://godbolt.org/z/va9EMvvMe

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

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f2339da1a04be..572961939d859 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -141,7 +141,10 @@ C++20 Feature Support
   `GH54300 `_,
   `GH54301 `_,
   and `GH49430 `_.
-
+- Consider explicitly defaulted constexpr/consteval special member function
+  template instantiation to be constexpr/consteval even though a call to such
+  a function cannot appear in a constant expression.
+  (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 
 
 

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ab3af1298be51..78c39197e47bb 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
@@ -7539,6 +7540,17 @@ bool 
Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
   // FIXME: This should not apply if the member is deleted.
   bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, RD, CSM,
  HasConstParam);
+
+  // C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358):
+  //   If the instantiated template specialization of a constexpr function
+  //   template or member function of a class template would fail to satisfy
+  //   the requirements for a constexpr function or constexpr constructor, that
+  //   specialization is still a constexpr function or constexpr constructor,
+  //   even though a call to such a function cannot appear in a constant
+  //   expression.
+  if (MD->isTemplateInstantiation() && MD->isConstexpr())
+Constexpr = true;
+
   if ((getLangOpts().CPlusPlus20 ||
(getLangOpts().CPlusPlus14 ? !isa(MD)
   : isa(MD))) &&

diff  --git a/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp 
b/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
index 73916fd3027e3..0c3dd1ea7aa27 100644
--- a/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
+++ b/clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
@@ -44,18 +44,20 @@ void tester() {
 }
 
 template struct S : T {
-  constexpr S() = default;
-  constexpr S(const S&) = default;
-  constexpr S(S&&) = defau

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D112374#3718417 , @ilya-biryukov 
wrote:

> We don't match GCC in other cases. GCC seems to always print the type name 
> without qualifiers, clang used to always print with qualifiers, but will now 
> print what was typed in the code, see https://gcc.godbolt.org/z/jznncGboM

Well, Clang always tried printing them as typed in the code, except for this 
bug that we fixed here.

It would fail to represent a name written without both qualifiers and 
elaboration. If it had either or both, it would print the name as-written.
Example: https://gcc.godbolt.org/z/3xv4xxYf1

So, consider that relying on `__PRETTY_FUNCTION__` output was pretty unreliable 
from the get go, as you already had different results across compilers.

Also, consider these other points:

- This patch had a lot of test churn, but no test churn on 
`__PRETTY_FUNCTION__` tests, so I think this means that this was pretty much 
untested on Clang's part.
- The implementation relies on reconstructing the function signature as-written 
from the AST. So any improvement in that area would cause changes, or extra 
complexity to keep the old limitations around behind switches.
- There seems to be no attempt to enforce the stability of this result at the 
architectural level in Clang. It relies directly on the type printer, which is 
used all around in many different scenarios, and you can see that folks here 
just make changes to the type printer for cosmetic reasons, with little vetting 
and no concern for stability.

Otherwise, the type printer is customizable with Policies, so you could think 
of adding a new one to customize this behavior. But even then, I think the old 
Clang behavior was too weird to make an option for it.
What would we even call such a policy, something like 
`InventFullNameQualifierIfNeitherQualifiedOrElaborated`?

We could make a separate type printer system for such a macro, stripping the 
function signature of any decoration and using a stable policy, and create all 
the test cases for it so that we don't regress accidentally.
We could leave the current macro as is, printing the signature as written, and 
add this new one behind a new macro, so that the user gets to choose between 
`__PRETTY_FUNCTION__` and `__NAKED_FUNCTION__`? 🤔


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D131724: [pseudo] Eliminate an ambiguity for the empty member declaration.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131724

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

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

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:157
 
+// Returns whether `Block` is a "back edge" in the CFG. Such a block has only
+// one successor, the start of the loop.

Let's start function comments with `///` throughout the file.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:164
+
+// Returns the predecessor to `Block` that is a "back edge", if one exists.
+//





Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:167-168
+// If this function returns a non-null pointer, that means `Block` dominates 
the
+// back edge block. (That is, all paths from the entry block to the back edge
+// block must go through `Block`.) It also means that there are only two
+// predecessors; the other is along the path from the entry block to `Block`.





Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:168-169
+// back edge block. (That is, all paths from the entry block to the back edge
+// block must go through `Block`.) It also means that there are only two
+// predecessors; the other is along the path from the entry block to `Block`.
+static const CFGBlock *findBackEdge(const CFGBlock *Block) {

li.zhe.hua wrote:
> xazax.hun wrote:
> > Is this also true when we have multiple `continue` statements in the loop?
> Yes. The end of the loop, and each of the `continue` statements, target the 
> back edge block. They all get funneled through that back edge to `Block`, 
> such that `Block` only has two predecessors. However, I haven't verified this 
> in the CFG code, only by not finding a counterexample.
Does that hold for back edges stemming from `goto` statements?



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:270
+
+TEST(DataflowAnalysisTest, ConvergesOnWidenAnalysis) {
+  std::string Code = R"(

There's already a set of "widening" tests - 
http://google3/third_party/llvm/llvm-project/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp;l=527-712;rcl=462638952

What do you think about refactoring those so that we have tests that exercise 
the framework with both `join` and `widen`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131646

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131351#3718421 , @mstorsjo wrote:

> I found another case of this warning, which is kinda borderline whether it 
> really is an error not:
>
>   #include 
>   static _Noreturn void my_exit(void) {
> exit(42);
>   }
>   __attribute__((noreturn)) void (*handler)(void) = my_exit;
>
> The fix is simple though, just be consistent with `_Noreturn` vs 
> `__attribute__((noreturn))` on both function and function pointer.

Oh wow, that one is neat, weird, and I'm not certain what I think about it yet. 
`__attribute__((noreturn))` is part of the function type, which is why you're 
allowed to write it on a function pointer declaration. `_Noreturn` is not part 
of the function type (nor is `[[noreturn]]` in C2x), so the two functions types 
do not match and that's why you get the diagnostic. This is the same logic that 
allows a diagnostic for a case like:

  __attribute__((stdcall)) void func(int a, int b);
  void (*fp)(int, int) = func; // Oops, calling convention mismatch

(on targets where these calling conventions exist and `stdcall` is not the 
default calling convention).

However, we don't care about the type mismatch when doing a redeclaration or 
when dropping the attribute, as in:

  __attribute__((noreturn)) void my_exit(void);
  void (*handler)(void) = my_exit; // Silently drops the attribute
  
  _Noreturn void my_exit(void) { // No concerns about the type mismatch on 
redeclaration because we inherit __attribute__((noreturn))
  }

so maybe we shouldn't worry about it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D131351#3718725 , @aaron.ballman 
wrote:

> However, we don't care about the type mismatch when doing a redeclaration or 
> when dropping the attribute, as in:
>
>   __attribute__((noreturn)) void my_exit(void);
>   void (*handler)(void) = my_exit; // Silently drops the attribute
>   
>   _Noreturn void my_exit(void) { // No concerns about the type mismatch on 
> redeclaration because we inherit __attribute__((noreturn))
>   }
>
> so maybe we shouldn't worry about it here.

Are you sure? If the function types don't match in C++, I would expect that to 
become a different overload, not a redeclaration, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D131351#3718725 , @aaron.ballman 
wrote:

> In D131351#3718421 , @mstorsjo 
> wrote:
>
>> I found another case of this warning, which is kinda borderline whether it 
>> really is an error not:
>>
>>   #include 
>>   static _Noreturn void my_exit(void) {
>> exit(42);
>>   }
>>   __attribute__((noreturn)) void (*handler)(void) = my_exit;
>>
>> The fix is simple though, just be consistent with `_Noreturn` vs 
>> `__attribute__((noreturn))` on both function and function pointer.
>
> Oh wow, that one is neat, weird, and I'm not certain what I think about it 
> yet.

FWIW, the source of the code here is a piece of code from glibc, distributed as 
part of gnulib and in gettext (in various older/newer copies of it). The bug 
has itself been fixed in gnulib already almost 2 years ago, as result of the 
Clang warning at the time: 
https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/obstack.c?id=0cc39712803ade7b2d4b89c36b143dad72404063
 However in this case, a (potentially old) packaged tarball of gettext 
contained two older copies of the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via Phabricator via cfe-commits]

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

address review comment -- make sure we skip the corresponding field when its
init expr is null, and polish the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,7 +141,7 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
+if (!Init || llvm::isa(Init))
   continue; // a "hole" for a subobject that was not explicitly initialized
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,7 +141,7 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
+if (!Init || llvm::isa(Init))
   continue; // a "hole" for a subobject that was not explicitly initialized
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:140
   for (const Expr *Init : Sem->inits()) {
+if (!Init)
+  continue;

kadircet wrote:
> we should have this bail out after introducing the scope_exit below to make 
> sure we skip the field.
good catch! You're right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131730

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

[clang-tools-extra] a1a1a78 - [pseudo] Eliminate an ambiguity for the empty member declaration.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-08-12T13:46:26+02:00
New Revision: a1a1a78ac8cf837e4c05152c9715f399b33bfb59

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

LOG: [pseudo] Eliminate an ambiguity for the empty member declaration.

We happened to introduce a `member-declaration := ;` rule
when inlining the `member-declaration := decl-specifier-seq_opt
member-declarator-list_opt ;`.
And with the `member-declaration := empty-declaration` rule, we had two parses 
of `;`.

This patch is to restrict the grammar to eliminate the
`member-declaration := ;` rule.

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

Added: 
clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp

Modified: 
clang-tools-extra/pseudo/lib/cxx/cxx.bnf

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf 
b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
index 5f1cba4503948..d5cc0a9d2c857 100644
--- a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -557,7 +557,8 @@ class-key := STRUCT
 class-key := UNION
 member-specification := member-declaration member-specification_opt
 member-specification := access-specifier : member-specification_opt
-member-declaration := decl-specifier-seq_opt member-declarator-list_opt ;
+member-declaration := decl-specifier-seq member-declarator-list_opt ;
+member-declaration := member-declarator-list ;
 member-declaration := function-definition
 member-declaration := using-declaration
 member-declaration := using-enum-declaration

diff  --git a/clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp 
b/clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
new file mode 100644
index 0..2540dd010fcef
--- /dev/null
+++ b/clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest 
--forest-abbrev=false | FileCheck %s
+class A {
+;
+// CHECK-NOT: member-declaration := ;
+// CHECK: member-declaration := empty-declaration
+// CHECK-NOT: member-declaration := ;
+};



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

[PATCH] D131724: [pseudo] Eliminate an ambiguity for the empty member declaration.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1a1a78ac8cf: [pseudo] Eliminate an ambiguity for the empty 
member declaration. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131724

Files:
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp


Index: clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest 
--forest-abbrev=false | FileCheck %s
+class A {
+;
+// CHECK-NOT: member-declaration := ;
+// CHECK: member-declaration := empty-declaration
+// CHECK-NOT: member-declaration := ;
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -557,7 +557,8 @@
 class-key := UNION
 member-specification := member-declaration member-specification_opt
 member-specification := access-specifier : member-specification_opt
-member-declaration := decl-specifier-seq_opt member-declarator-list_opt ;
+member-declaration := decl-specifier-seq member-declarator-list_opt ;
+member-declaration := member-declarator-list ;
 member-declaration := function-definition
 member-declaration := using-declaration
 member-declaration := using-enum-declaration


Index: clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest --forest-abbrev=false | FileCheck %s
+class A {
+;
+// CHECK-NOT: member-declaration := ;
+// CHECK: member-declaration := empty-declaration
+// CHECK-NOT: member-declaration := ;
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -557,7 +557,8 @@
 class-key := UNION
 member-specification := member-declaration member-specification_opt
 member-specification := access-specifier : member-specification_opt
-member-declaration := decl-specifier-seq_opt member-declarator-list_opt ;
+member-declaration := decl-specifier-seq member-declarator-list_opt ;
+member-declaration := member-declarator-list ;
 member-declaration := function-definition
 member-declaration := using-declaration
 member-declaration := using-enum-declaration
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1828c75 - [pseudo] Apply the function-declarator to member functions.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-08-12T13:49:01+02:00
New Revision: 1828c75d5f4ff657cf977476091fa224c8193e1d

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

LOG: [pseudo] Apply the function-declarator to member functions.

A followup patch of d489b3807f096584175c321ce7f20e9dcd49b1da, but for
member functions, this will eliminate a false parse of member
declaration.

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

Added: 
clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp

Modified: 
clang-tools-extra/pseudo/lib/cxx/cxx.bnf

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf 
b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
index d5cc0a9d2c857..697e79d609aa2 100644
--- a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -571,9 +571,9 @@ member-declaration := opaque-enum-declaration
 member-declaration := empty-declaration
 member-declarator-list := member-declarator
 member-declarator-list := member-declarator-list , member-declarator
-member-declarator := declarator virt-specifier-seq_opt pure-specifier_opt
-member-declarator := declarator requires-clause
-member-declarator := declarator brace-or-equal-initializer
+member-declarator := function-declarator virt-specifier-seq_opt 
pure-specifier_opt
+member-declarator := function-declarator requires-clause
+member-declarator := non-function-declarator brace-or-equal-initializer_opt
 member-declarator := IDENTIFIER_opt : constant-expression 
brace-or-equal-initializer_opt
 virt-specifier-seq := virt-specifier
 virt-specifier-seq := virt-specifier-seq virt-specifier

diff  --git a/clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp 
b/clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
new file mode 100644
index 0..58d0ff4ccae9a
--- /dev/null
+++ b/clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Similiar to declarator-function.cpp, but for member functions.
+class Foo {
+  void foo() {};
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+// CHECK: member-declaration~function-definition := decl-specifier-seq 
function-declarator function-body
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+};



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

[PATCH] D131720: [pseudo] Apply the function-declarator to member functions.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1828c75d5f4f: [pseudo] Apply the function-declarator to 
member functions. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131720

Files:
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp


Index: clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Similiar to declarator-function.cpp, but for member functions.
+class Foo {
+  void foo() {};
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+// CHECK: member-declaration~function-definition := decl-specifier-seq 
function-declarator function-body
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -571,9 +571,9 @@
 member-declaration := empty-declaration
 member-declarator-list := member-declarator
 member-declarator-list := member-declarator-list , member-declarator
-member-declarator := declarator virt-specifier-seq_opt pure-specifier_opt
-member-declarator := declarator requires-clause
-member-declarator := declarator brace-or-equal-initializer
+member-declarator := function-declarator virt-specifier-seq_opt 
pure-specifier_opt
+member-declarator := function-declarator requires-clause
+member-declarator := non-function-declarator brace-or-equal-initializer_opt
 member-declarator := IDENTIFIER_opt : constant-expression 
brace-or-equal-initializer_opt
 virt-specifier-seq := virt-specifier
 virt-specifier-seq := virt-specifier-seq virt-specifier


Index: clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Similiar to declarator-function.cpp, but for member functions.
+class Foo {
+  void foo() {};
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+// CHECK: member-declaration~function-definition := decl-specifier-seq function-declarator function-body
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -571,9 +571,9 @@
 member-declaration := empty-declaration
 member-declarator-list := member-declarator
 member-declarator-list := member-declarator-list , member-declarator
-member-declarator := declarator virt-specifier-seq_opt pure-specifier_opt
-member-declarator := declarator requires-clause
-member-declarator := declarator brace-or-equal-initializer
+member-declarator := function-declarator virt-specifier-seq_opt pure-specifier_opt
+member-declarator := function-declarator requires-clause
+member-declarator := non-function-declarator brace-or-equal-initializer_opt
 member-declarator := IDENTIFIER_opt : constant-expression brace-or-equal-initializer_opt
 virt-specifier-seq := virt-specifier
 virt-specifier-seq := virt-specifier-seq virt-specifier
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131762: [pseudo][wip] Enforce the C++ rule for the optional init-declarator-list.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

Basically it implements the https://eel.is/c++draft/dcl.pre#5 rule.

The motivation is to eliminate a false parse (simple declaration) for the enum
declaration.

  // A simple declaration without a declarator
  // or an opaque-enum-declaration
  enum a;

However, it has some negative effects on broken code (with the single-type
decl-specifier-seq guard):

- break the `const int;` case (which we used to parse as a declaration without 
declarator), now we fails to parse, because it is invalid code per the C++ spec.
- regress the `const Foo;` case, now we parse it as a declaration with a 
declarator named `Foo` and `const` as the decl-specifier-seq, vs we used to 
parsed as a declaration without declarator (which is better IMO);

Just post here to get thoughts about it:
My feeling is that this degrades the parser a lot for the little benefit we 
gain,
we probably don't move forward with this approach.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131762

Files:
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf


Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -334,7 +334,7 @@
 block-declaration := opaque-enum-declaration
 nodeclspec-function-declaration := function-declarator ;
 alias-declaration := USING IDENTIFIER = defining-type-id ;
-simple-declaration := decl-specifier-seq init-declarator-list_opt ;
+simple-declaration := decl-specifier-seq init-declarator-list_opt ; [guard]
 simple-declaration := decl-specifier-seq ref-qualifier_opt [ identifier-list ] 
initializer ;
 static_assert-declaration := STATIC_ASSERT ( constant-expression ) ;
 static_assert-declaration := STATIC_ASSERT ( constant-expression , 
string-literal ) ;
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -161,6 +161,59 @@
   return symbolToToken(P.Lookahead) != tok::kw_else;
 }
 
+bool canOmitDeclarator(const ForestNode* N) {
+  // Returns true if a decl-specifier is
+  //   a class-specifier
+  //   or an elaborated-type-specifier with class-key, or an enum_specifier.
+  auto hasTargetSpecifier = [](const ForestNode *N, auto &hasTargetSpecifier) {
+if (N->kind() == ForestNode::Opaque)
+  return false;
+if (N->kind() == ForestNode::Ambiguous)
+  return llvm::all_of(N->alternatives(),
+  std::bind(hasTargetSpecifier, std::placeholders::_1,
+hasTargetSpecifier));
+
+switch (N->rule()) {
+case rule::defining_type_specifier::type_specifier:
+case rule::defining_type_specifier::enum_specifier:
+case rule::type_specifier::elaborated_type_specifier:
+case rule::decl_specifier::defining_type_specifier:
+  return hasTargetSpecifier(N->elements()[0], hasTargetSpecifier);
+
+case rule::enum_specifier::enum_head__L_BRACE__R_BRACE:
+case rule::enum_specifier::
+enum_head__L_BRACE__enumerator_list__COMMA__R_BRACE:
+case rule::enum_specifier::enum_head__L_BRACE__enumerator_list__R_BRACE:
+case rule::elaborated_type_specifier::
+class_key__nested_name_specifier__IDENTIFIER:
+case rule::elaborated_type_specifier::
+class_key__nested_name_specifier__simple_template_id:
+case rule::elaborated_type_specifier::
+class_key__nested_name_specifier__TEMPLATE__simple_template_id:
+case rule::elaborated_type_specifier::class_key__simple_template_id:
+case rule::elaborated_type_specifier::class_key__IDENTIFIER:
+case rule::defining_type_specifier::class_specifier:
+  return true;
+default:
+  return false;
+}
+  };
+  while (true) {
+switch (N->rule()) {
+case cxx::rule::decl_specifier_seq::decl_specifier:
+  return hasTargetSpecifier(N->elements()[0], hasTargetSpecifier);
+case cxx::rule::decl_specifier_seq::decl_specifier__decl_specifier_seq:
+  if (hasTargetSpecifier(N->elements()[0], hasTargetSpecifier))
+return true;
+  N = N->elements()[1];
+  break;
+default:
+  LLVM_DEBUG(llvm::errs() << "Unhandled rule " << N->rule() << "\n");
+  llvm_unreachable("decl_specifier_seq be exhaustive!");
+}
+  }
+  return false;
+}
 // Whether this e.g. decl-specifier contains an "exclusive" type such as a 
class
 // name, and thus can't combine with a second exclusive type.
 //
@@ -320,6 +373,15 @@
   {rule::nested_name_specifier::COLONCOLON,
TOKEN_GUARD(coloncolon, Tok.prev().Kind != tok::identifier)},
 
+  // Implement C++ [dcl.pre#5]:
+ 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6558
+Here're resource binding examples with and without space:
+``register(t3, space1)``
+``register(t1)``
+The full documentation is available here: 
https://docs.microsoft.com/en-us/windows/win32/direct3d12/resource-binding-in-hlsl

These aren't really fully examples of how to use this properly. Can you add an 
actual example (it doesn't have to do anything particularly useful, the goal is 
largely to show off how a user would use this attribute)?



Comment at: clang/lib/Parse/ParseHLSL.cpp:112-113
+}
+if (!Tok.is(tok::identifier)) {
+  Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;
+  SkipUntil(tok::r_paren, StopAtSemi); // skip through )

Any reason not to use `ExpectAndConsume(tok::identifier, diag::err_expected)` 
here instead?



Comment at: clang/lib/Parse/ParseHLSL.cpp:138
+  // Add numeric_constant for fix-it.
+  if (SpaceStr.equals_insensitive("space") && 
Tok.is(tok::numeric_constant))
+fixSeparateAttrArgAndNumber(SpaceStr, SpaceLoc, Tok, ArgExprs, *this,

Do we really mean insensitive here? (See comment below.)



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6942-6952
+  StringRef Str;
+  SourceLocation ArgLoc;
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6992
+
+  if (!Space.startswith_insensitive("space")) {
+S.Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;

I'm surprised we want to be case insensitive here; we don't document that we 
are, but also, we're not case insensitive with the slot letters so it seems 
inconsistent. Is this part of the HLSL requirements?

(Note, I have no problems with doing a case insensitive check if the case 
sensitive check fails so we can give a better warning + fixit, if you thought 
that was useful.)


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] 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;

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
...
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Sarif.h:157
+/// 1. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648";>level
 property
+enum class SarifResultLevel { Note, Warning, Error };
+

vaibhav.y wrote:
> aaron.ballman wrote:
> > Should this include `Remark` for diagnostics like: 
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticFrontendKinds.td#L55
> > 
> > If not, I think the comments should explain how to map remarks to one of 
> > these levels.
> Ack, if we're adding `remark`, should I also add support the 
> "informational"/"fail" result kind that was previously discussed by you? 
> Currently there's no `kind`, so we are defaulting to "fail".
I think we should, but it could be done in a follow-up commit. Remarks aren't 
the most common of diagnostics (they're infrequent unless you're asking for 
them), but they certainly exist and we should have some plan for them. Doing it 
as a separate patch means we don't have to think about things like "What does 
it mean to have an optimization report in SARIF? What should that look like?" 
quite yet.



Comment at: clang/lib/Basic/Sarif.cpp:25
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/JSON.h"

vaibhav.y wrote:
> aaron.ballman wrote:
> > Why was this include required?
> It provides `llvm_unreachable`. so far it was available transitively, must 
> have been added when I implemented `resultLevelToStr(...)`.
> 
> I don't feel strongly about keeping this though.
Yeah, that's one I'd drop as it's transitively included through many different 
interfaces already. But I don't feel strongly because IWYU is a nice approach 
too.



Comment at: clang/lib/Basic/Sarif.cpp:396-399
+  if (Result.LevelOverride.hasValue())
+Ret["level"] = resultLevelToStr(Result.LevelOverride.getValue());
+  else
+Ret["level"] = resultLevelToStr(Rule.DefaultConfiguration.Level);

vaibhav.y wrote:
> aaron.ballman wrote:
> > (Probably have to re-run clang-format)
> Good catch! I notice Optional has `value_or`, is that okay as well here?
Oh yes, that's even better!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131084

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D129694#3718225 , @ssquare08 wrote:

> Yes, that is correct. My question is, is it okay to mangle the host and the 
> device side independently using `getTargetEntryUniqueInfo`? The reason I am 
> asking is because you had expressed some concerns regarding mangling them 
> separately. Or, maybe there is a way to mangle the original name before the 
> host and device compilation split?

You'll need to mangle them separately for the device and host, the difference 
is that we want to use a function that shares the input to create the mangled 
name. As far as I know, this is done using a metadata node in the host bitcode. 
So as long as we share the same method that kernels use it should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/InlayHints.cpp:144
 });
-if (llvm::isa(Init))
+if (!Init || llvm::isa(Init))
   continue; // a "hole" for a subobject that was not explicitly initialized

nit: can you also update the comment to mention `broken initializer` (and maybe 
even a fixme to handle these, as in theory this is likely spelled in the code, 
but wasn't retained in the AST even as a `recoveryexpr`, hence we still have a 
place to attach the hint)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

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

[clang] 48e1250 - [clang][SVE] Undefine preprocessor macro defined in

[Paul Walker via cfe-commits]

Author: Maciej Gabka
Date: 2022-08-12T12:25:49Z
New Revision: 48e1250a91d244741c8677fed248ace1fcd7c41c

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

LOG: [clang][SVE] Undefine preprocessor macro defined in

arm_sve.h defines and uses __ai macro which needs to be undefined (as it is
already in arm_neon.h).

Reviewed By: paulwalker-arm

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

Added: 


Modified: 
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/SveEmitter.cpp 
b/clang/utils/TableGen/SveEmitter.cpp
index 4f8419b78eb7d..2ab7d7372a2a8 100644
--- a/clang/utils/TableGen/SveEmitter.cpp
+++ b/clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,8 @@ void SVEEmitter::createHeader(raw_ostream &OS) {
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
+  OS << "#undef __aio\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }



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

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

[Paul Walker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG48e1250a91d2: [clang][SVE] Undefine preprocessor macro 
defined in (authored by mgabka, committed by paulwalker-arm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

Files:
  clang/utils/TableGen/SveEmitter.cpp


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


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

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd created this revision.
animeshk-amd added reviewers: saiislam, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
animeshk-amd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This lit test is added based upon the tests present in the
tests/5.0/metadirective directory of the SOLLVE repo
https://github.com/SOLLVE/sollve_vv


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131763

Files:
  clang/test/OpenMP/metadirective_device_arch_codegen_amd_or_nvidia.c

Index: clang/test/OpenMP/metadirective_device_arch_codegen_amd_or_nvidia.c
===
--- /dev/null
+++ clang/test/OpenMP/metadirective_device_arch_codegen_amd_or_nvidia.c
@@ -0,0 +1,122 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -w -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -w -std=c++11 -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -target-cpu gfx906 -o - | FileCheck %s
+// expected-no-diagnostics
+
+
+/*===---=== 
+
+Inspired from SOLLVE tests:
+ - 5.0/metadirective/test_metadirective_arch_is_nvidia.c
+ - 5.0/metadirective/test_metadirective_arch_nvidia_or_amd.c
+
+======*/
+
+
+#define N 1024
+
+int metadirective1() {
+   
+   int v1[N], v2[N], v3[N];
+
+   int target_device_num, host_device_num, default_device;
+   int errors = 0;
+
+   #pragma omp target map(to:v1,v2) map(from:v3, target_device_num) device(default_device)
+   {
+  #pragma omp metadirective \
+   when(device={arch("amdgcn")}: teams distribute parallel for) \
+   default(parallel for)
+
+ for (int i = 0; i < N; i++) {
+	#pragma omp atomic write
+v3[i] = v1[i] * v2[i];
+ }
+   }
+
+   return errors;
+}
+
+// CHECK-LABEL: define weak_odr amdgpu_kernel void {{.+}}metadirective1
+// CHECK: entry:
+// CHECK: %{{[0-9]}} = call i32 @__kmpc_target_init
+// CHECK: user_code.entry:
+// CHECK: call void @__omp_outlined__
+// CHECK-NOT: call void @__kmpc_parallel_51
+// CHECK: ret void
+
+
+// CHECK-LABEL: define internal void @__omp_outlined__
+// CHECK: entry:
+// CHECK: call void @__kmpc_distribute_static_init
+// CHECK: omp.loop.exit:  
+// CHECK: call void @__kmpc_distribute_static_fini
+
+
+// CHECK-LABEL: define internal void @__omp_outlined__.{{[0-9]+}}
+// CHECK: entry:
+// CHECK: call void @__kmpc_for_static_init_4
+// CHECK: omp.inner.for.body:
+// CHECK: store atomic {{.*}} monotonic
+// CHECK: omp.loop.exit:
+// CHECK-NEXT: call void @__kmpc_distribute_static_fini
+// CHECK-NEXT: ret void
+
+
+
+int metadirective2() {
+   int errors = 0;
+
+   int i, device_num, initial_device;
+   int a[N];
+ 
+ #pragma omp target device(device_num) map(from:initial_device)
+ {
+   #pragma omp metadirective \
+  when( implementation={vendor(nvidia)}: \
+teams num_teams(512) thread_limit(32) ) \
+  when( implementation={vendor(amd)}: \
+teams num_teams(512) thread_limit(64) ) \
+  when( implementation={vendor(llvm)}: \
+teams num_teams(512) thread_limit(64) ) \
+  default (teams)
+   #pragma omp distribute parallel for
+ for (i = 0; i < N; i++) {
+#pragma omp atomic write
+a[i] = i;
+ }
+ }
+   return errors;
+}
+
+
+
+
+// CHECK-LABEL: define weak_odr amdgpu_kernel void {{.+}}metadirective2
+// CHECK: entry:
+// CHECK: %{{[0-9]+}} = call i32 @__kmpc_target_init
+// CHECK: user_code.entry:
+// CHECK: call void @__omp_outlined__.{{[0-9]+}}
+// CHECK: call void @__kmpc_target_deinit
+
+
+// CHECK-LABEL: define internal void @__omp_outlined__.{{[0-9]+}}
+// CHECK: entry:
+// CHECK: call void @__kmpc_distribute_static_init_4
+// CHECK: omp.inner.for.body:
+// CHECK: call void @__kmpc_parallel_51({{.*}}ptr @__omp_outlined__.{{[0-9]+.*}})
+// CHECK: omp.loop.exit:
+// CHECK-NEXT: call void @__kmpc_distribute_static_fini
+
+
+// CHECK-LABEL: define internal void @__omp_outlined__.{{[0-9]+}}
+// CHECK: entry:
+// CHECK: call void @__kmpc_for_static_init_4
+// CHECK: omp.inner.for.body:
+// CHECK: store atomic {{.*}} monotonic
+// CHECK: omp.loop.exit:
+// CHECK-NEXT: call void @__kmpc_distribute_static_fini
+
+
+// CHECK: attributes #{{[0-9]+}} = {{.*}} "omp_target_num_teams"="512"
\ No newline at end of file
_

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Left a few more comments and also added Diana as a reviewer. Would be good to 
get an extra pair of eyes on this :)




Comment at: clang/include/clang/Driver/Options.td:6320-6325
+def pic_level : Separate<["-"], "pic-level">,
+  HelpText<"Value for __PIC__">,
+  MarshallingInfoInt>;
+def pic_is_pie : Flag<["-"], "pic-is-pie">,
+  HelpText<"File is for a position independent executable">,
+  MarshallingInfoFlag>;

awarzynski wrote:
> These are code-gen options to me. While originally located under "Language 
> Options", I think that it would make more sense to move them near "CodeGen 
> Options" instead (e.g. near `mrelocation_model`). @MaskRay any thoughts?
Turns out that in Clang these options are indeed [[ 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.def#L199
 | LangOptions ]]. That's a bit confusing to me, but oh well.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:120
 
+  // -fPIC/-fPIE and their variants. Similar to clang.
+  llvm::Reloc::Model RelocationModel;

awarzynski wrote:
> I would skip "Similar to clang" - this applies most of things here.
Also, I'd move this whole block to a dedicated method.



Comment at: flang/include/flang/Frontend/FrontendOptions.h:290-295
+  /// Some targets need the pic levels. These are stored as
+  /// LLVM module flags.
+  llvm::Optional PICLevel;
+  llvm::Optional PIELevel;
+  /// For setting up the target machine.
+  llvm::Optional RelocModel;

To me these are CodeGen options. Could you move this to 
https://github.com/llvm/llvm-project/blob/main/flang/include/flang/Frontend/CodeGenOptions.h?



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:151-162
+  if (model.equals("static"))
+return llvm::Reloc::Static;
+  if (model.equals("pic"))
+return llvm::Reloc::PIC_;
+  if (model.equals("dynamic-no-pic"))
+return llvm::Reloc::DynamicNoPIC;
+  if (model.equals("ropi"))

Only `-fpic` and `-fpie` are tested/supported right? Please, could you trim 
this accordingly? Or, alternatively, expand the test.



Comment at: flang/test/Driver/pic-flags.f90:3
 
-! RUN: %flang -### %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fpie 2>&1 | FileCheck %s 
--check-prefix=CHECK-PIE
-
-! CHECK-NOPIE: "-fc1"
-! CHECk-NOPIE-NOT: "-fpic"
-! CHECK-NOPIE: "{{.*}}ld"
-! CHECK-NOPIE-NOT: "-pie"
-
-! CHECK-PIE: "-fc1"
-!! TODO Once Flang supports `-fpie`, it //should// use -fpic when invoking 
`flang -fc1`. Update the following line once `-fpie` is
-! available.
-! CHECk-PIE-NOT: "-fpic"
-! CHECK-PIE: "{{.*}}ld"
-! CHECK-PIE-NOT: "-pie"
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fno-pie 
2>&1 | FileCheck %s --check-prefix=CHECK-NOPIE
+

mnadeem wrote:
> awarzynski wrote:
> > Why would you replace `-###` with `-v`? Same for other RUN lines. 
> I needed the command to run because I added check lines for the emitted LLVM 
> IR, for example:
> ! CHECK-PIE-LEVEL1: !"PIC Level", i32 1}
> ! CHECK-PIE-LEVEL1: !"PIE Level", i32 1}
Ah, of course! Thanks, I missed that earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131533

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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via Phabricator via cfe-commits]

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

update the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,8 +141,10 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
-  continue; // a "hole" for a subobject that was not explicitly initialized
+// Skip for a broken initializer or if it is a "hole" in a subobject that
+// was not explicitly initialized.
+if (!Init || llvm::isa(Init))
+  continue;
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
 if (BraceElidedSubobject &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,8 +141,10 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
-  continue; // a "hole" for a subobject that was not explicitly initialized
+// Skip for a broken initializer or if it is a "hole" in a subobject that
+// was not explicitly initialized.
+if (!Init || llvm::isa(Init))
+  continue;
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
 if (BraceElidedSubobject &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:144
 });
-if (llvm::isa(Init))
+if (!Init || llvm::isa(Init))
   continue; // a "hole" for a subobject that was not explicitly initialized

kadircet wrote:
> nit: can you also update the comment to mention `broken initializer` (and 
> maybe even a fixme to handle these, as in theory this is likely spelled in 
> the code, but wasn't retained in the AST even as a `recoveryexpr`, hence we 
> still have a place to attach the hint)
Done, updated the comment. I'd rather leave out the FIXME (it is unclear that 
we will address it).

The InitListExpr is tricky, the AST nodes is preserved in the syntactic form 
for InitListExpr (go-to-definition actually works on the broken initializer 
`A()` ), but here we're using the *semantic*-form...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

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

[clang-tools-extra] 06b97b4 - [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-08-12T14:37:46+02:00
New Revision: 06b97b4985ad0415f6cde4baad2bc7d73b456244

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

LOG: [clangd] Fix an inlay-hint crash on a broken designator.

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

Added: 


Modified: 
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 8983f6b7a9eb8..bff07dd078cc9 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -141,8 +141,10 @@ void collectDesignators(const InitListExpr *Sem,
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
-  continue; // a "hole" for a subobject that was not explicitly initialized
+// Skip for a broken initializer or if it is a "hole" in a subobject that
+// was not explicitly initialized.
+if (!Init || llvm::isa(Init))
+  continue;
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
 if (BraceElidedSubobject &&

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 87bb0cfc01f60..a429c08991879 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@ TEST(DesignatorHints, OnlyAggregateInit) {
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;



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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG06b97b4985ad: [clangd] Fix an inlay-hint crash on a broken 
designator. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131696

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,8 +141,10 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
-  continue; // a "hole" for a subobject that was not explicitly initialized
+// Skip for a broken initializer or if it is a "hole" in a subobject that
+// was not explicitly initialized.
+if (!Init || llvm::isa(Init))
+  continue;
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
 if (BraceElidedSubobject &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,17 @@
   )cpp" /*no designator hints expected (but param hints!)*/);
 }
 
+TEST(DesignatorHints, NoCrash) {
+  assertDesignatorHints(R"cpp(
+/*error-ok*/
+struct A {};
+struct Foo {int a; int b;};
+void test() {
+  Foo f{A(), $b[[1]]};
+}
+  )cpp", ExpectedHint{".b=", "b"});
+}
+
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
 auto a = false;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -141,8 +141,10 @@
   Fields.next();   // Always advance to the next subobject name.
   Prefix.resize(Size); // Erase any designator we appended.
 });
-if (llvm::isa(Init))
-  continue; // a "hole" for a subobject that was not explicitly initialized
+// Skip for a broken initializer or if it is a "hole" in a subobject that
+// was not explicitly initialized.
+if (!Init || llvm::isa(Init))
+  continue;
 
 const auto *BraceElidedSubobject = llvm::dyn_cast(Init);
 if (BraceElidedSubobject &&
___
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

[Shawn Zhong via Phabricator via cfe-commits]

ShawnZhong 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;

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? 



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:
> > 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"); }
}
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.

Did @royjacobson 's patch make it to the 15.0 branch?  If so, Roy: After this 
is committed, can you file to get this merged into the 15.x branch?  This seems 
trivial enough (HAH!) of a fix that it is zero risk, and also keeps it from 
being all that embarassing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131730

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

[clang] e857896 - [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via cfe-commits]

Author: Joe Loser
Date: 2022-08-12T06:55:59-06:00
New Revision: e8578968f684997840f680ed62bff5cad0accc13

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

LOG: [ADT] Replace STLForwardCompat.h's C++17 equivalents

STLForwardCompat.h defines several utilities and type traits to mimic that of
the ones in the C++17 standard library. Now that LLVM is built with the C++17
standards mode, remove use of these equivalents in favor of the ones from the
standard library.

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

Added: 


Modified: 
clang/include/clang/Basic/DirectoryEntry.h
clang/include/clang/Basic/FileEntry.h
llvm/include/llvm/ADT/Any.h
llvm/include/llvm/ADT/FunctionExtras.h
llvm/include/llvm/ADT/Optional.h
llvm/include/llvm/ADT/STLExtras.h
llvm/include/llvm/Support/HashBuilder.h
llvm/unittests/ADT/OptionalTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DirectoryEntry.h 
b/clang/include/clang/Basic/DirectoryEntry.h
index f1ec63d975a37..19d52c09dbcce 100644
--- a/clang/include/clang/Basic/DirectoryEntry.h
+++ b/clang/include/clang/Basic/DirectoryEntry.h
@@ -22,6 +22,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorOr.h"
 
+#include 
+
 namespace clang {
 namespace FileMgr {
 
@@ -125,7 +127,7 @@ template  class MapEntryOptionalStorage {
   MapEntryOptionalStorage() : MaybeRef(optional_none_tag()) {}
 
   template 
-  explicit MapEntryOptionalStorage(llvm::in_place_t, ArgTypes &&...Args)
+  explicit MapEntryOptionalStorage(std::in_place_t, ArgTypes &&...Args)
   : MaybeRef(std::forward(Args)...) {}
 
   void reset() { MaybeRef = optional_none_tag(); }
@@ -189,8 +191,8 @@ class OptionalStorage
   OptionalStorage() = default;
 
   template 
-  explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
-  : StorageImpl(in_place_t{}, std::forward(Args)...) {}
+  explicit OptionalStorage(std::in_place_t, ArgTypes &&...Args)
+  : StorageImpl(std::in_place_t{}, std::forward(Args)...) {}
 
   OptionalStorage &operator=(clang::DirectoryEntryRef Ref) {
 StorageImpl::operator=(Ref);

diff  --git a/clang/include/clang/Basic/FileEntry.h 
b/clang/include/clang/Basic/FileEntry.h
index 3ca07cb422b64..d9b92a73a1d82 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -24,6 +24,8 @@
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem/UniqueID.h"
 
+#include 
+
 namespace llvm {
 
 class MemoryBuffer;
@@ -222,8 +224,8 @@ class OptionalStorage
   OptionalStorage() = default;
 
   template 
-  explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
-  : StorageImpl(in_place_t{}, std::forward(Args)...) {}
+  explicit OptionalStorage(std::in_place_t, ArgTypes &&...Args)
+  : StorageImpl(std::in_place_t{}, std::forward(Args)...) {}
 
   OptionalStorage &operator=(clang::FileEntryRef Ref) {
 StorageImpl::operator=(Ref);

diff  --git a/llvm/include/llvm/ADT/Any.h b/llvm/include/llvm/ADT/Any.h
index 1c7ba03717816..649a9986c0a41 100644
--- a/llvm/include/llvm/ADT/Any.h
+++ b/llvm/include/llvm/ADT/Any.h
@@ -68,8 +68,8 @@ class LLVM_EXTERNAL_VISIBILITY Any {
   // instead.
   template , Any>>,
+std::conjunction<
+std::negation, Any>>,
 // We also disable this overload when an `Any` object can 
be
 // converted to the parameter type because in that case,
 // this constructor may combine with that conversion during
@@ -80,7 +80,7 @@ class LLVM_EXTERNAL_VISIBILITY Any {
 // DR in `std::any` as well, but we're going ahead and
 // adopting it to work-around usage of `Any` with types 
that
 // need to be implicitly convertible from an `Any`.
-llvm::negation>>,
+std::negation>>,
 std::is_copy_constructible>>::value,
 int> = 0>
   Any(T &&Value) {

diff  --git a/llvm/include/llvm/ADT/FunctionExtras.h 
b/llvm/include/llvm/ADT/FunctionExtras.h
index 5a37417ddde5d..8ff0fb787a77d 100644
--- a/llvm/include/llvm/ADT/FunctionExtras.h
+++ b/llvm/include/llvm/ADT/FunctionExtras.h
@@ -65,7 +65,7 @@ template 
 using EnableUnlessSameType =
 std::enable_if_t, ThisT>::value>;
 template 
-using EnableIfCallable = std::enable_if_t,
 
std::is_same()(std::declval()...)),
  Ret>,

diff  --git a/llvm/include/llvm/ADT/Optional.h 
b/llvm/include/llvm/ADT/Optional.h
index 09770c4f94a0e..1862a2d703aad 100644
--- a/llvm/include/llvm/ADT/Optional.h
+++ b/llvm/include/llvm/ADT/Optional.h
@@ -81,7 +81,7 @@ class OptionalStorage {
   }
 
   template 
-  constexpr explicit OptionalStorage(in_place_t, Args &&...args)
+  constexpr explicit

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8578968f684: [ADT] Replace STLForwardCompat.h's C++17 
equivalents (authored by joe_loser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

Files:
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  llvm/include/llvm/ADT/Any.h
  llvm/include/llvm/ADT/FunctionExtras.h
  llvm/include/llvm/ADT/Optional.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -14,7 +14,7 @@
 #include "gtest/gtest.h"
 
 #include 
-
+#include 
 
 using namespace llvm;
 
@@ -201,7 +201,7 @@
 
 TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
   NonDefaultConstructible::ResetCounts();
-  { Optional A{in_place, 1}; }
+  { Optional A{std::in_place, 1}; }
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
@@ -247,7 +247,7 @@
 TEST(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(A.has_value());
@@ -266,12 +266,12 @@
 TEST(OptionalTest, InPlaceConstructionMultiArgConstructorTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 EXPECT_TRUE(A.has_value());
 EXPECT_TRUE(A.has_value());
 EXPECT_EQ(1, A->x);
 EXPECT_EQ(2, A->y);
-Optional B{in_place, 5, false};
+Optional B{std::in_place, 5, false};
 EXPECT_TRUE(B.has_value());
 EXPECT_TRUE(B.has_value());
 EXPECT_EQ(5, B->x);
@@ -284,7 +284,7 @@
 TEST(OptionalTest, InPlaceConstructionAndEmplaceEquivalentTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 Optional B;
 B.emplace(1, 2);
 EXPECT_EQ(0u, MultiArgConstructor::Destructions);
@@ -442,7 +442,7 @@
 
 TEST(OptionalTest, ImmovableInPlaceConstruction) {
   Immovable::ResetCounts();
-  Optional A{in_place, 4};
+  Optional A{std::in_place, 4};
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
Index: llvm/include/llvm/Support/HashBuilder.h
===
--- llvm/include/llvm/Support/HashBuilder.h
+++ llvm/include/llvm/Support/HashBuilder.h
@@ -76,7 +76,7 @@
 
   template 
   explicit HashBuilderBase(ArgTypes &&...Args)
-  : OptionalHasher(in_place, std::forward(Args)...),
+  : OptionalHasher(std::in_place, std::forward(Args)...),
 Hasher(*OptionalHasher) {}
 
 private:
Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -155,12 +155,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct TypesAreDistinct;
@@ -1386,12 +1386,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct Visitor;
@@ -1550,7 +1550,7 @@
 template 
 // We can use qsort if the iterator type is a pointer and the underlying value
 // is trivially copyable.
-using sort_trivially_copyable = conjunction<
+using sort_trivially_copyable = std::conjunction<
 std::is_pointer,
 std::is_trivially_copyable::value_type>>;
 } // namespace detail
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -81,7 +81,7 @@
   }
 
   template 
-  constexpr explicit OptionalStorage(in_place_t, Args &&...args)
+  constexpr explicit OptionalStorage(std::in_place_t, Args &&...args)
   : val(std::forward(args)...), hasVal(true) {}
 
   void reset() noexcept {
@@ -196,7 +196,7 @@
   OptionalStorage &operator=(OptionalStorage &&other) = default;
 

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

A different but slightly similar case, where I've run into these errors too, 
FWIW: On ARM, when compiling with a hardfloat target, Clang does consider a 
function pointer with `__attribute__((pcs("aapcs-vfp")))` different from one 
without, even if the default still in the end would end up equal to it. I guess 
the problem there is that Clang doesn't know what the default implicit calling 
convention is (and that's something that ends up resolved on the LLVM side 
while lowering code) - while e.g. Clang does know that `__cdecl` is equivalent 
to not declaring any calling convention at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

SO, I charted the differences in runtimes, and my patch is BETTER time-wise in 
release mode, but WORSE with asserts enabled.  I don't know of any expensive 
assert that I added.  SO, if everyone could review this, I'd love to get this 
committed to see if it beats the buildbots this time:)


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

https://reviews.llvm.org/D126907

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

In D131730#3718921 , @erichkeane 
wrote:

> Did @royjacobson 's patch make it to the 15.0 branch?  If so, Roy: After this 
> is committed, can you file to get this merged into the 15.x branch?  This 
> seems trivial enough (HAH!) of a fix that it is zero risk, and also keeps it 
> from being all that embarrassing here.

Yes this is definitely in 15.  During RC testing the diagnostic fired.  The 
test build where I followed the suggestion and replaced __has_trivial_copy with 
__is_trivially_constructible didn't go very well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131730

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

In D131730#3718430 , @royjacobson 
wrote:

> Could you add name+email for the commit message?

Zachary Henkel
za...@microsoft.com
Github Username: ZacharyHenkel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131730

___
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

[Shawn Zhong via Phabricator via cfe-commits]

ShawnZhong 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:
> 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121365

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Pre-commit CI found build errors that should be addressed.




Comment at: clang/include/clang/AST/TransformTypeTraits.def:1-2
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.

Have to fix the formatting manually though.



Comment at: clang/include/clang/Sema/DeclSpec.h:424-431
+  static bool isTransformTypeTrait(TST T) {
+constexpr TST Traits[] = {
+#define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) TST_##Trait,
+#include "clang/AST/TransformTypeTraits.def"
+};
+
+return (T >= Traits[0] && T <= std::end(Traits)[-1]);

Errr, this isn't Python, so that `[-1]` terrifies me. It took me a good solid 
ten minutes to convince myself this was actually valid. Any interest in 
something along the lines of this form?



Comment at: clang/lib/AST/TypePrinter.cpp:1158
raw_ostream &OS) {
   IncludeStrongLifetimeRAII Strong(Policy);
 }

cjdb wrote:
> rsmith wrote:
> > Remove this line too. No point building an RAII scope with nothing in it.
> Can we get rid of this function entirely in that case?
I believe you can.



Comment at: clang/lib/Parse/ParseDecl.cpp:3475
+case tok::identifier:
+ParseIdentifier : {
   // This identifier can only be a typedef name if we haven't already seen





Comment at: clang/lib/Sema/SemaType.cpp:9267
+  constexpr auto UKind = UTTKind::RemovePointer;
+  // We don't want block pointers or ObjectiveC's id type
+  if (!BaseType->isAnyPointerType() || BaseType->isObjCIdType())





Comment at: clang/lib/Sema/SemaType.cpp:9305-9308
+  else if (const auto *AT = Context.getAsArrayType(BaseType))
+return AT->getElementType();
+  else
+return BaseType;





Comment at: clang/lib/Sema/SemaType.cpp:9350-9352
+if (auto BitInt = dyn_cast(Underlying)) {
+  return S.Context.getBitIntType(!IsMakeSigned, BitInt->getNumBits());
 }

An interesting test case for you to consider (both for enumeration underlying 
types as well as types in general): `signed _BitInt` requires at least two 
bits, so trying to do a make_signed on `_BitInt(1)` should be diagnosed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

And for context, here is an actual bug this check would help find:

https://github.com/ROCmSoftwarePlatform/MIOpen/pull/1578#discussion_r889038610


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

https://reviews.llvm.org/D129570

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

[clang] fba88ad - [Sema] Always perform ADL when searching for transormed operators in C++20

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-08-12T16:07:02+02:00
New Revision: fba88adc8961fc8b8d7dece7bfecbfc31c652aff

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

LOG: [Sema] Always perform ADL when searching for transormed operators in C++20

This fixes compilation errors in the following code with
latest libc++ and libstdc++:
```cpp

int main() {
  std::tuple s;
  std::tuple sv;
  s < sv;
}
```

See https://gcc.godbolt.org/z/vGvEhxWvh for the error.

This used to happen because repeated template instantiations during
C++20 constraint satisfaction will cause a call to 
`RebuildCXXRewrittenBinaryOperator`
with `PerformADL` set to false, see 
https://github.com/llvm/llvm-project/blob/59351fe340f20a605bae53260ed30a8e0fd95cb6/clang/lib/Sema/TreeTransform.h#L2737

Committing urgently to unbreak breakages we see in our configurations.
The change seems relatively safe and the right thing to do.

However, I will follow-up with tests and investigation on whether we
need to do this extra semantic checking in the first place next week.

Added: 


Modified: 
clang/lib/Sema/SemaOverload.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index c88413b3e5de..91e308d0d0fe 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13930,7 +13930,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation 
OpLoc,
 
 R = CreateOverloadedBinOp(
 OpLoc, Opc, Fns, IsReversed ? ZeroLiteral : R.get(),
-IsReversed ? R.get() : ZeroLiteral, PerformADL,
+IsReversed ? R.get() : ZeroLiteral, /*PerformADL=*/true,
 /*AllowRewrittenCandidates=*/false);
 
 popCodeSynthesisContext();



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

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

This doesn't actually test much, only once case/compilation is covered. In the 
second function nothing specific to LLVM as impl is checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131763

___
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

In D131763#3719132 , @jdoerfert wrote:

> This doesn't actually test much, only once case/compilation is covered. In 
> the second function nothing specific to LLVM as impl is checked.

The second function, is the only place in llvm-project where vendor(llvm) is 
being tested for a non-error test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131763

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[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, but you should add a release note for the change.




Comment at: clang/lib/Basic/TypeTraits.cpp:64
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};

inclyc wrote:
> inclyc wrote:
> > shafik wrote:
> > > @aaron.ballman do we really have to include this three times? We are 
> > > defining different macros so shouldn't we be able to include is just 
> > > once? 
> > > 
> > > I see we do this in several other places but a few we don't.
> > I've tried 
> > 
> > ```
> > #define TYPE_TRAIT(N, I, K) N,
> > #include "clang/Basic/TokenKinds.def"
> > ```
> > 
> > But using enum `TypeTrait` as array index seems to have incorrect mapping.
> > @aaron.ballman do we really have to include this three times? We are 
> > defining different macros so shouldn't we be able to include is just once? 
> > 
> > I see we do this in several other places but a few we don't.
> 
> Type trait definitions in `#include "clang/Basic/TokenKinds.def"` may not be 
> sorted by the number of arguments.
> 
> Example:
> 
> ```
> #define TYPE_TRAIT_1(some_stuff)
> #define TYPE_TRAIT_N(some_stuff)
> #define TYPE_TRAIT_2(some_stuff)
> ```
> 
> Might be necessary to include this 3 times to get sorted layouts, like
> 
> `[1, 1, 1, 2, 2, 2, 0, 0]`
Yeah, I think we ultimately either need to reorder the TokenKinds.def file to 
be in sorted order (fragile, surprising, tight coupling) or we have to do this 
dance because we need the initialization itself to be sorted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via Phabricator via cfe-commits]

samestep created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
samestep 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/D131779

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -64,7 +64,10 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match,
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -3896,8 +3899,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{llvm::Optional()}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -3926,8 +3928,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -3956,8 +3957,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
@@ -3997,8 +3997,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTwoLayers) {
@@ -4029,8 +4028,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleLines) {
@@ -4071,8 +4069,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleBlocks) {
@@ -4117,8 +4114,7 @@
 EXPECT_TRUE(Env.flowConditionImplies(BazVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(BazVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnVoid) {
@@ -4138,8 +4134,7 @@
 ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
 // This just tests that the analysis doesn't crash.
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnTrue) {
@@ -4166,8 +4161,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal

[PATCH] D131780: [clang-tidy] Do not trigger cppcoreguidelines-avoid-const-or-ref-data-members on lambda captures

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp created this revision.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Lambdas are implemented as regular classes internally,
and the captured variables end up as members there.
Do not diagnose those - the check should cover only
regular classes and structs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131780

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -167,3 +167,22 @@
 TemplatedConstRef t3{123};
 TemplatedRefRef t4{123};
 TemplatedRef t5{t1.t};
+
+// Lambdas capturing const or ref members should not trigger warnings
+void lambdas()
+{
+  int x1{123};
+  auto y1 = [x1]{};
+
+  const int x2{123};
+  auto y2 = [x2]{};
+
+  const int& x3{123};
+  auto y3 = [x3]{};
+
+  int&& x4{123};
+  auto y4 = [x4]{};
+
+  int& x5{x1};
+  auto y5 = [x5]{};
+}
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
@@ -15,12 +15,23 @@
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
+namespace {
+
+AST_MATCHER(FieldDecl, isMemberOfLambda) {
+  return Node.getParent()->isLambda();
+}
+
+} // namespace
 
 void AvoidConstOrRefDataMembersCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  fieldDecl(hasType(hasCanonicalType(referenceType(.bind("ref"), this);
-  Finder->addMatcher(
-  fieldDecl(hasType(qualType(isConstQualified(.bind("const"), this);
+  Finder->addMatcher(fieldDecl(unless(isMemberOfLambda()),
+   hasType(hasCanonicalType(referenceType(
+ .bind("ref"),
+ this);
+  Finder->addMatcher(fieldDecl(unless(isMemberOfLambda()),
+   hasType(qualType(isConstQualified(
+ .bind("const"),
+ this);
 }
 
 void AvoidConstOrRefDataMembersCheck::check(


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -167,3 +167,22 @@
 TemplatedConstRef t3{123};
 TemplatedRefRef t4{123};
 TemplatedRef t5{t1.t};
+
+// Lambdas capturing const or ref members should not trigger warnings
+void lambdas()
+{
+  int x1{123};
+  auto y1 = [x1]{};
+
+  const int x2{123};
+  auto y2 = [x2]{};
+
+  const int& x3{123};
+  auto y3 = [x3]{};
+
+  int&& x4{123};
+  auto y4 = [x4]{};
+
+  int& x5{x1};
+  auto y5 = [x5]{};
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
@@ -15,12 +15,23 @@
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
+namespace {
+
+AST_MATCHER(FieldDecl, isMemberOfLambda) {
+  return Node.getParent()->isLambda();
+}
+
+} // namespace
 
 void AvoidConstOrRefDataMembersCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  fieldDecl(hasType(hasCanonicalType(referenceType(.bind("ref"), this);
-  Finder->addMatcher(
-  fieldDecl(hasType(qualType(isConstQualified(.bind("const"), this);
+  Finder->addMatcher(fieldDecl(unless(isMemberOfLambda()),
+   hasType(hasCanonicalType(referenceType(
+ .bind("ref"),
+ this);
+  Finder->addMatcher(fieldDecl(unless(isMemberOfLambda()),
+   hasType(qualType(isConstQualified(
+ .bind("const"),
+ this);
 }
 
 void AvoidConstOrRefDataMembersCheck::check(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

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

xazax.hun added a comment.

LLVM switched to C++17 recently. I am not sure whether this means 
`std::optional` is preferred over `llvm::Optional`.




Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3902
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{llvm::Optional()}});
 }

I think `llvm::None` is a more concise way to create an empty optional (akin to 
`std::nullopt`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

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

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:70
+   ? TransferOptions{}
+   : llvm::Optional()}) {}
 





Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:31
+  /// unsupported.
+  llvm::Optional ContextSensitiveOpts;
 };

Perhaps keep the name `ContextSensitive`? In the context of `TransferOptions` 
it seems clear what this is. 



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:322
 case CFGElement::Initializer:
-  if (Analysis.applyBuiltinTransfer())
+  if (Analysis.builtinTransferOptions().hasValue())
 transferCFGInitializer(*Element.getAs(), State);

For consistency.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:69
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);





Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3902
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{llvm::Optional()}});
 }

xazax.hun wrote:
> I think `llvm::None` is a more concise way to create an empty optional (akin 
> to `std::nullopt`).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

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

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via Phabricator via cfe-commits]

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

Use llvm::None per Gábor's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -64,7 +64,10 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match,
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -3896,8 +3899,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{/*.ContextSensitiveOpts=*/llvm::None}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -3926,8 +3928,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -3956,8 +3957,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
@@ -3997,8 +3997,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTwoLayers) {
@@ -4029,8 +4028,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleLines) {
@@ -4071,8 +4069,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleBlocks) {
@@ -4117,8 +4114,7 @@
 EXPECT_TRUE(Env.flowConditionImplies(BazVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(BazVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnVoid) {
@@ -4138,8 +4134,7 @@
 ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
 // This just tests that the analysis doesn't crash.
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnTrue) {
@@ -4166,8 +4161,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.ApplyBuilti

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:70
+   ? TransferOptions{}
+   : llvm::Optional()}) {}
 

sgatev wrote:
> 
I tried that, but I get the following error message:

> Incompatible operand types ('TransferOptions' and 'const NoneType')



Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:31
+  /// unsupported.
+  llvm::Optional ContextSensitiveOpts;
 };

sgatev wrote:
> Perhaps keep the name `ContextSensitive`? In the context of `TransferOptions` 
> it seems clear what this is. 
I don't have a strong preference either way; does anyone else have an opinion 
on this?



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:322
 case CFGElement::Initializer:
-  if (Analysis.applyBuiltinTransfer())
+  if (Analysis.builtinTransferOptions().hasValue())
 transferCFGInitializer(*Element.getAs(), State);

sgatev wrote:
> For consistency.
Ah thanks! Will do.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:69
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);

sgatev wrote:
> 
Same error mentioned above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

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

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via Phabricator via cfe-commits]

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

Remove unnecessary explicit call to hasValue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -64,7 +64,10 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match,
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -3896,8 +3899,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{/*.ContextSensitiveOpts=*/llvm::None}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -3926,8 +3928,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -3956,8 +3957,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
@@ -3997,8 +3997,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTwoLayers) {
@@ -4029,8 +4028,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleLines) {
@@ -4071,8 +4069,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleBlocks) {
@@ -4117,8 +4114,7 @@
 EXPECT_TRUE(Env.flowConditionImplies(BazVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(BazVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnVoid) {
@@ -4138,8 +4134,7 @@
 ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
 // This just tests that the analysis doesn't crash.
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnTrue) {
@@ -4166,8 +4161,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.Appl

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

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

add release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5427,9 +5427,26 @@
 }
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);
 
+  bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
+
   ExprResult CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
 SourceLocation OpLoc,
 UnaryExprOrTypeTrait ExprKind,
Index: clang/include/clang/Basic/TypeTra

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

[Daniel Ruoso via Phabricator via cfe-commits]

ruoso added inline comments.



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

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.



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] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

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

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5427,9 +5427,26 @@
 }
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);
 
+  bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
+
   ExprResult CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
 SourceLocation OpLoc,
 UnaryExprOrTypeTrait ExprKind,
Index: clang/include/clang/Basic/TypeTraits.h
==

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:3
+
+const RWBuffer In;
+RWBuffer Out;

Why add const instead of using Buffer directly?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

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

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D131763#3719140 , @saiislam wrote:

> In D131763#3719132 , @jdoerfert 
> wrote:
>
>> This doesn't actually test much, only once case/compilation is covered. In 
>> the second function nothing specific to LLVM as impl is checked.
>
> The second function, is the only place in llvm-project where vendor(llvm) is 
> being tested for a non-error test.

Really?

  ag 'vendor\(llvm\)' clang/test/OpenMP --files-with-matches


  
  clang/test/OpenMP/begin_declare_variant_messages.c
  clang/test/OpenMP/begin_declare_variant_using_messages.cpp
  clang/test/OpenMP/declare_variant_ast_print.c
  clang/test/OpenMP/declare_variant_ast_print.cpp
  clang/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
  clang/test/OpenMP/declare_variant_messages.cpp
  clang/test/OpenMP/declare_variant_mixed_codegen.cpp
  clang/test/OpenMP/metadirective_ast_print.c
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nvptx_declare_variant_implementation_vendor_codegen.cpp
  clang/test/OpenMP/declare_variant_messages.c
  clang/test/OpenMP/metadirective_empty.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.cpp

That said, the above function still doesn't test anything wrt. llvm as impl 
anyway. We could just as well match amd or nvidia and the check lines still 
match just fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131763

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

[clang] e582519 - [clang] fix frontend crash when evaluating type trait

[YingChi Long via cfe-commits]

Author: YingChi Long
Date: 2022-08-13T00:02:19+08:00
New Revision: e5825190b8ad7ac8fe762fe4101cd4af04f4c057

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

LOG: [clang] fix frontend crash when evaluating type trait

Before this patch type traits are checked in Parser, so use type traits
directly did not cause assertion faults. However if type traits are initialized
from a template, we didn't perform arity checks before evaluating. This
patch moves arity checks from Parser to Sema, and performing arity
checks in Sema actions, so type traits get checked corretly.

Crash input:

```
template bool b = __is_constructible(Ts...);
bool x = b<>;
```

After this patch:

```
clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp:5:32: error: type 
trait requires 1 or more arguments; have 0 arguments
template bool b = __is_constructible(Ts...);
   ^~
clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp:6:10: note: in 
instantiation of variable template specialization 'b<>' requested here
bool x = b<>;
 ^
1 error generated.
```

See https://godbolt.org/z/q39W78hsK.

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

Reviewed By: aaron.ballman

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

Added: 
clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/TypeTraits.h
clang/include/clang/Sema/Sema.h
clang/lib/Basic/TypeTraits.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 572961939d859..6970149444d1e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -68,6 +68,9 @@ Bug Fixes
 - Fixed a crash-on-valid with consteval evaluation of a list-initialized
   constructor for a temporary object. This fixes
   `Issue 55871 `_.
+- Fix `#57008 `_ - Builtin
+  C++ language extension type traits instantiated by a template with unexpected
+  number of arguments cause an assertion fault.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Basic/TypeTraits.h 
b/clang/include/clang/Basic/TypeTraits.h
index a0f06bec6697b..eb8b1923152db 100644
--- a/clang/include/clang/Basic/TypeTraits.h
+++ b/clang/include/clang/Basic/TypeTraits.h
@@ -67,6 +67,10 @@ const char *getTraitName(UnaryExprOrTypeTrait T) 
LLVM_READONLY;
 const char *getTraitSpelling(TypeTrait T) LLVM_READONLY;
 const char *getTraitSpelling(ArrayTypeTrait T) LLVM_READONLY;
 const char *getTraitSpelling(UnaryExprOrTypeTrait T) LLVM_READONLY;
+
+/// Return the arity of the type trait \p T.
+unsigned getTypeTraitArity(TypeTrait T) LLVM_READONLY;
+
 } // namespace clang
 
 #endif

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bb2fa139af187..6908eaf8ff954 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@ class Sema final {
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);
 
+  bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
+
   ExprResult CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
 SourceLocation OpLoc,
 UnaryExprOrTypeTrait ExprKind,

diff  --git a/clang/lib/Basic/TypeTraits.cpp b/clang/lib/Basic/TypeTraits.cpp
index 3b723afff70b5..4dbf678dc395b 100644
--- a/clang/lib/Basic/TypeTraits.cpp
+++ b/clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@ static constexpr const char *UnaryExprOrTypeTraitSpellings[] 
= {
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@ const char *clang::getTraitSpelling(UnaryExprOrTypeTrait T) {
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 9bd89eddb4551..2f30075f7c3cd 1006

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5825190b8ad: [clang] fix frontend crash when evaluating 
type trait (authored by inclyc).

Changed prior to commit:
  https://reviews.llvm.org/D131423?vs=452199&id=452201#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5427,9 +5427,26 @@
 }
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);
 
+  bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
+
   ExprResult C

[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3f1a6bf1080: [clang][dataflow] Encode options using 
llvm::Optional (authored by samestep).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -64,7 +64,10 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match,
+  {ApplyBuiltinTransfer ? TransferOptions{}
+: llvm::Optional()},
+  Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -3896,8 +3899,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+  {TransferOptions{/*.ContextSensitiveOpts=*/llvm::None}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -3926,8 +3928,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -3956,8 +3957,7 @@
 *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
@@ -3997,8 +3997,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTwoLayers) {
@@ -4029,8 +4028,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleLines) {
@@ -4071,8 +4069,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(BarVal));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetMultipleBlocks) {
@@ -4117,8 +4114,7 @@
 EXPECT_TRUE(Env.flowConditionImplies(BazVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(BazVal)));
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnVoid) {
@@ -4138,8 +4134,7 @@
 ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
 // This just tests that the analysis doesn't crash.
   },
-  {/*.ApplyBuiltinTransfer=*/true,
-   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+  {TransferOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveReturnTrue) {
@@ -4166,8 +4161,7 @@
 *cast(Env.getValue(*FooDecl, S

[clang] b3f1a6b - [clang][dataflow] Encode options using llvm::Optional

[Sam Estep via cfe-commits]

Author: Sam Estep
Date: 2022-08-12T16:29:41Z
New Revision: b3f1a6bf1080fb67cb1760a924a56d38d51211aa

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

LOG: [clang][dataflow] Encode options using llvm::Optional

This patch restructures `DataflowAnalysisOptions` and `TransferOptions` to use 
`llvm::Optional`, in preparation for adding more sub-options to the 
`ContextSensitiveOptions` struct introduced here.

Reviewed By: sgatev, xazax.hun

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

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 99e16f8255446..4c785b21526da 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -65,8 +65,9 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis {
 
   /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
   explicit DataflowAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
-  : DataflowAnalysis(Context, DataflowAnalysisOptions{ApplyBuiltinTransfer,
-  TransferOptions{}}) 
{}
+  : DataflowAnalysis(Context, {ApplyBuiltinTransfer
+   ? TransferOptions{}
+   : llvm::Optional()}) {}
 
   explicit DataflowAnalysis(ASTContext &Context,
 DataflowAnalysisOptions Options)

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h 
b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
index 8babc5724d46e..93afcc284e1af 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -16,17 +16,19 @@
 
 #include "clang/AST/Stmt.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "llvm/ADT/Optional.h"
 
 namespace clang {
 namespace dataflow {
 
+struct ContextSensitiveOptions {};
+
 struct TransferOptions {
-  /// Determines whether to analyze function bodies when present in the
-  /// translation unit. Note: this is currently only meant to be used for
-  /// inlining of specialized model code, not for context-sensitive analysis of
-  /// arbitrary subject code. In particular, some fundamentals such as 
recursion
-  /// are explicitly unsupported.
-  bool ContextSensitive = false;
+  /// Options for analyzing function bodies when present in the translation
+  /// unit, or empty to disable context-sensitive analysis. Note that this is
+  /// fundamentally limited: some constructs, such as recursion, are explicitly
+  /// unsupported.
+  llvm::Optional ContextSensitiveOpts;
 };
 
 /// Maps statements to the environments of basic blocks that contain them.

diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h 
b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
index 3bd1c8e12e9b2..58acda7e6389d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -32,14 +32,11 @@ namespace clang {
 namespace dataflow {
 
 struct DataflowAnalysisOptions {
-  /// Determines whether to apply the built-in transfer functions.
+  /// Options for the built-in transfer functions, or empty to not apply them.
   // FIXME: Remove this option once the framework supports composing analyses
   // (at which point the built-in transfer functions can be simply a standalone
   // analysis).
-  bool ApplyBuiltinTransfer = true;
-
-  /// Only has an effect if `ApplyBuiltinTransfer` is true.
-  TransferOptions BuiltinTransferOptions;
+  llvm::Optional BuiltinTransferOpts = TransferOptions{};
 };
 
 /// Type-erased lattice element container.
@@ -87,13 +84,11 @@ class TypeErasedDataflowAnalysis : public 
Environment::ValueModel {
   virtual void transferTypeErased(const Stmt *, TypeErasedLattice &,
   Environment &) = 0;
 
-  /// Determines whether to apply the built-in transfer functions, which model
-  /// the heap and stack in the `Environment`.
-  bool applyBuiltinTransfer() const { return Options.ApplyBuiltinTransfer; }
-
-  /// Returns the options to be passed to the built-in transfer functions.
-  Transfe

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131528#3718405 , @mstorsjo wrote:

> In D131528#3716876 , @shafik wrote:
>
>> In D131528#3715841 , @thakis wrote:
>>
>>> We're also still seeing the diag fire after this: 
>>> https://ci.chromium.org/p/chromium/builders/ci/ToTLinux
>>>
>>> (And we cleaned up our codebase back when it was still an error.)
>>>
>>> Our bots have been red since the change to turn this into a warning landed.
>>
>> Apologies, my condition was not strict enough, I have put up D131704 
>> 
>
> Unfortunately, even after D131704  (with 
> both commits), I'm still hitting a couple cases that weren't an error before 
> this. With https://martin.st/temp/qt-jit-enum.cpp and 
> https://martin.st/temp/protobuf-wire-format.cpp, built with `clang -target 
> i686-w64-mingw32 -c qt-jit-enum.cpp -std=c++17 -Wno-user-defined-literals 
> -Wno-ignored-attributes` (and same for the other file, although `-std=c++17` 
> can be omitted for the protobuf source) I'm still hitting errors for things 
> that weren't even warned about before.

Thank you for those examples, it is super helpful to receive this feedback 
especially with simple reproducers.

This is expected, it is constant initialization and I verified that they can be 
turned into warning using `-Wno-error=enum-constexpr-conversion`

Both of these cases are undefined behavior and they can fixed a number of ways. 
One approach would be to give them a fixed underlying type either like `enum 
WireType : int` for example of making them a scoped enum like `enum class 
WireType`. Another solution would be to add the value you want to use an 
enumerator explicitly.


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] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131351#3718982 , @mstorsjo wrote:

> A different but slightly similar case, where I've run into these errors too, 
> FWIW: On ARM, when compiling with a hardfloat target, Clang does consider a 
> function pointer with `__attribute__((pcs("aapcs-vfp")))` different from one 
> without, even if the default still in the end would end up equal to it. I 
> guess the problem there is that Clang doesn't know what the default implicit 
> calling convention is (and that's something that ends up resolved on the LLVM 
> side while lowering code) - while e.g. Clang does know that `__cdecl` is 
> equivalent to not declaring any calling convention at all.

I think that one is a bug with the target architecture not properly specifying 
what the default calling convention is that case. My guess is they're falling 
back on cdecl as the default calling convention in the frontend when they 
should be looking at the target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

[PATCH] 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.

Was this and previous change intended to capture this case?
 const enum NumberType neg_one = (enum NumberType) ((enum NumberType) 0 - (enum 
NumberType) 1);


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] D131789: [clang-tools-extra] Rewrite prints in python3 compatible way

[Danil Sidoruk via Phabricator via cfe-commits]

eoanermine created this revision.
eoanermine added reviewers: klimek, HazardyKnusperkeks.
eoanermine added a project: clang-tools-extra.
Herald added a project: All.
eoanermine requested review of this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131789

Files:
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py


Index: 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -39,7 +39,7 @@
   result = './'
   while not os.path.isfile(os.path.join(result, path)):
 if os.path.realpath(result) == '/':
-  print 'Error: could not find compilation database.'
+  print('Error: could not find compilation database.')
   sys.exit(1)
 result += '../'
   return os.path.realpath(result)
@@ -49,7 +49,7 @@
   """Merge all symbol files (yaml) in a given directory into a single file."""
   invocation = [args.binary, '-merge-dir='+directory, args.saving_path]
   subprocess.call(invocation)
-  print 'Merge is finished. Saving results in ' + args.saving_path
+  print('Merge is finished. Saving results in ' + args.saving_path)
 
 
 def run_find_all_symbols(args, tmpdir, build_path, queue):
@@ -118,7 +118,7 @@
   except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
-print '\nCtrl-C detected, goodbye.'
+print('\nCtrl-C detected, goodbye.')
 os.kill(0, 9)
 
 


Index: clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -39,7 +39,7 @@
   result = './'
   while not os.path.isfile(os.path.join(result, path)):
 if os.path.realpath(result) == '/':
-  print 'Error: could not find compilation database.'
+  print('Error: could not find compilation database.')
   sys.exit(1)
 result += '../'
   return os.path.realpath(result)
@@ -49,7 +49,7 @@
   """Merge all symbol files (yaml) in a given directory into a single file."""
   invocation = [args.binary, '-merge-dir='+directory, args.saving_path]
   subprocess.call(invocation)
-  print 'Merge is finished. Saving results in ' + args.saving_path
+  print('Merge is finished. Saving results in ' + args.saving_path)
 
 
 def run_find_all_symbols(args, tmpdir, build_path, queue):
@@ -118,7 +118,7 @@
   except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
-print '\nCtrl-C detected, goodbye.'
+print('\nCtrl-C detected, goodbye.')
 os.kill(0, 9)
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/include/llvm/ADT/iterator_range.h:50
+/// As of C++17, the types of the begin-expr and the end-expr do not have to be
+/// the same in 'based-range for loop'. This class represents a tag that should
+/// be returned from the 'end' member function of an iterator.

"based-range" should be "range-based" shouldn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131448

___
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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D131528#3719414 , @shafik wrote:

> In D131528#3718405 , @mstorsjo 
> wrote:
>
>> In D131528#3716876 , @shafik wrote:
>>
>>> In D131528#3715841 , @thakis 
>>> wrote:
>>>
 We're also still seeing the diag fire after this: 
 https://ci.chromium.org/p/chromium/builders/ci/ToTLinux

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

 Our bots have been red since the change to turn this into a warning landed.
>>>
>>> Apologies, my condition was not strict enough, I have put up D131704 
>>> 
>>
>> Unfortunately, even after D131704  (with 
>> both commits), I'm still hitting a couple cases that weren't an error before 
>> this. With https://martin.st/temp/qt-jit-enum.cpp and 
>> https://martin.st/temp/protobuf-wire-format.cpp, built with `clang -target 
>> i686-w64-mingw32 -c qt-jit-enum.cpp -std=c++17 -Wno-user-defined-literals 
>> -Wno-ignored-attributes` (and same for the other file, although `-std=c++17` 
>> can be omitted for the protobuf source) I'm still hitting errors for things 
>> that weren't even warned about before.
>
> Thank you for those examples, it is super helpful to receive this feedback 
> especially with simple reproducers.
>
> This is expected, it is constant initialization and I verified that they can 
> be turned into warning using `-Wno-error=enum-constexpr-conversion`

Ok, but however, before D131307  which 
allowed to downgrade the error to a warning, these two source examples didn't 
produce any single warning or error (relating to the enums) at all - and AFAIK 
the intent of D131307  was to lower the 
impact of the enum-conversion error, not widen the scope of what it covers - 
right?


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] D131793: Fix assert on startup in clang-doc

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

When using `clang-doc --format=html` it will crash on startup because of an 
assertion doing a self-assignment of a `SmallString`. This patch removes the 
self-assignment by creating an intermediate copy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131793

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131739: Fix some clang-doc issues.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 452227.
brettw edited the summary of this revision.
brettw added a comment.

Startup assert fix broken out into https://reviews.llvm.org/D131793


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

https://reviews.llvm.org/D131739

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp


Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   RecordInfo I;
   I.Name = "r";
   I.Path = "path/to/A";
+  I.IsTypeDef = true;
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType: Class
+IsTypeDef:   true
 Members:
   - Type:
   Name:'int'
@@ -154,6 +156,7 @@
   - USR: ''
 Name:'F'
 Path:'path/to/F'
+TagType: Struct
 Members:
   - Type:
   Name:'int'
Index: clang-tools-extra/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MergeTest.cpp
@@ -78,6 +78,7 @@
 TEST(MergeTest, mergeRecordInfos) {
   RecordInfo One;
   One.Name = "r";
+  One.IsTypeDef = true;
   One.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   One.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -119,6 +120,7 @@
 
   auto Expected = std::make_unique();
   Expected->Name = "r";
+  Expected->IsTypeDef = true;
   Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -127,7 +127,8 @@
 
 static void RecordInfoMapping(IO &IO, RecordInfo &I) {
   SymbolInfoMapping(IO, I);
-  IO.mapOptional("TagType", I.TagType, clang::TagTypeKind::TTK_Struct);
+  IO.mapOptional("TagType", I.TagType);
+  IO.mapOptional("IsTypeDef", I.IsTypeDef, false);
   IO.mapOptional("Members", I.Members);
   IO.mapOptional("Bases", I.Bases);
   IO.mapOptional("Parents", I.Parents, llvm::SmallVector());
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -348,10 +348,15 @@
 
   void merge(RecordInfo &&I);
 
-  TagTypeKind TagType = TagTypeKind::TTK_Struct; // Type of this record
- // (struct, class, union,
- // interface).
-  bool IsTypeDef = false; // Indicates if record was declared using typedef
+  // Type of this record (struct, class, union, interface).
+  TagTypeKind TagType = TagTypeKind::TTK_Struct;
+
+  // Indicates if the record was declared using a typedef. Things like 
anonymous
+  // structs in a typedef:
+  //   typedef struct { ... } foo_t;
+  // are converted into records with the typedef as the Name + this flag set.
+  bool IsTypeDef = false;
+
   llvm::SmallVector
   Members; // List of info about record 
members.
   llvm::SmallVector Parents; // List of base/parent records
Index: clang-tools-extra/clang-doc/Representation.cpp
===
--- clang-tools-extra/clang-doc/Representation.cpp
+++ clang-tools-extra/clang-doc/Representation.cpp
@@ -222,6 +222,7 @@
   assert(mergeable(Other));
   if (!TagType)
 TagType = Other.TagType;
+  IsTypeDef = IsTypeDef || Other.IsTypeDef;
   if (Members.empty())
 Members = std::move(Other.Members);
   if (Bases.empty())


Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   RecordInfo I;
   I.Name = "r";
   I.Path = "path/to/A";
+  I.IsTypeDef = true;
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType:

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

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 452232.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Limit to lower case.


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/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Sema/SemaDecl.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 a : register(i0) {
+
+}
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+
+}
+// expected-error@+1 {{register number should be an integer}}
+cbuffer a : register(bf, s2) {
+
+}
+// expected-error@+1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : 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 B : 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 B : 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,90 @@
   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;
+}
+
+

[libunwind] b559777 - [libunwind] Remove __ANDROID_API__ < 18 workaround

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-08-12T10:46:46-07:00
New Revision: b559777c308dcdf40a5dc8dffd35867083a46c3c

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

LOG: [libunwind] Remove __ANDROID_API__ < 18 workaround

https://github.com/android/ndk/wiki/Changelog-r24 shows that the NDK has
moved forward to at least a minimum target API of 19. Remove old workaround.

Reviewed By: #libunwind, danalbert

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

Added: 


Modified: 
libunwind/src/AddressSpace.hpp

Removed: 




diff  --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index f1ba94ed732e3..315289cd4211b 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -373,28 +373,6 @@ LocalAddressSpace::getEncodedP(pint_t &addr, pint_t end, 
uint8_t encoding,
   typedef ElfW(Addr) Elf_Addr;
 #endif
 
-static Elf_Addr calculateImageBase(struct dl_phdr_info *pinfo) {
-  Elf_Addr image_base = pinfo->dlpi_addr;
-#if defined(__ANDROID__) && __ANDROID_API__ < 18
-  if (image_base == 0) {
-// Normally, an image base of 0 indicates a non-PIE executable. On
-// versions of Android prior to API 18, the dynamic linker reported a
-// dlpi_addr of 0 for PIE executables. Compute the true image base
-// using the PT_PHDR segment.
-// See https://github.com/android/ndk/issues/505.
-for (Elf_Half i = 0; i < pinfo->dlpi_phnum; i++) {
-  const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i];
-  if (phdr->p_type == PT_PHDR) {
-image_base = reinterpret_cast(pinfo->dlpi_phdr) -
-  phdr->p_vaddr;
-break;
-  }
-}
-  }
-#endif
-  return image_base;
-}
-
 struct _LIBUNWIND_HIDDEN dl_iterate_cb_data {
   LocalAddressSpace *addressSpace;
   UnwindInfoSections *sects;
@@ -468,7 +446,7 @@ static int findUnwindSectionsByPhdr(struct dl_phdr_info 
*pinfo,
   (void)pinfo_size;
 #endif
 
-  Elf_Addr image_base = calculateImageBase(pinfo);
+  Elf_Addr image_base = pinfo->dlpi_addr;
 
   // Most shared objects seen in this callback function likely don't contain 
the
   // target address, so optimize for that. Scan for a matching PT_LOAD segment



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

[PATCH] D131793: Fix assert on startup in clang-doc

[Paul Kirth via Phabricator via cfe-commits]

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

This is LGTM, but can you add a tag to the summary title/commit message? ie. 
`[clang-doc] Fix assert on startup in clang-doc`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131793

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

[PATCH] D131739: Fix some clang-doc issues.

[Paul Kirth via Phabricator via cfe-commits]

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

Can you update the summary to reflect what's in this change now + add a tag for 
clang-doc?

`[clang-doc] Always emit the TagType for RecordInfo`

I can land this and D131793  once the 
summaries are in order.


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

https://reviews.llvm.org/D131739

___
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 marked an inline comment as done.
python3kgae added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:112-113
+}
+if (!Tok.is(tok::identifier)) {
+  Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;
+  SkipUntil(tok::r_paren, StopAtSemi); // skip through )

aaron.ballman wrote:
> Any reason not to use `ExpectAndConsume(tok::identifier, diag::err_expected)` 
> here instead?
Need to save the identifier string and loc for later use.
And the consume is done in ParseIdentifierLoc.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6992
+
+  if (!Space.startswith_insensitive("space")) {
+S.Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;

aaron.ballman wrote:
> I'm surprised we want to be case insensitive here; we don't document that we 
> are, but also, we're not case insensitive with the slot letters so it seems 
> inconsistent. Is this part of the HLSL requirements?
> 
> (Note, I have no problems with doing a case insensitive check if the case 
> sensitive check fails so we can give a better warning + fixit, if you thought 
> that was useful.)
Discussed with the team, we'll limit to lower case now.
If need for case insensitive for back-compat in the future, it is easy to 
enable it.


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

[clang-tools-extra] 42ee0d8 - [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the result is not used

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2022-08-12T21:00:11+03:00
New Revision: 42ee0d8c16f7052c3bc8434325868f48a501baf2

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

LOG: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if 
the result is not used

IncludeCleaner.RecursiveInclusion and IncludeCleaner.IWYUPragmaExport tests 
don't check referenced files list, so we don't need to call 
findReferencedFiles() there.

Reviewed By: kbobyrev

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 527db2a5d2193..d79f1219511f8 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -571,9 +571,6 @@ TEST(IncludeCleaner, RecursiveInclusion) {
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
@@ -596,9 +593,6 @@ TEST(IncludeCleaner, IWYUPragmaExport) {
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
   // because we ignore headers with IWYU export pragmas for now.



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

[PATCH] D131706: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the result is not used

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG42ee0d8c16f7: [clangd][unittests][IncludeCleaner] Don't 
call findReferencedFiles() if the… (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131706

Files:
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -571,9 +571,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
@@ -596,9 +593,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
   // because we ignore headers with IWYU export pragmas for now.


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -571,9 +571,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
@@ -596,9 +593,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
   // because we ignore headers with IWYU export pragmas for now.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131351#3718728 , @mizvekov wrote:

> In D131351#3718725 , @aaron.ballman 
> wrote:
>
>> However, we don't care about the type mismatch when doing a redeclaration or 
>> when dropping the attribute, as in:
>>
>>   __attribute__((noreturn)) void my_exit(void);
>>   void (*handler)(void) = my_exit; // Silently drops the attribute
>>   
>>   _Noreturn void my_exit(void) { // No concerns about the type mismatch on 
>> redeclaration because we inherit __attribute__((noreturn))
>>   }
>>
>> so maybe we shouldn't worry about it here.
>
> Are you sure? If the function types don't match in C++, I would expect that 
> to become a different overload, not a redeclaration, right?

The changes here only impact C so it shouldn't be a big deal, but we do not 
create an overload set for them in C with the `overloadable` attribute: 
https://godbolt.org/z/1ETe6ff5s

I tracked down the `noreturn` behavior and this turns out to be intentional for 
the past ten years and is a result of a bug report I can't see: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L1526
 (https://github.com/llvm/llvm-project/commit/48c69106e4ce1 was what added the 
functionality.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131351

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

[PATCH] D131793: [clang-doc] Fix assert on startup

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

In D131793#3719619 , @paulkirth wrote:

> This is LGTM, but can you add a tag to the summary title/commit message? ie. 
> `[clang-doc] Fix assert on startup in clang-doc`

Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131793

___
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 added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6558
+Here're resource binding examples with and without space:
+``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

Two things:

* The example doesn't use valid RST, it should look more like what's done on 
AttrDocs:649 (using `.. code-block:: c++`)
* I think the example is missing semicolons at the end of the declarations?



Comment at: clang/lib/Parse/ParseHLSL.cpp:112-113
+}
+if (!Tok.is(tok::identifier)) {
+  Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;
+  SkipUntil(tok::r_paren, StopAtSemi); // skip through )

python3kgae wrote:
> aaron.ballman wrote:
> > Any reason not to use `ExpectAndConsume(tok::identifier, 
> > diag::err_expected)` here instead?
> Need to save the identifier string and loc for later use.
> And the consume is done in ParseIdentifierLoc.
Ah, that's a good reason, thanks!



Comment at: clang/lib/Sema/SemaDecl.cpp:2893-2894
+  else if (const auto *RB = dyn_cast(Attr))
+NewAttr =
+S.mergeHLSLResourceBindingAttr(D, *RB, RB->getSlot(), RB->getSpace());
   else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))

I don't see any tests covering this code path. How do you get such a 
redeclaration in HLSL?



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) {
+

Isn't this a re-definition of `a` which would cause an error?


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] D131739: [clang-doc] Always emit the TagType for RecordInfo

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

In D131739#3719634 , @paulkirth wrote:

> Can you update the summary to reflect what's in this change now + add a tag 
> for clang-doc?
>
> `[clang-doc] Always emit the TagType for RecordInfo`
>
> I can land this and D131793  once the 
> summaries are in order.

Done


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

https://reviews.llvm.org/D131739

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

[PATCH] D131796: [clang][deps] Use a cc1 invocation in FullDependencies::getCommandLine()

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, Bigcheese.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of trying to "fix" the original driver invocatin by appending
arguments to it, use a CompilerInvocation to give precise control over
the invocation, and serialize it to a -cc1 command.

This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache) or apply
optimizations, similar to what we do for module build commands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131796

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/diagnostics.c
  clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
  clang/test/ClangScanDeps/modules-context-hash-outputs.c
  clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/ClangScanDeps/modules-inferred.m
  clang/test/ClangScanDeps/modules-pch-common-submodule.c
  clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
  clang/test/ClangScanDeps/modules-pch.c
  clang/test/ClangScanDeps/removed-args.c

Index: clang/test/ClangScanDeps/removed-args.c
===
--- clang/test/ClangScanDeps/removed-args.c
+++ clang/test/ClangScanDeps/removed-args.c
@@ -73,9 +73,10 @@
 // CHECK-NEXT: }
 // CHECK-NEXT:   ]
 // CHECK-NEXT:   "command-line": [
-// CHECK-NEXT: "-fsyntax-only",
+// CHECK-NEXT: "-cc1",
 // CHECK-NOT:  "-fmodules-cache-path=
 // CHECK-NOT:  "-fmodules-validate-once-per-build-session"
+// CHECK-NOT:  "-fbuild-session-timestamp=
 // CHECK-NOT:  "-fbuild-session-file=
 // CHECK-NOT:  "-fmodules-prune-interval=
 // CHECK-NOT:  "-fmodules-prune-after=
Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -92,11 +92,11 @@
 // CHECK-PCH-NEXT: }
 // CHECK-PCH-NEXT:   ],
 // CHECK-PCH-NEXT:   "command-line": [
-// CHECK-PCH:  "-fno-implicit-modules",
-// CHECK-PCH-NEXT: "-fno-implicit-module-maps",
-// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
-// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm"
-// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NOT:  "-fimplicit-modules",
+// CHECK-PCH-NOT:  "-fmplicit-module-maps",
+// CHECK-PCH:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
+// CHECK-PCH:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm"
+// CHECK-PCH:],
 // CHECK-PCH-NEXT:   "file-deps": [
 // CHECK-PCH-NEXT: "[[PREFIX]]/pch.h"
 // CHECK-PCH-NEXT:   ],
@@ -154,10 +154,10 @@
 // CHECK-TU-NEXT: }
 // CHECK-TU-NEXT:   ],
 // CHECK-TU-NEXT:   "command-line": [
-// CHECK-TU:  "-fno-implicit-modules",
-// CHECK-TU-NEXT: "-fno-implicit-module-maps",
-// CHECK-TU-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm"
-// CHECK-TU-NEXT:   ],
+// CHECK-TU-NOT:  "-fimplicit-modules",
+// CHECK-TU-NOT:  "-fimplicit-module-maps",
+// CHECK-TU:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm"
+// CHECK-TU:],
 // CHECK-TU-NEXT:   "file-deps": [
 // CHECK-TU-NEXT: "[[PREFIX]]/tu.c",
 // CHECK-TU-NEXT: "[[PREFIX]]/pch.h.gch"
@@ -213,11 +213,11 @@
 // CHECK-TU-WITH-COMMON-NEXT: }
 // CHECK-TU-WITH-COMMON-NEXT:   ],
 // CHECK-TU-WITH-COMMON-NEXT:   "command-line": [
-// CHECK-TU-WITH-COMMON:  "-fno-implicit-modules",
-// CHECK-TU-WITH-COMMON-NEXT: "-fno-implicit-module-maps",
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModTUWithCommon-{{.*}}.pcm"
-// CHECK-TU-WITH-COMMON-NEXT:   ],
+// CHECK-TU-WITH-COMMON-NOT:  "-fimplicit-modules",
+// CHECK-TU-WITH-COMMON-NOT:  "-fimplicit-module-maps",
+// CHECK-TU-WITH-COMMON:  "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
+// CHECK-TU-WITH-COMMON:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModT

[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.

> In D119051#3715939 , @aaron.ballman 
> wrote:
>
>> In D119051#3714645 , @dblaikie 
>> wrote:
>>
>>> 
>>
>> I would have thought use of `__is_pod` would tell us, but I'm not seeing the 
>> behavior described in the test case when using that: 
>> https://godbolt.org/z/1vr3MK4KW Oddly, it seems that 
>> `QualType::isCXX11PODType()` doesn't look at `PlainOldData` at all! What is 
>> your expectation as to how the type trait should be behaving?
>
> Oh, yeah, seems @rsmith and I discussed this naming/expectations issue a bit 
> over here previously: https://reviews.llvm.org/D117616#inline-1132622

Ah, thank you for that!

In D119051#3717934 , @dblaikie wrote:

> I guess the other way to test for pod-for-purposes-of-ABI is IRgen. Looks 
> like MSVC isn't observable based on sizeof or alignof on these issues (the 
> previous godbolt shows MSVC's answer to alignment for the packed and size for 
> the trailing packing don't change based on any of the variations (pod, 
> non-pod, pod-with-defaulted-special-members)) - but should be observable 
> based on the returning ABI.
>
> Ah, here we go: https://godbolt.org/z/sd88zTjPP
>
> So MSVC does consider the defaulted special member as still a valid 
> pod-for-purposes-of-ABI and clang is incorrect/not ABI compatible with this. 
> So MSVC does want this fix.

Yeah this strikes me as the right kind of test to add for testing the ABI 
(codegen cares more about ABI than Sema, broadly speaking).


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

[clang-tools-extra] 75c7e79 - [clang-doc] Fix assert on startup

[Paul Kirth via cfe-commits]

Author: Brett Wilson
Date: 2022-08-12T18:37:23Z
New Revision: 75c7e79464a3e77043571d187cd8019370326bcb

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

LOG: [clang-doc] Fix assert on startup

When using `clang-doc --format=html` it will crash on startup because of an 
assertion doing a self-assignment of a `SmallString`. This patch removes the 
self-assignment by creating an intermediate copy.

Reviewed By: paulkirth

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

Added: 


Modified: 
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp 
b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
index fc22e55edfb1a..7531f219a6a40 100644
--- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@ Example usage for a project using a compile commands 
database:
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);



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

[PATCH] D131793: [clang-doc] Fix assert on startup

[Paul Kirth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG75c7e79464a3: [clang-doc] Fix assert on startup (authored by 
brettw, committed by paulkirth).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131793

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits